-
Notifications
You must be signed in to change notification settings - Fork 13.1k
feat: ai enhanced message composer. #36526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/real-time-composer
Are you sure you want to change the base?
feat: ai enhanced message composer. #36526
Conversation
- Replaced the `ComposerMessageInput`'s underlying `<textarea>` with a `<div contenteditable>`. - Currently, this implementation is non-functional beyond accepting plain text input. - Placeholder support is currently not working due to limitations with `contenteditable`. - Further work is needed to restore full functionality, including keyboard event handling, Markdown formatting, and cursor management. - Clipboard paste and file attachments continue to work as expected. - Updated Storybook stories to include `MessageComposerInputNew` for preview and testing. - Refactored existing stories and exports for clarity and compatibility with the new input component.
- Cloned `CreateComposerAPI.ts` as `newCreateComposerAPI.ts` and updated references from `HTMLTextAreaElement` to `HTMLDivElement`. - Implemented `getSelectionRange` and `setSelectionRange` using the Selection API to replace `input.selectionStart` and `input.selectionEnd`. - Fixed Enter key functionality, allowing messages to be sent as expected. - Restored Bold, Italics, and Strikethrough button functionality, confirming successful Selection API migration. Known Issues & Next Steps - Enabled `/` key to trigger the slash command menu, but pressing Enter currently inserts the command and sends the message immediately. - Emoji autocomplete works, but pressing Enter finalizes the emoji and sends the message. - **The Send button remains visually disabled**, requiring an additional fix to update its state. - Next step: Prevent message sending when popups (slash command, emoji autocomplete) are active.
- Updated reducer function to handle FormEvent<HTMLDivElement> instead of FormEvent<HTMLInputElement>. - Changed event target from HTMLInputElement to HTMLDivElement for compatibility with contenteditable. - Used `innerText.trim()` instead of `value.trim()` to correctly determine if the message input is empty. - Replaced `onChange` with `onInput` in `MessageComposerInputNew` to properly detect user input. This ensures the Send button is correctly enabled or disabled based on input presence. Known Issues & Next Steps: - Pressing `/` opens the slash command menu, but pressing Enter inserts the command and sends the message immediately. - Emoji autocomplete works, but Enter finalizes the emoji and sends the message. - Next step: Prevent message sending when popups (slash command or emoji autocomplete) are active.
- Introduced `featurePreviewComposer` flag to toggle between the new <div contenteditable> input and the classic <textarea> input. - Conditionally render `MessageBoxNew` or `MessageBox` based on the flag. - Added `MessageComposerHint` in `MessageBoxNew` to label the feature as experimental.
- Changed class name from `rc-message-box__textarea` to `rc-message-box__divcontenteditable`. - Rectified string values for `minHeight` and `maxHeight` with numeric values. - Corrected `minHeight` from `52px` to `20px` to match new layout requirements.
This refactor removes ambiguity and improves naming consistency across the message composer components. Renamed: - `MessageComposerInputNew.tsx` → `RichTextComposerInput.tsx` - `MessageBoxNew.tsx` → `RichTextMessageBox.tsx` - `newCreateComposerAPI.ts` → `createRichTextComposerAPI.ts` Updated references in: - `ComposerMessage.tsx` - `MessageComposer.stories.tsx` - `MessageComposer/index.ts`
- Moved `getSelectionRange` and `setSelectionRange` from `createRichTextComposerAPI.ts` to a new `selectionRange.ts` file. - Improves separation of concerns and allows reuse across other modules.
- Replaced `input.value` and `input.selectionEnd` with `innerText` and `getSelectionRange` - Used `setSelectionRange` to handle caret movement - Updated event targets and types from HTMLTextAreaElement to HTMLDivElement
This is a major bugfix that resolves an erratic behavior during keydown events - Added check to skip auto-focus when target is a <div contenteditable="true"> - Prevented focus tug-of-war between main and thread composers - Improved stability of typing behavior in multi-composer layouts - Resolved a critical issue where main and thread composers kept stealing focus from each other - Stabilized typing behavior in multi-composer scenarios
- Implemented a `WeakMap` to store the last cursor position per `contenteditable div` instance - Saved cursor position on `blur` event and restored it on `focus` event
This is a major bugfix that resolves an erratic behavior during Editing mode - Editing mode failed to reset due to `.innerText` collapsing multiple spaces - Caused mismatch between original message and `RichTextComposer` content - Fixed by adding `whiteSpace: 'pre-wrap'` `to RichTextComposerInput` - Ensures consistent text comparison and reliable edit cancellation - Added comment in the `RichTextComposerInput` definition to highlight the significance
- Blocked browser insertion of <b> and <i> tags - Delegated formatting to custom shortcut handler
This issue was caused due to the editor losing focus and cursor position state when clicking a Formatter button. - Added `setSelectionRange` and `focus` prior to `execCommand` call - Fixed issue where the button would append instead of replacing the selected text
- Fixed cursor moving to end of contentEditable input and regaining focus after editing reset
Draft messages were not properly stored and restored due to use of innerText, which strips formatting. - Switched to innerHTML to retain full content structure in Accounts.storageLocation.
- Added TypingState reducer to manage `isTyping` and `hidePlaceholder` flags - Adjusted placeholder visibility based on input DOM structure - Normalized content to treat `<div><br></div>` and empty input as `<br>` - Updated RichTextComposerInput to support and render `hidePlaceholder` prop - Ensured cursor is text-style inside `contenteditable` element - Prevented saving of Composer drafts when content is only `<br>`
- Replaced `_MessageComposerNew` story with `RichTextComposer` for clarity - Added `MessageComposerHint` to highlight experimental status - Passed `placeholder` and new `hidePlaceholder` prop to `RichTextComposerInput` - Updated Storybook args for enhanced prop control
- Corrected the forwarded `ref` target to match the `contentEditable div` - Ensures proper typing and `ref` behavior for editable container - Minor style cleanup for whiteSpace and cursor properties
- Applied `overflow-y: scroll` to the `contenteditable div` for consistent scrollbar visibility - Styled `::-webkit-scrollbar-thumb` to fix invisible scrollbar in Chromium browsers
- Updated `getSelectionRange` to walk DOM and compute offsets precisely - Skipped inline elements and handled `<br>` inside empty `<div>` as visual linebreak - Added offset increment for block-level elements to reflect line structure - Updated `setSelectionRange` to match same offset logic for accurate selection - Resolved inconsistency between `selectionStart` and `selectionEnd` and visual cursor placement
- Normalized multiple newlines in `innerText` during selection wrapping - Added selectionchange listener to debug selection range - Implemented listener cleanup on component release
- Updated `cursorMap` to store both `selectionStart` and `selectionEnd` for accurate range restoration - Replaced single-value tracking with object structure in `WeakMap` - Improved `setSelectionRange` on focus to restore full selection, not just caret
- Emoji shortcodes convert into Unicode emoticons. - Custom uploaded emojis render as <span> with inline <img> and shortcode fallback.
ada8c6b to
e34c352
Compare
This is still a work in progress - Paragraphs: replaced `<p>` wrappers with raw text + newline - Headings: preserve `#` prefix inside `<hX>` tags - Unordered lists: prefix list items with `-` - Ordered lists: include numeric prefix and `value` attribute - Code blocks: render with Markdown-style fences (```lang ... ```) - Line breaks: output raw `\n` instead of `<br />`
- Switched `resolveComposerBox` to run on `input` instead of `beforeinput`
- Removed unused undo/redo and focus event handling logic
- Improved `protectLinks`:
- Added email detection
- Prevented mentions (`@something`) from being parsed as URLs
- Adjusted bare domain regex to exclude `@` prefixes
- Updated rendering pipeline:
- Writes `innerHTML` directly (works but clears undo history)
- Left comments for `execCommand('insertHTML')` as alternative that preserves history
- Cursor position is restored after re-render
- Removed unused types (`Dispatch`, `SetStateAction`, `CursorHistory`) - Dropped debug helper `printSelection` and selection tracking - Replaced `beforeinput` listener with unified `input` listener - Updated `resolveComposerBox` signature to remove state dependencies - Cleaned up event removal in `release` function
- Dropped unused types (`Dispatch`, `SetStateAction`, `CursorHistory`) - Removed `setCursorHistory` state and related resolver calls - Simplified `handleFormattingShortcut` to only wrap selection - Cleaned up `setLastCursorPosition` and keyboard/newline handlers by removing resolver logic - Updated `createRichTextComposerAPI` usage to new signature without cursor/history state - Removed `useEffect` that resolved composer after popup option selection
e34c352 to
7ed8bb9
Compare
WalkthroughThis pull request introduces a complete rich text message composer feature for Rocket.Chat. It includes a composer API factory with state management and formatting support, message parsing infrastructure, DOM selection utilities, a React component for the rich text editor with AI enhancement integration, and a feature preview toggle to conditionally render between the legacy and new composer implementations. Changes
Sequence DiagramsequenceDiagram
participant User
participant RichTextBox as RichTextMessageBox
participant API as ComposerAPI
participant Parser as messageParser
participant Handler as messageStateHandler
participant Selection as selectionRange
participant DOM as contentEditable
User->>DOM: Input text
DOM->>RichTextBox: onInput event
RichTextBox->>Selection: getSelectionRange()
Selection->>Selection: traverse DOM tree
Selection-->>RichTextBox: cursor position
RichTextBox->>Handler: resolveComposerBox()
Handler->>Handler: protect links
Handler->>Parser: parseAST()
Parser->>Parser: render tokens to HTML
Parser-->>Handler: HTML string
Handler->>Handler: restore links
Handler->>DOM: inject HTML + restore cursor
User->>RichTextBox: press Enter
RichTextBox->>API: getText()
API-->>RichTextBox: message content
RichTextBox->>RichTextBox: onSend callback
User->>RichTextBox: select text (AI)
RichTextBox->>RichTextBox: useAIEnhancement trigger
RichTextBox->>RichTextBox: show AI toolbar
User->>RichTextBox: click Summary
RichTextBox->>RichTextBox: wrap selection + animate
RichTextBox->>RichTextBox: simulate API (5s)
RichTextBox->>RichTextBox: typing animation reveal
RichTextBox->>RichTextBox: show accept/reject
User->>RichTextBox: accept
RichTextBox->>DOM: replace with result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (4)
apps/meteor/client/views/room/composer/ComposerMessage.tsx (1)
31-31: Remove or relocate the misplaced comment.The comment on line 31 describes contenteditable toggle behavior, but it's placed above unrelated code (
const chat = useChat()). The actual feature toggle happens at lines 95-102 via theFeaturePreviewcomponent.Apply this diff to remove the disconnected comment:
- // true: enables contenteditable <div>; false: uses classic <textarea> composer const chat = useChat();Or relocate it to the actual toggle point:
- // true: enables contenteditable <div>; false: uses classic <textarea> composer const chat = useChat(); const room = useRoom(); const dispatchToastMessage = useToastMessageDispatch(); const composerProps = useMemo( ... if (!publicationReady) { return <ComposerSkeleton />; } + // Feature flag: realtimeMessageComposer enables contenteditable <div> (RichTextMessageBox); + // otherwise uses classic <textarea> (MessageBox) return ( <FeaturePreview feature='realtimeMessageComposer'>apps/meteor/client/views/room/composer/messageBox/hooks/useMessageBoxAutoFocus.ts (1)
32-35: Consider broadening the contentEditable check for future-proofing.The guard correctly prevents focus stealing when users type in contentEditable SPANs within the RichTextComposer. However, the check is specific to SPAN elements only.
If the RichTextComposer might use other contentEditable elements (DIV, P, etc.) now or in the future, consider a more generic check:
- // This prevents focus from being stolen between the two RichText Composers - if ((target as HTMLElement).tagName === 'SPAN' && (target as HTMLElement).isContentEditable) { - return; - } + // This prevents focus from being stolen when typing in contentEditable elements within RichText Composers + if ((target as HTMLElement).isContentEditable) { + return; + }This would catch any contentEditable element, not just SPANs, making the code more resilient to future changes in the RichTextComposer implementation.
packages/i18n/src/locales/en.i18n.json (1)
4103-4104: Prefer “Real‑time” and tighten the description for consistency.Other entries (e.g., “Real‑time Monitoring”) hyphenate “Real‑time.” Suggest updating only values (no key rename) to keep consistency and improve clarity.
- "Realtime_message_composer": "Realtime message composer", - "Realtime_message_composer_description": "Realtime preview of message formatting when using the message composer", + "Realtime_message_composer": "Real-time message composer", + "Realtime_message_composer_description": "Real-time preview of message formatting while composing messages",
- Confirm useFeaturePreviewList (and any UI) references these exact keys (names unchanged).
- If your workflow requires, mirror these strings to other locales or ensure fallback behavior.
apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts (1)
90-93: Drop debug logging before merge
console.log(ast);spams the console on every input event. Please remove it.Apply this diff:
- console.log(ast);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts(1 hunks)apps/meteor/app/ui-message/client/messageBox/messageParser.ts(1 hunks)apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts(1 hunks)apps/meteor/app/ui-message/client/messageBox/selectionRange.ts(1 hunks)apps/meteor/client/views/room/composer/ComposerMessage.tsx(4 hunks)apps/meteor/client/views/room/composer/hooks/useAIEnhancement.tsx(1 hunks)apps/meteor/client/views/room/composer/messageBox/RichTextMessageBox.tsx(1 hunks)apps/meteor/client/views/room/composer/messageBox/hooks/useMessageBoxAutoFocus.ts(1 hunks)package.json(1 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/ui-client/src/hooks/useFeaturePreviewList.ts(2 hunks)packages/ui-composer/src/MessageComposer/MessageComposer.stories.tsx(6 hunks)packages/ui-composer/src/MessageComposer/RichTextComposerInput.tsx(1 hunks)packages/ui-composer/src/MessageComposer/index.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/room/composer/ComposerMessage.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/app/ui-message/client/messageBox/messageStateHandler.tsapps/meteor/client/views/room/composer/messageBox/RichTextMessageBox.tsx
🧬 Code graph analysis (8)
apps/meteor/client/views/room/composer/ComposerMessage.tsx (1)
packages/ui-client/src/components/FeaturePreview/FeaturePreview.tsx (3)
FeaturePreview(8-26)FeaturePreviewOff(32-34)FeaturePreviewOn(28-30)
packages/ui-composer/src/MessageComposer/RichTextComposerInput.tsx (1)
packages/ui-composer/src/MessageComposer/index.ts (1)
RichTextComposerInput(26-26)
apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts (5)
apps/meteor/client/lib/chats/ChatAPI.ts (1)
ComposerAPI(8-60)apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts (1)
resolveComposerBox(72-116)apps/meteor/app/ui-message/client/messageBox/selectionRange.ts (2)
getSelectionRange(3-85)setSelectionRange(88-156)apps/meteor/app/ui-message/client/messageBox/messageParser.ts (1)
escapeHTML(5-6)apps/meteor/app/ui-message/client/messageBox/messageBoxFormatting.ts (2)
FormattingButton(30-30)formattingButtons(34-106)
apps/meteor/client/views/room/composer/hooks/useAIEnhancement.tsx (1)
apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts (1)
selection(354-360)
apps/meteor/app/ui-message/client/messageBox/selectionRange.ts (1)
apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts (2)
selection(354-360)text(351-353)
apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts (3)
apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts (2)
text(351-353)selection(354-360)apps/meteor/app/ui-message/client/messageBox/selectionRange.ts (2)
getSelectionRange(3-85)setSelectionRange(88-156)apps/meteor/app/ui-message/client/messageBox/messageParser.ts (1)
parseAST(90-131)
apps/meteor/client/views/room/composer/messageBox/RichTextMessageBox.tsx (7)
apps/meteor/app/ui-message/client/messageBox/messageBoxFormatting.ts (2)
formattingButtons(34-106)FormattingButton(30-30)apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts (1)
getTextLines(12-28)apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts (2)
createRichTextComposerAPI(14-381)text(351-353)apps/meteor/client/views/room/contexts/ComposerPopupContext.ts (1)
useComposerPopupOptions(45-51)apps/meteor/client/views/room/composer/hooks/useComposerBoxPopup.ts (1)
useComposerBoxPopup(52-286)packages/ui-client/src/hooks/useSafeRefCallback/useSafeRefCallback.ts (1)
useSafeRefCallback(29-44)apps/meteor/client/views/room/composer/hooks/useMessageComposerMergedRefs.ts (1)
useMessageComposerMergedRefs(25-30)
packages/ui-composer/src/MessageComposer/MessageComposer.stories.tsx (1)
packages/ui-composer/src/MessageComposer/index.ts (10)
MessageComposerAction(16-16)MessageComposerActionsDivider(17-17)MessageComposerToolbarActions(20-20)MessageComposer(15-15)RichTextComposerInput(26-26)MessageComposerHint(24-24)MessageComposerToolbar(19-19)MessageComposerToolbarSubmit(21-21)MessageComposerInput(18-18)MessageComposerSkeleton(22-22)
🪛 ast-grep (0.39.7)
apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts
[warning] 79-79: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: input.innerHTML = escapeHTML(textAreaTxt.substring(0, selectionStart) + text + textAreaTxt.substring(selectionStart))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 86-86: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: input.innerHTML = escapeHTML(text)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 79-79: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: input.innerHTML = escapeHTML(textAreaTxt.substring(0, selectionStart) + text + textAreaTxt.substring(selectionStart))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 86-86: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: input.innerHTML = escapeHTML(text)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts
[warning] 104-104: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = finalHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 104-104: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = finalHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🪛 Biome (2.1.2)
apps/meteor/app/ui-message/client/messageBox/messageParser.ts
[error] 27-30: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 49-67: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: Builds matrix rust bindings against alpine
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
apps/meteor/client/views/room/composer/ComposerMessage.tsx (1)
94-103: Clean feature flag implementation.The
FeaturePreviewwrapper correctly enables progressive rollout of the newRichTextMessageBoxcomposer while maintaining backward compatibility with the legacyMessageBox. Both components receive identical props, ensuring consistent behavior during the transition.packages/ui-composer/src/MessageComposer/index.ts (1)
12-12: LGTM! RichTextComposerInput added to the public API.The import and export follow the existing barrel export pattern, making
RichTextComposerInputavailable for consumption alongside other MessageComposer components.Also applies to: 26-26
package.json (1)
73-73: ****The
honodependency is actively used throughout the codebase for backend API routing and middleware. Search results show direct imports and usage inapps/meteor/app/api/server/router.ts,apps/meteor/app/integrations/server/api/api.ts, and multiple middleware files. It is a legitimate backend infrastructure dependency, not an accidental addition.Likely an incorrect or invalid review comment.
| input.addEventListener('input', persist); | ||
| input.addEventListener('input', (event: Event) => { | ||
| resolveComposerBox(event, parseOptions); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store the input handler so release actually detaches it
We add the parser handler with an inline arrow function:
input.addEventListener('input', (event) => resolveComposerBox(event, parseOptions));but release removes a different function instance, so the listener survives every time you recreate the composer. That means duplicate parsing, stale references, and unnecessary work.
Keep the handler in a variable and remove the same reference:
- input.addEventListener('input', (event: Event) => {
- resolveComposerBox(event, parseOptions);
- });
+ const handleComposerParse = (event: Event) => {
+ resolveComposerBox(event, parseOptions);
+ };
+ input.addEventListener('input', handleComposerParse);
…
- input.removeEventListener('input', (event: Event) => {
- resolveComposerBox(event, parseOptions);
- });
+ input.removeEventListener('input', handleComposerParse);This stops the leak and ensures release() leaves the DOM clean.
Also applies to: 224-226
🤖 Prompt for AI Agents
In apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts
around lines 50-53, the parser input listener is added as an inline arrow
function so release() cannot remove it; capture the handler in a const (e.g.
const parseHandler = (event: Event) => resolveComposerBox(event, parseOptions)),
use that variable when calling input.addEventListener('input', parseHandler),
and call input.removeEventListener('input', parseHandler) in release(); apply
the same change to the other occurrence around lines 224-226 so the exact same
function reference is removed.
| const { selectionStart, selectionEnd } = getSelectionRange(input); | ||
| const textAreaTxt = input.innerHTML; | ||
|
|
||
| if (typeof selection === 'function') { | ||
| selection = selection({ start: selectionStart, end: selectionEnd }); | ||
| } | ||
|
|
||
| if (selection) { | ||
| if (!document.execCommand?.('insertText', false, text)) { | ||
| input.innerHTML = escapeHTML(textAreaTxt.substring(0, selectionStart) + text + textAreaTxt.substring(selectionStart)); | ||
| !skipFocus && focus(); | ||
| } | ||
| setSelectionRange(input, selection.start ?? 0, selection.end ?? text.length); | ||
| } | ||
|
|
||
| if (!selection) { | ||
| input.innerHTML = escapeHTML(text); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix non-execCommand fallback and stop double-escaping stored HTML
The manual branch in setText is splicing input.innerHTML with selectionStart values that come from plain-text offsets, so the selected range is never removed (substring(selectionStart) ignores selectionEnd). At the same time we wrap the whole string with escapeHTML, which re-escapes any existing markup. The first time execCommand is unavailable you'll duplicate the selected text and strip all formatting. It also breaks draft persistence: we store input.innerHTML, but on load setText escapes it and the user sees raw <strong> tags.
We need to rework this block to operate on text content (or a DOM range) and only escape the newly inserted text, while leaving previously sanitised HTML intact. For example:
- const { selectionStart, selectionEnd } = getSelectionRange(input);
- const textAreaTxt = input.innerHTML;
+ const { selectionStart, selectionEnd } = getSelectionRange(input);
+ const currentText = input.innerText;
…
- if (!document.execCommand?.('insertText', false, text)) {
- input.innerHTML = escapeHTML(textAreaTxt.substring(0, selectionStart) + text + textAreaTxt.substring(selectionStart));
- !skipFocus && focus();
- }
+ if (!document.execCommand?.('insertText', false, text)) {
+ const nextText = currentText.slice(0, selectionStart) + text + currentText.slice(selectionEnd);
+ input.innerText = nextText;
+ !skipFocus && focus();
+ }
…
- if (!selection) {
- input.innerHTML = escapeHTML(text);
- }
+ if (!selection) {
+ input.innerText = text;
+ }That keeps the fallback consistent with the plaintext model and lets resolveComposerBox rebuild safe HTML without corrupting existing formatting.
🧰 Tools
🪛 ast-grep (0.39.7)
[warning] 79-79: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: input.innerHTML = escapeHTML(textAreaTxt.substring(0, selectionStart) + text + textAreaTxt.substring(selectionStart))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 86-86: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: input.innerHTML = escapeHTML(text)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 79-79: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: input.innerHTML = escapeHTML(textAreaTxt.substring(0, selectionStart) + text + textAreaTxt.substring(selectionStart))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 86-86: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: input.innerHTML = escapeHTML(text)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🤖 Prompt for AI Agents
In apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts
around lines 71 to 89, the non-execCommand fallback currently splices
input.innerHTML using plain-text offsets (ignoring selectionEnd) and re-escapes
the entire innerHTML, causing duplicated selection, loss of formatting and
double-escaping stored HTML; fix by reading and slicing the plaintext via
input.textContent (or compute a DOM Range) using both selectionStart and
selectionEnd, build the new content as: existing left HTML (unchanged) +
escaped(newText) + existing right HTML (unchanged) — do NOT re-escape the
existing HTML string — then assign input.innerHTML to that combined string (or
replace via Range.insertNode) and update the selection with correct start and
end offsets (use selection.start ?? selectionStart and selection.end ??
selectionStart + text.length); ensure focus/skipFocus behavior remains the same.
| case 'MENTION_USER': | ||
| const classes = | ||
| token.value.value === 'all' | ||
| ? 'rcx-box rcx-box--full rcx-message__highlight rcx-message__highlight--relevant' | ||
| : 'rcx-box rcx-box--full rcx-message__highlight rcx-message__highlight--critical rcx-message__highlight--clickable'; | ||
|
|
||
| return `<span class="${classes}">@${escapeHTML(token.value.value)}</span>`; | ||
|
|
||
| case 'MENTION_CHANNEL': | ||
| return `<span class="mention-channel">#${escapeHTML(token.value.value)}</span>`; | ||
|
|
||
| case 'BOLD': | ||
| return `*<b>${renderInline(token.value)}</b>*`; | ||
|
|
||
| case 'ITALIC': | ||
| return `_<i>${renderInline(token.value)}</i>_`; | ||
|
|
||
| case 'STRIKE': | ||
| return `~<del>${renderInline(token.value)}</del>~`; | ||
|
|
||
| case 'INLINE_CODE': | ||
| return `\`<code class="code-colors inline">${escapeHTML(token.value.value)}</code>\``; | ||
|
|
||
| case 'EMOJI': | ||
| if ('shortCode' in token) { | ||
| const emoji = getEmojiClassNameAndDataTitle(`:${escapeHTML(token.shortCode)}:`); | ||
|
|
||
| // Checking emojione to replace the shortCode with Unicode emoji | ||
| if (emoji.className?.includes('emojione')) { | ||
| return emoji.children ?? ''; | ||
| } | ||
|
|
||
| // Changing the shortcode to display rendition of the custom uploaded | ||
| if (emoji.image) { | ||
| const rawUrl = emoji.image.replace(/^url\(["']?/, '').replace(/["']?\)$/, ''); | ||
| return ( | ||
| `<span style="font-size:0.1px">` + | ||
| `<img src="${rawUrl}" style="width:1.5rem;height:1.5rem;vertical-align:middle"/>` + | ||
| `:${token.shortCode}:</span>` | ||
| ); | ||
| } | ||
| } else return escapeHTML(token.unicode || ''); | ||
|
|
||
| // case 'COLOR': | ||
| // return `<span style="color: ${escapeHTML(token.value.color)}">${renderInline(token.value.value)}</span>`; | ||
|
|
||
| // case 'IMAGE': | ||
| // return `<img src="${escapeHTML(token.value.src.value)}" alt="${escapeHTML(token.value.label.value as string)}" />`; | ||
|
|
||
| // case 'TIMESTAMP': | ||
| // return `<time data-timestamp="${escapeHTML(token.value.timestamp.toString())}">${escapeHTML( | ||
| // token.value.timestamp.toString(), | ||
| // )}</time>`; | ||
|
|
||
| // case 'INLINE_KATEX': | ||
| // return `<span class="katex">${escapeHTML(token.value)}</span>`; | ||
|
|
||
| // eslint-disable-next-line no-fallthrough | ||
| default: | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap switch declarations and ensure the emoji case returns explicitly
Biome is flagging case 'MENTION_USER' because const classes is declared without a block, and the EMOJI case can fall through to default when the shortcode path doesn't return. Beyond the lint failure, the fallthrough means we silently drop custom emoji that miss both branches.
Wrap both cases in their own blocks and return explicitly:
- case 'MENTION_USER':
- const classes =
- token.value.value === 'all'
- ? 'rcx-box rcx-box--full rcx-message__highlight rcx-message__highlight--relevant'
- : 'rcx-box rcx-box--full rcx-message__highlight rcx-message__highlight--critical rcx-message__highlight--clickable';
-
- return `<span class="${classes}">@${escapeHTML(token.value.value)}</span>`;
+ case 'MENTION_USER': {
+ const classes =
+ token.value.value === 'all'
+ ? 'rcx-box rcx-box--full rcx-message__highlight rcx-message__highlight--relevant'
+ : 'rcx-box rcx-box--full rcx-message__highlight rcx-message__highlight--critical rcx-message__highlight--clickable';
+
+ return `<span class="${classes}">@${escapeHTML(token.value.value)}</span>`;
+ }
…
- case 'EMOJI':
- if ('shortCode' in token) {
+ case 'EMOJI': {
+ if ('shortCode' in token) {
const emoji = getEmojiClassNameAndDataTitle(`:${escapeHTML(token.shortCode)}:`);
…
if (emoji.image) {
…
- } else return escapeHTML(token.unicode || '');
+ }
+
+ return escapeHTML(token.unicode || '');
+ }This satisfies the linter and guarantees we always return from the emoji branch.
🧰 Tools
🪛 Biome (2.1.2)
[error] 27-30: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 49-67: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
🤖 Prompt for AI Agents
In apps/meteor/app/ui-message/client/messageBox/messageParser.ts around lines 26
to 86, wrap the MENTION_USER and EMOJI switch cases in their own { } blocks so
declarations like const classes are block-scoped and cannot leak, and make every
control path in the EMOJI case explicitly return a string (e.g., ensure the
shortcode branch returns, the image branch returns, and the fallback path
returns escapeHTML(token.unicode || '') rather than falling through); this
satisfies the linter and guarantees the emoji case never falls through to
default.
| case 'BOLD': | ||
| return `*<b>${renderInline(token.value)}</b>*`; | ||
|
|
||
| case 'ITALIC': | ||
| return `_<i>${renderInline(token.value)}</i>_`; | ||
|
|
||
| case 'STRIKE': | ||
| return `~<del>${renderInline(token.value)}</del>~`; | ||
|
|
||
| case 'INLINE_CODE': | ||
| return `\`<code class="code-colors inline">${escapeHTML(token.value.value)}</code>\``; | ||
|
|
||
| case 'EMOJI': | ||
| if ('shortCode' in token) { | ||
| const emoji = getEmojiClassNameAndDataTitle(`:${escapeHTML(token.shortCode)}:`); | ||
|
|
||
| // Checking emojione to replace the shortCode with Unicode emoji | ||
| if (emoji.className?.includes('emojione')) { | ||
| return emoji.children ?? ''; | ||
| } | ||
|
|
||
| // Changing the shortcode to display rendition of the custom uploaded | ||
| if (emoji.image) { | ||
| const rawUrl = emoji.image.replace(/^url\(["']?/, '').replace(/["']?\)$/, ''); | ||
| return ( | ||
| `<span style="font-size:0.1px">` + | ||
| `<img src="${rawUrl}" style="width:1.5rem;height:1.5rem;vertical-align:middle"/>` + | ||
| `:${token.shortCode}:</span>` | ||
| ); | ||
| } | ||
| } else return escapeHTML(token.unicode || ''); | ||
|
|
||
| // case 'COLOR': | ||
| // return `<span style="color: ${escapeHTML(token.value.color)}">${renderInline(token.value.value)}</span>`; | ||
|
|
||
| // case 'IMAGE': | ||
| // return `<img src="${escapeHTML(token.value.src.value)}" alt="${escapeHTML(token.value.label.value as string)}" />`; | ||
|
|
||
| // case 'TIMESTAMP': | ||
| // return `<time data-timestamp="${escapeHTML(token.value.timestamp.toString())}">${escapeHTML( | ||
| // token.value.timestamp.toString(), | ||
| // )}</time>`; | ||
|
|
||
| // case 'INLINE_KATEX': | ||
| // return `<span class="katex">${escapeHTML(token.value)}</span>`; | ||
|
|
||
| // eslint-disable-next-line no-fallthrough | ||
| default: | ||
| return ''; | ||
| } | ||
| }) | ||
| .join(''); | ||
|
|
||
| export const parseAST = (tokens: MessageParser.Root): string => { | ||
| return tokens | ||
| .map((block) => { | ||
| switch (block.type) { | ||
| case 'BIG_EMOJI': | ||
| return `<p>${renderInline(block.value)}</p>`; | ||
|
|
||
| case 'PARAGRAPH': | ||
| return `${renderInline(block.value)}\n`; | ||
|
|
||
| case 'HEADING': | ||
| return `<h${block.level}>${'#'.repeat(block.level)} ${renderInline(block.value)}</h${block.level}>`; | ||
|
|
||
| case 'UNORDERED_LIST': | ||
| return `<ul>${block.value.map((item) => `<li>- ${renderInline(item.value)}</li>`).join('')}</ul>`; | ||
|
|
||
| case 'ORDERED_LIST': | ||
| return `<ol>${block.value.map((item) => `<li value="${item.number}">${item.number}. ${renderInline(item.value)}</li>`).join('')}</ol>`; | ||
|
|
||
| case 'TASKS': | ||
| return `<ul class="task-list">${block.value | ||
| .map((task) => `<li><input type="checkbox" disabled ${task.status ? 'checked' : ''}> ${renderInline(task.value)}</li>`) | ||
| .join('')}</ul>`; | ||
|
|
||
| case 'QUOTE': | ||
| return `<blockquote>${block.value.map((item) => `<div>\> ${renderInline(item.value)}</div>`).join('')}</blockquote>`; | ||
|
|
||
| case 'CODE': | ||
| return `<pre><p>\`\`\`${block.language}\n</p><code>${block.value.map((item) => `${item.value.value}`).join('\n')}</code><p>\`\`\`</p></pre>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Render semantic HTML instead of reintroducing markdown tokens
The renderer is wrapping formatted segments with the original markdown markers (*, _, ~, backticks, list prefixes, #, etc.). When resolveComposerBox writes this HTML back into the composer we end up showing literal markdown characters around the formatted text, and repeated parsing duplicates the markers. The rich-text experience breaks as soon as users type any formatted content or we rehydrate stored drafts.
Please emit only the semantic tags and let the browser handle presentation. For example:
- case 'BOLD':
- return `*<b>${renderInline(token.value)}</b>*`;
+ case 'BOLD':
+ return `<strong>${renderInline(token.value)}</strong>`;
…
- case 'ITALIC':
- return `_<i>${renderInline(token.value)}</i>_`;
+ case 'ITALIC':
+ return `<em>${renderInline(token.value)}</em>`;
…
- case 'INLINE_CODE':
- return `\`<code class="code-colors inline">${escapeHTML(token.value.value)}</code>\``;
+ case 'INLINE_CODE':
+ return `<code class="code-colors inline">${escapeHTML(token.value.value)}</code>`;
…
- case 'UNORDERED_LIST':
- return `<ul>${block.value.map((item) => `<li>- ${renderInline(item.value)}</li>`).join('')}</ul>`;
+ case 'UNORDERED_LIST':
+ return `<ul>${block.value.map((item) => `<li>${renderInline(item.value)}</li>`).join('')}</ul>`;
…
- case 'HEADING':
- return `<h${block.level}>${'#'.repeat(block.level)} ${renderInline(block.value)}</h${block.level}>`;
+ case 'HEADING':
+ return `<h${block.level}>${renderInline(block.value)}</h${block.level}>`;
…
- case 'QUOTE':
- return `<blockquote>${block.value.map((item) => `<div>\> ${renderInline(item.value)}</div>`).join('')}</blockquote>`;
+ case 'QUOTE':
+ return `<blockquote>${block.value.map((item) => `<p>${renderInline(item.value)}</p>`).join('')}</blockquote>`;
…
- case 'CODE':
- return `<pre><p>\`\`\`${block.language}\n</p><code>${block.value.map((item) => `${item.value.value}`).join('\n')}</code><p>\`\`\`</p></pre>`;
+ case 'CODE':
+ return `<pre><code>${block.value.map((item) => escapeHTML(item.value.value)).join('\n')}</code></pre>`;Apply the same treatment to the remaining cases (ordered/task lists, strike, etc.) so we only output valid HTML.
Also applies to: 94-118
🧰 Tools
🪛 Biome (2.1.2)
[error] 49-67: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
🤖 Prompt for AI Agents
In apps/meteor/app/ui-message/client/messageBox/messageParser.ts around lines 37
to 118, the renderer currently reintroduces markdown markers (e.g. "*", "_",
"~", backticks, list prefixes, "#", ">" etc.) into the output which causes
duplicated markers when re-parsed; change each case to emit only semantic HTML:
BOLD -> return <b>...</b> (no surrounding *), ITALIC -> <i>...</i> (no _),
STRIKE -> <del>...</del> (no ~), INLINE_CODE -> <code class="...">escaped
content</code> (no backticks), EMOJI -> output the emoji Unicode or an <img> tag
without wrapping with :shortcode: or extra span text, HEADING -> emit
<hN>content</hN> with no leading #, PARAGRAPH -> emit <p>content</p> (remove
trailing raw newline), UNORDERED_LIST/ORDERED_LIST/TASKS -> emit proper
<ul>/<ol>/<li> and for tasks include checkbox input but no "- " or "N. " text
prefixes, QUOTE -> emit <blockquote>...</blockquote> without leading "> "
strings, and CODE block -> emit a <pre><code class="language-...">escaped block
content</code></pre> (remove fenced ``` markers); update any concatenation logic
accordingly so the output is pure HTML and free of markdown tokens.
| const restoreLinks = (html: string, matches: string[]): string => { | ||
| return html.replace(/\[\[\[LINK_(\d+)\]\]\]/g, (_, i) => matches[parseInt(i, 10)] || ''); | ||
| }; | ||
|
|
||
| // Resolve the Composer after the user modifies text | ||
| export const resolveComposerBox = (event: Event, parseOptions: Options) => { | ||
| if (!event.isTrusted) return; | ||
|
|
||
| const target = event.target as HTMLDivElement; | ||
| const text = target.innerText; | ||
|
|
||
| // Get the position of the cursor after text modification | ||
| // This is so that after parsing and rendering inside the editor | ||
| // the cursor is restored to the correct position | ||
| const selection = getSelectionRange(target); | ||
| const { selectionStart, selectionEnd } = selection; | ||
|
|
||
| // Extract the URL and substitue with a safe template | ||
| const { output: safeText, matches } = protectLinks(text === '' ? '\n' : text); | ||
|
|
||
| // Parse the safetext | ||
| const ast = parseMessage(safeText, parseOptions); | ||
|
|
||
| // Parse the AST | ||
| const html = parseAST(ast); | ||
| console.log(ast); | ||
|
|
||
| // Restore the substituted links | ||
| let finalHtml = restoreLinks(html, matches); | ||
|
|
||
| // Prevent newline explosion after every end of heading updation | ||
| finalHtml = finalHtml | ||
| .replace(/<\/h1><br\s*\/?>/gi, '</h1>') | ||
| .replace(/<\/h2><br\s*\/?>/gi, '</h2>') | ||
| .replace(/<\/h3><br\s*\/?>/gi, '</h3>') | ||
| .replace(/<\/h4><br\s*\/?>/gi, '</h4>'); | ||
|
|
||
| // Rendering pipeline | ||
| target.innerHTML = finalHtml; // This works but it destroys the undo history |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocker: restoring raw link text reintroduces XSS
protectLinks stores untrusted user input and restoreLinks splices it straight back into finalHtml just before target.innerHTML = finalHtml. A payload such as https://example.com"><img src=x onerror=alert(1)> survives the placeholder swap and executes once restored. We must HTML-escape the restored value (same helper we already use in messageParser.ts) before re-inserting it.
Apply this diff:
-import { parse, type Options, type Root } from '@rocket.chat/message-parser';
+import { parse, type Options, type Root } from '@rocket.chat/message-parser';
+import { escapeHTML } from '@rocket.chat/string-helpers';
@@
-const restoreLinks = (html: string, matches: string[]): string => {
- return html.replace(/\[\[\[LINK_(\d+)\]\]\]/g, (_, i) => matches[parseInt(i, 10)] || '');
+const restoreLinks = (html: string, matches: string[]): string => {
+ return html.replace(/\[\[\[LINK_(\d+)\]\]\]/g, (_, i) => escapeHTML(matches[parseInt(i, 10)] || ''));
};Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ast-grep (0.39.7)
[warning] 104-104: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = finalHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 104-104: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = finalHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🤖 Prompt for AI Agents
In apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts around
lines 67 to 105, restoreLinks currently reinserts raw user-provided link
fragments (matches) into finalHtml which reintroduces XSS; update the flow to
HTML-escape each match before putting it back into the template. Import or reuse
the existing escapeHtml (or equivalent) helper from messageParser.ts and change
restoreLinks (or the replacement step) to call that helper on
matches[parseInt(i, 10)] (falling back to empty string) so only escaped text is
injected into finalHtml prior to assigning target.innerHTML.
| // We store the last known cursor position for each contenteditable div using a WeakMap keyed by the element. | ||
| // This allows us to restore the caret position when the user refocuses the editor, | ||
| // preventing the cursor from jumping to the start and improving typing experience in multiple composer instances. | ||
| const cursorMap = new WeakMap< | ||
| HTMLElement, | ||
| { | ||
| selectionStart: number; | ||
| selectionEnd: number; | ||
| } | ||
| >(); | ||
|
|
||
| // This state will update every time the input is updated | ||
| const [, setMdLines] = useState<string[]>([]); | ||
|
|
||
| const setLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => { | ||
| const node = e.currentTarget as HTMLDivElement; | ||
| cursorMap.set(node, getSelectionRange(node)); | ||
| console.log('Saved cursor position for:', node); | ||
| }; | ||
|
|
||
| const getLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => { | ||
| const node = e.currentTarget as HTMLDivElement; | ||
| const savedPosition = cursorMap.get(node); | ||
| if (savedPosition === undefined) { | ||
| console.log('There is no savedPosition for current node'); | ||
| return; | ||
| } // no saved cursor position | ||
|
|
||
| // Retrieve the value onFocus | ||
| setSelectionRange(node, savedPosition.selectionStart, savedPosition.selectionEnd); | ||
|
|
||
| console.log('Restored cursor position for:', node); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Major: cursor position cache resets every render
Because cursorMap is instantiated inside the component body, every render wipes the map. Any state update (typing, formatting, popup changes, etc.) happens before focus returns, so getLastCursorPosition almost always finds undefined and the caret jumps back to the start. Persist the cache in a ref instead of recreating it per render.
Apply this diff:
- const cursorMap = new WeakMap<
+ const cursorMapRef = useRef(
+ new WeakMap<
HTMLElement,
{
selectionStart: number;
selectionEnd: number;
}
- >();
+ >(),
+ );
@@
const setLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => {
const node = e.currentTarget as HTMLDivElement;
- cursorMap.set(node, getSelectionRange(node));
- console.log('Saved cursor position for:', node);
+ cursorMapRef.current.set(node, getSelectionRange(node));
};
const getLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => {
const node = e.currentTarget as HTMLDivElement;
- const savedPosition = cursorMap.get(node);
+ const savedPosition = cursorMapRef.current.get(node);
if (savedPosition === undefined) {
- console.log('There is no savedPosition for current node');
return;
} // no saved cursor position
// Retrieve the value onFocus
setSelectionRange(node, savedPosition.selectionStart, savedPosition.selectionEnd);
-
- console.log('Restored cursor position for:', node);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // We store the last known cursor position for each contenteditable div using a WeakMap keyed by the element. | |
| // This allows us to restore the caret position when the user refocuses the editor, | |
| // preventing the cursor from jumping to the start and improving typing experience in multiple composer instances. | |
| const cursorMap = new WeakMap< | |
| HTMLElement, | |
| { | |
| selectionStart: number; | |
| selectionEnd: number; | |
| } | |
| >(); | |
| // This state will update every time the input is updated | |
| const [, setMdLines] = useState<string[]>([]); | |
| const setLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => { | |
| const node = e.currentTarget as HTMLDivElement; | |
| cursorMap.set(node, getSelectionRange(node)); | |
| console.log('Saved cursor position for:', node); | |
| }; | |
| const getLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => { | |
| const node = e.currentTarget as HTMLDivElement; | |
| const savedPosition = cursorMap.get(node); | |
| if (savedPosition === undefined) { | |
| console.log('There is no savedPosition for current node'); | |
| return; | |
| } // no saved cursor position | |
| // Retrieve the value onFocus | |
| setSelectionRange(node, savedPosition.selectionStart, savedPosition.selectionEnd); | |
| console.log('Restored cursor position for:', node); | |
| }; | |
| const cursorMapRef = useRef( | |
| new WeakMap< | |
| HTMLElement, | |
| { | |
| selectionStart: number; | |
| selectionEnd: number; | |
| } | |
| >(), | |
| ); | |
| // This state will update every time the input is updated | |
| const [, setMdLines] = useState<string[]>([]); | |
| const setLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => { | |
| const node = e.currentTarget as HTMLDivElement; | |
| cursorMapRef.current.set(node, getSelectionRange(node)); | |
| }; | |
| const getLastCursorPosition = (e: React.FocusEvent<HTMLElement>) => { | |
| const node = e.currentTarget as HTMLDivElement; | |
| const savedPosition = cursorMapRef.current.get(node); | |
| if (savedPosition === undefined) { | |
| return; | |
| } // no saved cursor position | |
| // Retrieve the value onFocus | |
| setSelectionRange(node, savedPosition.selectionStart, savedPosition.selectionEnd); | |
| }; |
🤖 Prompt for AI Agents
In apps/meteor/client/views/room/composer/messageBox/RichTextMessageBox.tsx
around lines 161 to 193, the WeakMap cursorMap is created inside the component
body so it gets recreated on every render causing the saved cursor positions to
be lost; change it to a persistent ref by creating it with useRef(new
WeakMap<HTMLElement, { selectionStart: number; selectionEnd: number }>()), then
update setLastCursorPosition and getLastCursorPosition to read/write
cursorMap.current (and handle undefined current defensively) so the map survives
re-renders and caret positions are restored correctly.
| RichTextComposer.args = { | ||
| // Define the props (args) you want to control | ||
| placeholder: 'Type a message...', | ||
| hidePlaceholder: false, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Storybook arg casing
The story registers hidePlaceholder, but the component expects hideplaceholder. As written, the control never reaches the component. Update the arg key to match the prop.
Apply this diff:
RichTextComposer.args = {
// Define the props (args) you want to control
placeholder: 'Type a message...',
- hidePlaceholder: false,
+ hideplaceholder: false,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RichTextComposer.args = { | |
| // Define the props (args) you want to control | |
| placeholder: 'Type a message...', | |
| hidePlaceholder: false, | |
| }; | |
| RichTextComposer.args = { | |
| // Define the props (args) you want to control | |
| placeholder: 'Type a message...', | |
| hideplaceholder: false, | |
| }; |
🤖 Prompt for AI Agents
In packages/ui-composer/src/MessageComposer/MessageComposer.stories.tsx around
lines 72 to 76, the Storybook args use hidePlaceholder but the component prop is
hideplaceholder, so the control never reaches the component; change the arg key
from hidePlaceholder to hideplaceholder (keep the value false) so the story
passes the correct prop name to the component.
c8dd16d to
800321b
Compare
GSoC 2025: AI-Enhanced Message Composer 🚀
This pull request introduces the completed front-end UI for the AI-Enhanced Message Composer, a project developed as part of the Google Summer of Code 2025. This feature provides users with on-the-fly, AI-powered tools to refine and transform their messages directly within the Rocket.Chat composer.
My primary focus was on the frontend, creating a seamless, intuitive, and componentized workflow for AI interactions within the rocket.chat message composer.
🤔 The Problem
Have you ever found yourself hesitating before sending a message, asking questions like:
The typical solution involves a cumbersome, multi-step process:
This workflow is disruptive and inefficient, pulling the user out of the application and breaking the flow of communication.
✨ Our Solution
AI-Enhanced Message Composer, a smart feature integrated directly into the Rocket.Chat composer.The goal is to make AI interaction intuitive and effortless, allowing users to transform messages on-the-fly with just a couple of clicks.
🛠️ Key Features & Implementation Details
The entire feature is powered by a single custom React hook,
useAIEnhancement, which manages the state, user interactions, and DOM manipulations.1. The
useAIEnhancementHook LogicThe hook is the brain of the operation. It listens for a
mouseupevent on the message composerdiv. When a text selection is detected within the composer, it captures the cursor's coordinates (event.clientX,event.clientY) and updates its state to render a popup menu at that exact position. The core logic for processing user actions is handled by therunAIActionfunction, which is memoized withuseCallbackfor performance. This function takes the action type (e.g., 'summary', 'emoji') and orchestrates the entire DOM manipulation workflow.2. The DOM Interaction & UX Flow
Instead of just replacing text, the hook provides a rich user experience through direct DOM manipulation:
<span class="ai-enhancement-transform">. The original text is saved in adataset.originalTextattribute on this span, ensuring the user can always revert.ai-enhancement-suggestion, and the AI-generated text is rendered with a "typing" effect.onclickhandlers for these buttons either replace the span with the final transformed text (Accept) or with the original text stored in thedataset(Reject), giving the user complete control over the final output.Development Timeline and Collaboration.
This project was developed as an integral part of the Rocket.Chat main repository. My mentor, Martin Schöeler, created a dedicated branch, feat/real-time-composer, and my task was to develop this feature and submit it as a PR to that branch. The AI-Enhanced Composer is deeply connected to Ishan Mitra's "Real-Time Composer" project. Consequently, my code was built directly upon his work, and we collaborated closely from the start.
Project Milestones
Midterm: I created a comprehensive Storybook for my components. This served as a live design system, depicting how the entire feature would look and feel.
Final: I focused on implementing a fully functional React component and integrating it into the application.
StoryBook Demo Video:
Take.5.mp4
Project demo Video:
projcet.demo.mp4
📖 How It Works: A Step-by-Step Flow
The user experience is designed to be simple and intuitive.
Live Project Branch: https://pr-36736.qa.rocket.chat/
🙏 Acknowledgements
A huge thank you to my incredible mentors for their guidance and support throughout this project:
Thank you to the entire Rocket.Chat team and Google for this amazing opportunity.
BLOG LINK : https://enhance-your-messages-lg3e0bq.gamma.site/
Demo Day Video: https://www.youtube.com/watch?v=8QFLKasLf20
Summary by CodeRabbit
Release Notes