feat: Add markdown file viewer with GitHub-flavored rendering#3277
feat: Add markdown file viewer with GitHub-flavored rendering#3277jeanfbrito merged 4 commits intodevfrom
Conversation
Extend the document viewer to support markdown files alongside PDFs. When a .md file is clicked in a server, it is intercepted at the download level and rendered in an overlay using marked with GFM support, syntax highlighting via highlight.js, and DOMPurify for XSS sanitization. - Add MarkdownContent component with GitHub-style CSS and theme support - Extract PdfContent into its own component for cleaner separation - Thread documentViewerFormat through Redux state and IPC - Intercept .md downloads via session will-download handler - Fetch markdown content via main process IPC using server session auth - Open links in external browser, contain scroll within viewer - Add document viewer translation keys
Add documentViewer.title.pdf and documentViewer.title.markdown translation keys to all 21 non-English locale files.
WalkthroughAdds a document viewer with PDF and Markdown support: new runtime deps for Markdown rendering, IPC to fetch remote content via persisted Electron session, Redux/server state updates for per-server format, i18n entries across locales, and new React components for Markdown and PDF rendering. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as ServerPane / DocumentViewer
participant MarkdownComp as MarkdownContent
participant IPC as Electron IPC
participant Session as Electron Session (persist:${serverUrl})
participant Origin as Remote Server
participant Parser as marked + highlight.js
participant Sanitizer as DOMPurify
participant Shell as electron.shell
User->>UI: open document (format=markdown)
UI->>MarkdownComp: render(url, partition)
MarkdownComp->>IPC: invoke document-viewer/fetch-content(url, serverUrl)
IPC->>Session: perform fetch using persisted partition
Session->>Origin: HTTP GET url
Origin-->>Session: 200 OK + body
Session-->>IPC: return text
IPC-->>MarkdownComp: deliver text
MarkdownComp->>Parser: parse + highlight
Parser-->>MarkdownComp: HTML
MarkdownComp->>Sanitizer: sanitize HTML
Sanitizer-->>MarkdownComp: safe HTML
MarkdownComp->>UI: render sanitized HTML
User->>MarkdownComp: click external link
MarkdownComp->>Shell: openExternal(link)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/servers/common.ts (1)
32-32: NarrowdocumentViewerFormatto a literal union for safer state typing.Using
stringpermits invalid formats to flow through reducers/UI. Restricting this field improves compile-time safety and documents allowed values.♻️ Proposed type narrowing
- documentViewerFormat?: string; + documentViewerFormat?: 'pdf' | 'markdown' | '';As per coding guidelines, “Use TypeScript strict mode enabled in TypeScript configuration” and “Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/servers/common.ts` at line 32, documentViewerFormat is currently typed as string which allows invalid values; replace it with a literal union type (e.g. declare type DocumentViewerFormat = 'pdf' | 'text' | 'image' | 'none' or your app's permitted format strings) and change the property to documentViewerFormat?: DocumentViewerFormat; update any reducers, action creators, and UI usages (where documentViewerFormat is referenced) to use the new DocumentViewerFormat type and, if needed, add a runtime guard or exhaustive switch to handle unexpected values.src/ui/main/serverView/index.ts (1)
175-189: Consider case-insensitive extension matching for.mdfiles.The current check
item.getFilename().endsWith('.md')is case-sensitive and won't intercept files with uppercase extensions likeREADME.MDornotes.Md.♻️ Proposed fix for case-insensitive matching
// Intercept markdown file downloads and open in document viewer webviewSession.on('will-download', (_event, item) => { - if (item.getFilename().endsWith('.md')) { + if (item.getFilename().toLowerCase().endsWith('.md')) { const downloadUrl = item.getURL(); item.cancel(); dispatch({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/main/serverView/index.ts` around lines 175 - 189, The filename extension check in the webviewSession.on('will-download', ...) handler uses item.getFilename().endsWith('.md') which is case-sensitive; update the condition to a case-insensitive test (e.g. normalize the filename with item.getFilename().toLowerCase().endsWith('.md') or use a case-insensitive regex) so files like README.MD or notes.Md are intercepted, leaving the rest of the dispatch logic (SERVER_DOCUMENT_VIEWER_OPEN_URL, payload keys server/documentUrl/documentFormat) unchanged.src/ui/components/ServersView/PdfContent.tsx (1)
19-26: Use one cancellable reload timer per URL change.This effect re-runs on every
documentUrlupdate, so theabout:blankhop schedules duplicate timers and a quick A→B switch can briefly reload A after B was requested. Key the effect tourlonly and clear the timeout in cleanup.♻️ Suggested change
- useEffect(() => { - if (documentUrl !== url && url !== '') { - setDocumentUrl('about:blank'); - setTimeout(() => { - setDocumentUrl(url); - }, 100); - } - }, [url, documentUrl]); + useEffect(() => { + if (url === '') { + setDocumentUrl(''); + return; + } + + setDocumentUrl('about:blank'); + const timer = window.setTimeout(() => { + setDocumentUrl(url); + }, 100); + + return () => window.clearTimeout(timer); + }, [url]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/ServersView/PdfContent.tsx` around lines 19 - 26, The current useEffect depends on documentUrl so it re-runs when we set 'about:blank' and can schedule duplicate timers; update the effect on PdfContent to depend only on 'url', create a single timeout handle (e.g. const timer = window.setTimeout(...)) when url !== '' and url !== documentUrl, and in the effect's cleanup clearTimeout(timer) to cancel the scheduled reload; keep using setDocumentUrl('about:blank') then setDocumentUrl(url) inside the timer but ensure the effect signature references useEffect, url, documentUrl, setDocumentUrl and returns a cleanup that clears the timer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/documentViewer/ipc.ts`:
- Around line 38-49: The IPC handler 'document-viewer/fetch-content' currently
accepts untrusted url and serverUrl; validate both by parsing them with new
URL(...) and ensure the url.protocol is "http:" or "https:" and that new
URL(url).origin === new URL(serverUrl).origin before creating the
partition/session and calling session.fromPartition/ ses.fetch; if parsing fails
or checks fail, throw an Error to reject the request. Use the existing handler
name ('document-viewer/fetch-content') and the partition logic (partition =
`persist:${serverUrl}` / session.fromPartition) when locating where to add these
checks.
In `@src/ui/components/ServersView/DocumentViewer.tsx`:
- Around line 40-41: The IconButton near the top is icon-only and lacks an
accessible name and the raw <h2> breaks Fuselage theming; update the IconButton
(the one using icon='arrow-back' and onClick={closeDocumentViewer}) to include
an i18n-backed aria-label (use the app i18n mechanism to provide the label text)
and replace the raw h2 {title} with the appropriate Fuselage heading/text
primitive (e.g., Heading or Text from `@rocket.chat/fuselage`) so the overlay
follows app typography and theme.
In `@src/ui/components/ServersView/MarkdownContent.tsx`:
- Around line 82-83: The current call to
ipcRenderer.invoke('document-viewer/fetch-content', url, serverUrl) in
MarkdownContent.tsx returns only the markdown HTML, but embedded assets (e.g.,
<img> src, CSS/JS links) are later loaded by the root window and thus cannot
access partition-backed/authenticated server resources; update the flow so
embedded URLs are rewritten or proxied through the same partition-backed fetch:
either (A) change the fetch-content handler to parse the generated HTML and
rewrite all resource URLs (img/src, source/src, link/href, script/src, etc.) to
point to a partition-proxy endpoint (or include a tokenized URL) before
returning to the renderer, or (B) intercept in MarkdownContent (or the marked
renderer used there) to transform relative URLs into absolute serverUrl-based
URLs and replace them with ipc/proxy routes that call
document-viewer/fetch-content for each asset; reference the ipcRenderer.invoke
call and the MarkdownContent component (and the adjacent resource handling at
lines ~139-147) to locate where to implement the URL rewrite/proxy.
- Around line 61-66: In handleClick in MarkdownContent.tsx, stop using
anchor.href (which resolves against the app window) and instead read the raw
href via anchor.getAttribute('href'); if rawHref is null return; if rawHref
starts with '#' treat as in-page navigation and do not call shell.openExternal;
otherwise resolve the link using new URL(rawHref, url) (where url is the
markdown document URL in scope) and only call
shell.openExternal(resolved.toString()) after checking the resolved.protocol
against an allowlist (e.g., http:, https:, mailto:). Ensure you still call
e.preventDefault() for links you handle.
---
Nitpick comments:
In `@src/servers/common.ts`:
- Line 32: documentViewerFormat is currently typed as string which allows
invalid values; replace it with a literal union type (e.g. declare type
DocumentViewerFormat = 'pdf' | 'text' | 'image' | 'none' or your app's permitted
format strings) and change the property to documentViewerFormat?:
DocumentViewerFormat; update any reducers, action creators, and UI usages (where
documentViewerFormat is referenced) to use the new DocumentViewerFormat type
and, if needed, add a runtime guard or exhaustive switch to handle unexpected
values.
In `@src/ui/components/ServersView/PdfContent.tsx`:
- Around line 19-26: The current useEffect depends on documentUrl so it re-runs
when we set 'about:blank' and can schedule duplicate timers; update the effect
on PdfContent to depend only on 'url', create a single timeout handle (e.g.
const timer = window.setTimeout(...)) when url !== '' and url !== documentUrl,
and in the effect's cleanup clearTimeout(timer) to cancel the scheduled reload;
keep using setDocumentUrl('about:blank') then setDocumentUrl(url) inside the
timer but ensure the effect signature references useEffect, url, documentUrl,
setDocumentUrl and returns a cleanup that clears the timer.
In `@src/ui/main/serverView/index.ts`:
- Around line 175-189: The filename extension check in the
webviewSession.on('will-download', ...) handler uses
item.getFilename().endsWith('.md') which is case-sensitive; update the condition
to a case-insensitive test (e.g. normalize the filename with
item.getFilename().toLowerCase().endsWith('.md') or use a case-insensitive
regex) so files like README.MD or notes.Md are intercepted, leaving the rest of
the dispatch logic (SERVER_DOCUMENT_VIEWER_OPEN_URL, payload keys
server/documentUrl/documentFormat) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 560748e9-8eb7-46ca-9eb1-753373ec2adf
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
package.jsonrollup.config.mjssrc/documentViewer/ipc.tssrc/i18n/ar.i18n.jsonsrc/i18n/de-DE.i18n.jsonsrc/i18n/en.i18n.jsonsrc/i18n/es.i18n.jsonsrc/i18n/fi.i18n.jsonsrc/i18n/fr.i18n.jsonsrc/i18n/hu.i18n.jsonsrc/i18n/it-IT.i18n.jsonsrc/i18n/ja.i18n.jsonsrc/i18n/nb-NO.i18n.jsonsrc/i18n/nn.i18n.jsonsrc/i18n/no.i18n.jsonsrc/i18n/pl.i18n.jsonsrc/i18n/pt-BR.i18n.jsonsrc/i18n/ru.i18n.jsonsrc/i18n/se.i18n.jsonsrc/i18n/sv.i18n.jsonsrc/i18n/tr-TR.i18n.jsonsrc/i18n/uk-UA.i18n.jsonsrc/i18n/zh-CN.i18n.jsonsrc/i18n/zh-TW.i18n.jsonsrc/i18n/zh.i18n.jsonsrc/ipc/channels.tssrc/public/main.csssrc/servers/actions.tssrc/servers/common.tssrc/servers/reducers.tssrc/ui/components/ServersView/DocumentViewer.tsxsrc/ui/components/ServersView/MarkdownContent.tsxsrc/ui/components/ServersView/PdfContent.tsxsrc/ui/components/ServersView/ServerPane.tsxsrc/ui/components/ServersView/index.tsxsrc/ui/main/serverView/index.ts
📜 Review details
⏰ 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: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/ui/components/ServersView/index.tsxsrc/documentViewer/ipc.tssrc/servers/common.tssrc/servers/actions.tssrc/servers/reducers.tssrc/ui/components/ServersView/ServerPane.tsxsrc/ui/main/serverView/index.tssrc/ui/components/ServersView/PdfContent.tsxsrc/ui/components/ServersView/MarkdownContent.tsxsrc/ipc/channels.tssrc/ui/components/ServersView/DocumentViewer.tsx
🧠 Learnings (3)
📚 Learning: 2026-02-04T19:29:54.650Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T19:29:54.650Z
Learning: Applies to **/*.{ts,tsx} : Use Fuselage components from `rocket.chat/fuselage` for all UI work and only create custom components when Fuselage doesn't provide what's needed
Applied to files:
src/public/main.css
📚 Learning: 2026-03-11T06:38:40.426Z
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
Applied to files:
src/public/main.cssrollup.config.mjs
📚 Learning: 2026-02-04T19:29:54.650Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T19:29:54.650Z
Learning: Applies to **/*.{ts,tsx} : Check `Theme.d.ts` for valid color tokens when using Fuselage components
Applied to files:
src/public/main.css
🪛 ast-grep (0.41.1)
src/ui/components/ServersView/MarkdownContent.tsx
[warning] 145-145: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (31)
src/public/main.css (1)
2-2: Scoped markdown styling import looks good.The added stylesheet is appropriate for the new markdown viewer and aligns with the
.markdown-bodyscoping used by the renderer.src/i18n/zh.i18n.json (1)
2-7: Localization block is correctly structured and consistent with the new viewer keys.src/i18n/fi.i18n.json (1)
148-153: Finnish document viewer titles are added cleanly and with the expected key hierarchy.src/i18n/en.i18n.json (1)
176-181: English titles for PDF/Markdown viewer are correctly introduced and ready for consumption by the header switch logic.src/i18n/tr-TR.i18n.json (1)
106-111: Turkish document viewer localization keys are correctly added and consistent with the cross-locale contract.src/i18n/it-IT.i18n.json (1)
2-7: Italian viewer title keys are correctly added with the expected hierarchy.src/i18n/ar.i18n.json (1)
2-7: Arabic document viewer titles are well-formed and consistent with the new i18n namespace.rollup.config.mjs (1)
190-199: Good externalization adjustment for markdown renderer dependencies.Line 190-Line 199 correctly keeps
marked,marked-highlight,highlight.js, anddompurifyout ofexternalfor thesrc/rootWindow.tsbundle, which aligns with the new markdown rendering path.src/i18n/sv.i18n.json (1)
169-174: Locale key addition looks consistent.Line 169-Line 174 adds the expected
documentViewer.titlekeys with valid Swedish labels and consistent structure.src/i18n/fr.i18n.json (1)
148-153: French document viewer titles are correctly introduced.Line 148-Line 153 follows the same
documentViewer.title.{pdf,markdown}schema used by the feature.src/i18n/de-DE.i18n.json (1)
141-146: German locale update is clean and aligned.Line 141-Line 146 adds the required viewer title keys without structural issues.
src/i18n/ja.i18n.json (1)
106-111: Japanese document viewer key additions look good.Line 106-Line 111 correctly introduces the new
documentViewer.titleentries.src/i18n/ru.i18n.json (1)
142-147: Russian locale keys are correctly added.Line 142-Line 147 matches the expected i18n key structure for PDF/Markdown viewer titles.
src/i18n/es.i18n.json (1)
160-165: Spanish document viewer labels are correctly structured.Line 160-Line 165 introduces the required keys in the expected namespace.
src/i18n/pt-BR.i18n.json (1)
160-165: Portuguese (Brazil) viewer titles are correctly added.Line 160-Line 165 follows the shared
documentViewer.titleschema and looks ready.src/i18n/zh-TW.i18n.json (1)
85-90: Localization block is correctly added for document viewer titles.Line 85 introduces the new
documentViewer.titlenamespace with bothmarkdown, matching the new viewer modes cleanly.src/ui/components/ServersView/index.tsx (1)
21-21: Viewer format prop is correctly threaded through the view layer.Line 21 correctly forwards
documentViewerFormattoServerPane, enabling format-specific rendering downstream.src/i18n/no.i18n.json (1)
169-174: Document viewer localization keys are correctly introduced.Line 169 adds the expected
documentViewer.titleentries for both formats, consistent with the new UI behavior.src/i18n/nb-NO.i18n.json (1)
2-7: Locale entries for viewer modes are correctly added.Line 2 adds a properly structured
documentViewer.titleblock with both required keys.src/i18n/zh-CN.i18n.json (1)
107-112: New document viewer translation namespace looks correct.Line 107 introduces
documentViewer.titlewith bothmarkdown, matching feature requirements.src/i18n/se.i18n.json (1)
2-7: Document viewer i18n keys are correctly added for this locale.Line 2 adds the expected
documentViewer.titlemapping for both file formats.src/i18n/hu.i18n.json (1)
169-174: Hungarian localization entries for document viewer are correctly structured.Line 169 adds
documentViewer.title.pdfanddocumentViewer.title.markdownin the expected namespace.src/i18n/pl.i18n.json (1)
118-123: Polish document viewer translations are correctly added.Line 118 introduces the new
documentViewer.titlekeys for both supported viewer formats.src/i18n/uk-UA.i18n.json (1)
88-93: LGTM!The Ukrainian translations follow the existing structure and appear grammatically correct.
src/ipc/channels.ts (1)
82-82: LGTM!The IPC channel type definition follows the established pattern and provides proper typing for the markdown content fetching.
src/i18n/nn.i18n.json (1)
2-7: LGTM!The Norwegian Nynorsk translations are consistent with the locale structure.
src/servers/actions.ts (1)
17-21: LGTM!The optional
documentFormatfield with reducer-side defaulting via nullish coalescing is a clean design that maintains backward compatibility.src/ui/components/ServersView/ServerPane.tsx (1)
27-27: LGTM!The
documentViewerFormatprop is properly threaded through the component with correct typing and passed toDocumentViewer. The close action correctly resets the format to an empty string.Also applies to: 39-39, 189-189, 217-217
src/servers/reducers.ts (2)
270-277: LGTM!The reducer correctly extracts and persists
documentFormatwith appropriate defaulting via nullish coalescing. The empty string default integrates well with theformat === 'markdown'check inDocumentViewer.
245-252: Good initialization of viewer state on app settings load.Resetting both
documentViewerOpenUrlanddocumentViewerFormatto empty strings when the app loads ensures a clean state and prevents stale viewer state from persisting across sessions.package.json (1)
69-80: Markdown rendering dependencies look solid.The implementation is well-structured for the feature:
markedfor Markdown parsing with GFM support (lines 30, 39)marked-highlight+highlight.jsfor syntax highlighting integration (lines 31-37)dompurifyfor XSS sanitization—properly configured withhtmlprofile (lines 90-92)github-markdown-cssfor styling consistency (line 143)The code correctly chains parsing through sanitization before rendering, following security best practices. Versions are current and secure; dompurify 3.3.3 is the latest with recent CVE fixes included.
- Validate URL protocol and origin in fetch-content IPC handler
- Use getAttribute('href') for links, resolve relative URLs against
document base, and enforce protocol allowlist (http, https, mailto)
- Skip in-page anchor links (#) instead of opening externally
- Add aria-label to back button for screen readers
- Replace raw <h2> with Fuselage Box fontScale for theme consistency
- Add documentViewer.back translation key to all 22 languages
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/ui/components/ServersView/MarkdownContent.tsx (1)
97-108:⚠️ Potential issue | 🟠 MajorEmbedded assets still bypass the authenticated fetch path.
Only the markdown document goes through
document-viewer/fetch-content. Any<img>,<source>, etc. emitted bymarkedare then resolved by the root window, so relative URLs resolve against the desktop app page and protected server assets won't load with the server partition. Rewrite or proxy resource URLs through the same IPC/session flow before injecting the HTML.Also applies to: 154-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/ServersView/MarkdownContent.tsx` around lines 97 - 108, The generated HTML from marked.parse is injecting asset tags (<img>, <source>, etc.) that bypass the authenticated fetch path; update the pipeline that currently calls ipcRenderer.invoke('document-viewer/fetch-content', url, serverUrl) → marked.parse(...) → DOMPurify.sanitize(...) → setHtmlContent(...) to rewrite or proxy all resource URLs before setHtmlContent. Specifically, after marked.parse (and before DOMPurify.sanitize) walk the parsed HTML (or use a DOM parser) to find src/srcset/href attributes on img, source, video, audio, link, and any inline CSS urls, and replace each resource URL with a proxied URL that routes through the same IPC/session handler (e.g., an endpoint the main process serves that calls the same 'document-viewer/fetch-content' flow), so assets load via the server partition; apply the same fix for the same logic used around the other block referenced (the code around setHtmlContent at the later 154-161 region).
🧹 Nitpick comments (2)
src/ui/components/ServersView/MarkdownContent.tsx (1)
16-26: Prefer bundle-managed URLs for the highlight styles.These
../node_modules/...hrefs are coupled to the current HTML file layout, so a packaging/layout change can drop syntax highlighting at runtime. Import or otherwise resolve the highlight CSS through the renderer build instead of reaching intonode_modulesfrom the DOM, and please verify it in a packaged build as well as dev mode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/ServersView/MarkdownContent.tsx` around lines 16 - 26, The DOM-inserted stylesheet links (lightLink and darkLink) that point directly into node_modules should be replaced by bundler-managed imports so the CSS is included in the renderer build; remove the code that creates link elements (lightLink and darkLink) and instead import the highlight styles via the module system (e.g., import 'highlight.js/styles/github.css' and import 'highlight.js/styles/github-dark.css' or conditionally import them via the renderer entry) so the files are resolved by the build pipeline, and then verify syntax highlighting in both dev and packaged builds.src/ui/components/ServersView/DocumentViewer.tsx (1)
12-15: Narrowformatto the supported literals.
format?: stringmakes typos across the new IPC/Redux flow look valid and silently falls back to the PDF branch here. A shared'pdf' | 'markdown'union would let TypeScript catch drift earlier; please verify the IPC channel and reducer types are narrowed too. As per coding guidelines, "Use TypeScript strict mode enabled in TypeScript configuration".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/ServersView/DocumentViewer.tsx` around lines 12 - 15, The DocumentViewer component currently types the prop format as a plain string which allows typos to silently fall back to the PDF branch; change the format prop type to the explicit union 'pdf' | 'markdown' in the DocumentViewer props and update any related types used for the IPC channel and reducer (e.g., the action/reducer type for document format and the IPC message payload type) so they use the same union; ensure the component's switch/conditional logic strictly narrows on that union and run TypeScript strict mode to catch mismatches across the IPC channel and reducer definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/components/ServersView/MarkdownContent.tsx`:
- Around line 68-71: The click handler currently calls e.preventDefault() before
checking rawHref, which prevents in-page anchor behavior; update the handler
(the function where rawHref and e.preventDefault() are used, e.g., the link
click callback in MarkdownContent.tsx) to only prevent default for non-hash
links—either check if rawHref.startsWith('#') first and return, or wrap the
preventDefault call with if (!rawHref.startsWith('#')) so in-page “#…” links
retain their default scrolling behavior.
---
Duplicate comments:
In `@src/ui/components/ServersView/MarkdownContent.tsx`:
- Around line 97-108: The generated HTML from marked.parse is injecting asset
tags (<img>, <source>, etc.) that bypass the authenticated fetch path; update
the pipeline that currently calls
ipcRenderer.invoke('document-viewer/fetch-content', url, serverUrl) →
marked.parse(...) → DOMPurify.sanitize(...) → setHtmlContent(...) to rewrite or
proxy all resource URLs before setHtmlContent. Specifically, after marked.parse
(and before DOMPurify.sanitize) walk the parsed HTML (or use a DOM parser) to
find src/srcset/href attributes on img, source, video, audio, link, and any
inline CSS urls, and replace each resource URL with a proxied URL that routes
through the same IPC/session handler (e.g., an endpoint the main process serves
that calls the same 'document-viewer/fetch-content' flow), so assets load via
the server partition; apply the same fix for the same logic used around the
other block referenced (the code around setHtmlContent at the later 154-161
region).
---
Nitpick comments:
In `@src/ui/components/ServersView/DocumentViewer.tsx`:
- Around line 12-15: The DocumentViewer component currently types the prop
format as a plain string which allows typos to silently fall back to the PDF
branch; change the format prop type to the explicit union 'pdf' | 'markdown' in
the DocumentViewer props and update any related types used for the IPC channel
and reducer (e.g., the action/reducer type for document format and the IPC
message payload type) so they use the same union; ensure the component's
switch/conditional logic strictly narrows on that union and run TypeScript
strict mode to catch mismatches across the IPC channel and reducer definitions.
In `@src/ui/components/ServersView/MarkdownContent.tsx`:
- Around line 16-26: The DOM-inserted stylesheet links (lightLink and darkLink)
that point directly into node_modules should be replaced by bundler-managed
imports so the CSS is included in the renderer build; remove the code that
creates link elements (lightLink and darkLink) and instead import the highlight
styles via the module system (e.g., import 'highlight.js/styles/github.css' and
import 'highlight.js/styles/github-dark.css' or conditionally import them via
the renderer entry) so the files are resolved by the build pipeline, and then
verify syntax highlighting in both dev and packaged builds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 254f4a57-b0c6-4505-981f-99acdef942f9
📒 Files selected for processing (25)
src/documentViewer/ipc.tssrc/i18n/ar.i18n.jsonsrc/i18n/de-DE.i18n.jsonsrc/i18n/en.i18n.jsonsrc/i18n/es.i18n.jsonsrc/i18n/fi.i18n.jsonsrc/i18n/fr.i18n.jsonsrc/i18n/hu.i18n.jsonsrc/i18n/it-IT.i18n.jsonsrc/i18n/ja.i18n.jsonsrc/i18n/nb-NO.i18n.jsonsrc/i18n/nn.i18n.jsonsrc/i18n/no.i18n.jsonsrc/i18n/pl.i18n.jsonsrc/i18n/pt-BR.i18n.jsonsrc/i18n/ru.i18n.jsonsrc/i18n/se.i18n.jsonsrc/i18n/sv.i18n.jsonsrc/i18n/tr-TR.i18n.jsonsrc/i18n/uk-UA.i18n.jsonsrc/i18n/zh-CN.i18n.jsonsrc/i18n/zh-TW.i18n.jsonsrc/i18n/zh.i18n.jsonsrc/ui/components/ServersView/DocumentViewer.tsxsrc/ui/components/ServersView/MarkdownContent.tsx
✅ Files skipped from review due to trivial changes (15)
- src/i18n/nb-NO.i18n.json
- src/i18n/uk-UA.i18n.json
- src/i18n/fi.i18n.json
- src/i18n/no.i18n.json
- src/i18n/ar.i18n.json
- src/i18n/de-DE.i18n.json
- src/i18n/ru.i18n.json
- src/i18n/nn.i18n.json
- src/i18n/zh-CN.i18n.json
- src/i18n/fr.i18n.json
- src/i18n/es.i18n.json
- src/i18n/pl.i18n.json
- src/i18n/pt-BR.i18n.json
- src/i18n/ja.i18n.json
- src/i18n/zh.i18n.json
🚧 Files skipped from review as they are similar to previous changes (6)
- src/i18n/en.i18n.json
- src/i18n/se.i18n.json
- src/i18n/hu.i18n.json
- src/i18n/it-IT.i18n.json
- src/i18n/zh-TW.i18n.json
- src/i18n/sv.i18n.json
📜 Review details
⏰ 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). (6)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/documentViewer/ipc.tssrc/ui/components/ServersView/DocumentViewer.tsxsrc/ui/components/ServersView/MarkdownContent.tsx
🧠 Learnings (2)
📚 Learning: 2026-02-04T19:29:54.650Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T19:29:54.650Z
Learning: Applies to **/*.{ts,tsx} : Use Fuselage components from `rocket.chat/fuselage` for all UI work and only create custom components when Fuselage doesn't provide what's needed
Applied to files:
src/ui/components/ServersView/DocumentViewer.tsx
📚 Learning: 2026-03-11T06:38:40.426Z
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
Applied to files:
src/ui/components/ServersView/DocumentViewer.tsx
🪛 ast-grep (0.41.1)
src/ui/components/ServersView/MarkdownContent.tsx
[warning] 160-160: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (1)
src/documentViewer/ipc.ts (1)
38-60: Good hardening on the new fetch IPC.Protocol and same-origin validation before
session.fetchis the right place to scope this partition-backed fetch to the current server.
macOS installer download |
Move preventDefault() after the hash-link check so that #anchor links retain their default scrolling behavior.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/ui/components/ServersView/MarkdownContent.tsx (1)
97-108:⚠️ Potential issue | 🟠 MajorEmbedded markdown assets still bypass the authenticated session.
src/documentViewer/ipc.ts:38-60stops atresponse.text(), so only the top-level markdown file goes through the partition-backed fetch. The generated HTML is then injected unchanged, which leaves relative resource URLs resolving against the desktop app window and causes protected same-origin assets (img,source, etc.) to miss the server session entirely. Please rewrite resource URLs againsturland proxy protected loads through the same fetch flow.Also applies to: 154-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/ServersView/MarkdownContent.tsx` around lines 97 - 108, The generated HTML currently injects resource URLs unchanged, letting relative/protected assets bypass the partition-backed fetch; fix MarkdownContent.tsx by parsing the HTML output from marked.parse into a DOM fragment, rewrite all resource attributes (img.src, source.src, video/audio src, link.href, etc.) to absolute URLs resolved against the original document URL, and for protected/same-origin resources proxy their loads through the existing ipcRenderer.invoke('document-viewer/fetch-content', resourceUrl, serverUrl) call (fetch the resource via the partition, convert the response to a blob or data URL, and replace the attribute with that blob/data URL) before calling DOMPurify.sanitize and setHtmlContent; apply the same rewrite/proxy logic to the other content path referenced around lines 154-161 so all embedded assets use the partition-backed fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/ui/components/ServersView/MarkdownContent.tsx`:
- Around line 97-108: The generated HTML currently injects resource URLs
unchanged, letting relative/protected assets bypass the partition-backed fetch;
fix MarkdownContent.tsx by parsing the HTML output from marked.parse into a DOM
fragment, rewrite all resource attributes (img.src, source.src, video/audio src,
link.href, etc.) to absolute URLs resolved against the original document URL,
and for protected/same-origin resources proxy their loads through the existing
ipcRenderer.invoke('document-viewer/fetch-content', resourceUrl, serverUrl) call
(fetch the resource via the partition, convert the response to a blob or data
URL, and replace the attribute with that blob/data URL) before calling
DOMPurify.sanitize and setHtmlContent; apply the same rewrite/proxy logic to the
other content path referenced around lines 154-161 so all embedded assets use
the partition-backed fetch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5575641-8fce-431e-9b65-0953ad9e928c
📒 Files selected for processing (1)
src/ui/components/ServersView/MarkdownContent.tsx
📜 Review details
⏰ 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). (4)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/ui/components/ServersView/MarkdownContent.tsx
🪛 ast-grep (0.41.1)
src/ui/components/ServersView/MarkdownContent.tsx
[warning] 160-160: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (1)
src/ui/components/ServersView/MarkdownContent.tsx (1)
61-63: Avoid casting away the delegated click target type.This handler is attached to the container, so
e.targetis only anEventTarget. Theas HTMLElementassertion sidesteps strict mode and makes.closest()depend on the target already being anElement.As per coding guidelines, `**/*.{ts,tsx}`: Use TypeScript strict mode enabled in TypeScript configuration.🛡️ Minimal fix
const handleClick = (e: MouseEvent) => { - const anchor = (e.target as HTMLElement).closest('a'); + if (!(e.target instanceof Element)) { + return; + } + + const anchor = e.target.closest('a'); if (!anchor) return;
1 participant
Summary
Adds a markdown file viewer to the Electron desktop client, extending the existing PDF viewer infrastructure to render
.mdfiles with GitHub-flavored markdown styling.When a user clicks a
.mdfile attachment in a server, the download is intercepted and the file is rendered in an overlay viewer instead — same UX pattern as the PDF viewer.Changes
markedwith GFM support,highlight.jsfor syntax highlighting, andDOMPurifyfor XSS sanitization.mdfile downloads at the sessionwill-downloadlevel, cancels the download, and opens the viewersession.fetch) to include auth cookiesprefers-color-schemePdfContentand addsMarkdownContentas separate components for cleaner architecturedocumentViewerFormatthrough Redux state, IPC, and React componentsdocumentViewer.title.pdfanddocumentViewer.title.markdowntranslation keys for all 22 languagesopenDocumentViewer(url, 'markdown')Dependencies added
markedmarked-highlighthighlight.jsdompurifygithub-markdown-cssTest plan
.mdfile attachment in a server → should open markdown viewer overlay (not download)Summary by CodeRabbit
New Features
Internationalization
Style
Chores