refactor: update ui and logic for comments#166
Open
fwy13 wants to merge 1 commit into
Open
Conversation
|
|
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Review Summary by QodoImplement custom comments system with replies and spoiler support
WalkthroughsDescription• Implemented comprehensive comments component with nested replies support • Added spoiler reveal functionality and content rendering with mentions/links • Integrated like/dislike voting system and pinned comment indicators • Added internationalization support for English and Vietnamese languages • Replaced Facebook comments embed with custom comments component Diagramflowchart LR
A["Comments Component"] --> B["Display Comments List"]
A --> C["Load More Comments"]
B --> D["Render Replies"]
B --> E["Spoiler Reveal"]
B --> F["Like/Dislike Votes"]
D --> G["Load More Replies"]
A --> H["i18n Translations"]
I["Season Page"] --> J["Replace FB Comments"]
J --> A
File Changes1. src/components/comments/index.vue
|
Code Review by Qodo
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+104
to
+112
| <div class="relative"> | ||
| <div | ||
| class="text-sm text-gray-300 leading-relaxed mb-2 whitespace-pre-wrap break-words transition-all duration-300" | ||
| :class="{ | ||
| 'filter blur-md select-none pointer-events-none opacity-50': | ||
| comment.is_spoiler == 1 && !comment.isRevealed | ||
| }" | ||
| v-html="renderContent(comment.content)" | ||
| ></div> |
There was a problem hiding this comment.
1. Xss via v-html 🐞 Bug ⛨ Security
The component renders comment/reply content with v-html using renderContent() that returns unsanitized HTML built from user data. A crafted comment (or sticker/url) can inject arbitrary HTML/JS by breaking out of attributes in the generated <a href="..."> or <img src="...">.
Agent Prompt
### Issue description
`renderContent()` returns HTML strings derived from user-controlled comment text and then injects them into the DOM via `v-html`. Because the code interpolates captured URLs and sticker URLs into HTML attributes without escaping/sanitization, an attacker can inject scripts/HTML.
### Issue Context
You need linkification/mentions/stickers, but it must be done without allowing arbitrary HTML execution.
### Fix Focus Areas
- src/components/comments/index.vue[104-112]
- src/components/comments/index.vue[235-243]
- src/components/comments/index.vue[493-520]
### Implementation guidance
- Escape user text first (e.g., `escapeHtml(content)`) before doing any replacements.
- Avoid building HTML with string concatenation for attributes. Prefer creating DOM nodes (or Vue VNodes) and setting `href/src/textContent` properties.
- If you must use `v-html`, sanitize the final HTML with a vetted sanitizer (e.g., DOMPurify) and explicitly restrict allowed tags/attributes.
- Validate URLs before rendering:
- allow only `http:`/`https:` schemes
- reject/strip quotes and control characters
- consider normalizing and rendering unsafe URLs as plain text.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+394
to
+429
| watch( | ||
| () => props.filmId, | ||
| () => { | ||
| commentsList.value = [] | ||
| currentOffset.value = 0 | ||
| hasMoreComments.value = true | ||
| totalComments.value = 0 | ||
| loadMoreComments() | ||
| } | ||
| ) | ||
|
|
||
| const loadMoreComments = async () => { | ||
| if (isLoadingComments.value || !hasMoreComments.value) return | ||
| isLoadingComments.value = true | ||
|
|
||
| try { | ||
| const response = await get({ | ||
| url: `${C_URL}/ajax/comment?action=get&film_id=${props.filmId}&sort=newest&offset=${currentOffset.value}`, | ||
| responseType: "json" | ||
| }) | ||
|
|
||
| const data = response.data as any | ||
| if (data.success) { | ||
| const newComments = data.comments.map((c: any) => ({ | ||
| ...c, | ||
| replies: c.replies || [], | ||
| replies_offset: 0, | ||
| isLoadingReplies: false, | ||
| isRevealed: false | ||
| })) | ||
|
|
||
| commentsList.value.push(...newComments) | ||
| currentOffset.value = data.offset | ||
| hasMoreComments.value = data.has_more | ||
| totalComments.value = data.total | ||
| } |
There was a problem hiding this comment.
2. Stale comments on filmid change 🐞 Bug ≡ Correctness
When filmId changes, the watcher clears state but does not cancel/ignore in-flight requests, and responses are applied without verifying they match the current filmId. This can append comments from the previous film into the new film’s list if the earlier request resolves after the reset.
Agent Prompt
### Issue description
Changing `filmId` can cause older in-flight requests to resolve after state reset and still mutate the current `commentsList`, showing comments for the wrong film.
### Issue Context
The component uses a shared `isLoadingComments` flag and does not cancel or validate responses against the `filmId` used to start the request.
### Fix Focus Areas
- src/components/comments/index.vue[394-403]
- src/components/comments/index.vue[405-429]
### Implementation guidance
Choose one (or combine):
- **Request token approach (no AbortController needed):**
- Maintain `let requestSeq = 0` in module scope.
- In `loadMoreComments`, capture `const seq = ++requestSeq` and `const filmIdAtStart = props.filmId`.
- After `await get(...)`, before applying results, check `if (seq !== requestSeq || props.filmId !== filmIdAtStart) return`.
- In the `watch` handler, increment `requestSeq` to invalidate prior requests.
- **Abort approach (if supported by your Http.get wrapper):**
- Keep an `AbortController` per filmId and abort it on `filmId` change.
Also consider guarding the watcher so it doesn’t call `loadMoreComments()` when `filmId` is falsy.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feature
To-Do