[CLNP-6501][fix]: disable mentions and sanitize HTML in edit mode#1418
Conversation
Mentions are not functional in edit mode, so disable mention detection
by setting isMentionEnabled={false} on MessageInput. Also sanitize
mentionTemplate in editMessage to prevent HTML tag injection via
contentEditable.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
✅ Deploy Preview for sendbird-uikit-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f248bd516
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ref={editMessageInputRef} | ||
| mentionSelectedUser={selectedUser} | ||
| isMentionEnabled={groupChannel.enableMention} | ||
| isMentionEnabled={false} |
There was a problem hiding this comment.
Setting isMentionEnabled to false in edit mode makes MessageInput take its mention-disabled initialization path, so a previously mentioned message is loaded as plain text instead of mention-label nodes; when the user saves, extractTextAndMentions() no longer emits @{userId} tokens and onUpdateMessage sends a non-mention template. In practice, editing even a typo in a message that already mentions users will silently strip those mentions from the updated message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
isMentionEnabled={false} 접근을 철회하고, Android UIKit과 동일하게 편집 시에도 멘션 기능이 완전히 동작하도록 수정했습니다.
주요 변경:
- isMentionEnabled={groupChannel.enableMention} 유지 (기존 멘션 보존)
- 저장 시 DOM에서 mentionedUserIds를 직접 추출하여 mentionedMessageTemplate과 항상 일치하도록 처리
- 기존 코드의 mentionedUsers state가 항상 []이었던 문제도 함께 수정
| ref={editMessageInputRef} | ||
| mentionSelectedUser={selectedUser} | ||
| isMentionEnabled={groupChannel.enableMention} | ||
| isMentionEnabled={false} |
There was a problem hiding this comment.
codex comment 확인해 주세요.
{false}로 지정을 하게 되면 mention 기능 자체를 막게 되는거 아닌가요?
There was a problem hiding this comment.
맞습니다. isMentionEnabled={false}는 기존 멘션까지 strip하는 문제가 있었습니다. Android UIKit을 확인해보니 편집 시에도 멘션 추가/삭제/보존이 모두 지원되고 있어서, 동일하게 동작하도록 방향을 변경했습니다. 추가로 기존 코드에서 mentionedUsers state가 항상 빈 배열로 전달되던 문제도 수정하여, DOM에서 mentionedUserIds를 직접 추출하는 방식으로 개선했습니다.
감사합니다~!!
…dUserIds from DOM Extract mentionedUserIds from DOM mention labels at save time instead of using always-empty mentionedUsers state. This ensures mentionedUserIds and mentionedMessageTemplate are always consistent, matching Android UIKit behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Use mentionedUserIds from DOM extraction in Thread's ParentMessageInfo and ThreadListItem edit flows, consistent with GroupChannel fix. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
| const params = { | ||
| messageId, | ||
| message: messageText, | ||
| mentionTemplate: isMentionedMessage ? sanitizeString(mentionTemplate) : messageText, |
There was a problem hiding this comment.
이 부분은 mention이 없이 messageText에 HTML Tag가 들어가 있으면 senitizeString()없이 진행이 되게 되는 거라 PR의 2번 내용은 해결이 안되는 거 아닌가요?
예를 들어 "Hi. @danney-chun " 이거 였었는데..
"Hi. " 이렇게 변경이 되는 경우에는 계속 HTML Tag가 그대로 보이는 형태가 될 것 같은데요.
There was a problem hiding this comment.
수정 완료했습니다. 확인 중에 message 필드도 sanitize 가 빠져있고 sendMessage 경로도 동일 누락이 있어서 함께 수정했습니다. 감사합니다!
There was a problem hiding this comment.
이거 시작 시점과 display되는 시점을 다시 확인해 보니 제가 단 comment에 문제가 있습니다.
messageText를 sanitizeString()을 하게 되면, 메시지를 전송 시점에 raw data가 변경되는 거라, 표시하는 쪽에 별도 처리가 없는 상태라면 breaking change 요소가 될 것 같습니다.
요건 죄송하지만 mentionTemplate만 하는 것으로 rollback을 ㅠ.ㅠ
There was a problem hiding this comment.
네, 확인 감사합니다. 수정완료하였습니다!
- editMessage: apply sanitizeString to message and mentionTemplate regardless of mention presence (addresses review by danney-chun) - sendMessage: apply sanitizeString to message and mentionTemplate - extractTextAndMentions: also return mentionedUserIds to remove duplicate DOM traversal in editMessage - tests: add sanitize regression cases for sendMessage and editMessage Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Per review feedback on PR #1418: sanitizing the `message` field at send time changes the cross-platform raw data contract and would surface escaped entities (e.g. `<b>`) on other clients (iOS/Android/SDK) that don't HTML-decode for display. The React display layer already escapes angle brackets via JSX, and the edit-input load path already sanitizes via innerHTML, so the original CLNP-6501 concern is still covered without modifying the persisted message data. - sendMessage: revert sanitizeString on `message`, keep it on mentionTemplate - editMessage: revert sanitizeString on `message`, keep it on mentionTemplate - tests: re-pin behavior as raw passthrough for `message` + sanitize for mentionTemplate Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Resolve conflicts in three files where main introduced the
useTypingLifecycle hook while this branch refactored
onUpdateMessage to destructure mentionedUserIds.
- src/modules/GroupChannel/components/Message/MessageView.tsx
- src/modules/Thread/components/ParentMessageInfo/index.tsx
- src/modules/Thread/components/ThreadList/ThreadListItem.tsx
For each conflict, keep main's onStartTyping={startTyping} /
onStopTyping={stopTyping} (from the new useTypingLifecycle hook) and
this branch's destructuring of message: editedMessage,
mentionedUserIds: currentMentionedUserIds in onUpdateMessage, so the
edit-mode message and mentionedUserIds are sourced from the DOM at
save time and typing state is cleaned up consistently.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
2 participants
Description
Fixes two issues in message edit mode:
Mention suggestion list shown in edit mode — The
SuggestedMentionListViewappeared when typing@in edit mode, even though mentions are not functional during editing. Fixed by settingisMentionEnabled={false}onMessageInputin edit mode, which disables mention detection and prevents the suggestion list from appearing.HTML tag injection via contentEditable — The
mentionTemplatepassed toonUpdateMessagewas not sanitized, allowing HTML tags to be included in the edited message. Fixed by applyingsanitizeString()tomentionTemplatein theeditMessagefunction.Root Cause
MessageInputin edit mode hadisMentionEnabled={groupChannel.enableMention}, enabling mention detection even though edit mode does not support adding new mentions.editMessagefunction passed rawmentionTemplatefromextractTextAndMentions()without sanitization.Changes
src/modules/GroupChannel/components/Message/MessageView.tsxisMentionEnabled={false}forMessageInputin edit modesrc/ui/MessageInput/index.tsxsanitizeString()tomentionTemplateineditMessageTicket