fix(web): render project images in markdown previews - #7857
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:unvouched
flamboh
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| <FileMarkdownPreview | ||
| markdown={props.fileContents} | ||
| onRefresh={props.onRefresh} | ||
| renderImage={props.renderMarkdownImage} |
There was a problem hiding this comment.
🟠 High files/ThreadFilesRouteScreen.tsx:154
On Android, workspace-relative images remain broken because FileMarkdownPreview passes renderImage only to SelectableMarkdownText, while hasNativeSelectableMarkdownText() selects the fallback Markdown renderer that ignores this prop. Pass the image renderer into the fallback Markdown renderers as well (for example via createMarkdownRenderers).
Also found in 3 other location(s)
apps/mobile/src/features/files/FileMarkdownPreview.tsx:215
renderImageis passed only toSelectableMarkdownText, buthasNativeSelectableMarkdownText()is false on the default/Android implementation, so Android renders the fallbackMarkdowncomponent without any image override. Consequently workspace-relative images remain broken on Android, including the README/favicon case this code path is intended to support.
apps/mobile/src/features/threads/ThreadFeed.tsx:1436
renderWorkspaceImageis passed only intouseMarkdownStyles, where it is stored asstyles.renderImageforSelectableMarkdownText; the active fallback at lines 969–977 rendersMarkdownwithout an image renderer. SincehasNativeSelectableMarkdownText()currently always returnsfalse, workspace images in thread messages still use the default renderer and the newly added signed-asset flow is never reached.
apps/mobile/src/features/threads/ThreadFeed.tsx:708
renderImageis stored only on theMarkdownStyleSetforSelectableMarkdownText, but it is not added to the Nitro Markdownrenderers. On Android,hasNativeSelectableMarkdownText()is always false, so messages use<Markdown renderers={styles.renderers}>and never invoke this workspace image renderer; workspace-relative images therefore remain broken on Android. Add an image renderer tocreateMarkdownRenderers(or otherwise pass the override into the fallback renderer).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx around line 154:
On Android, workspace-relative images remain broken because `FileMarkdownPreview` passes `renderImage` only to `SelectableMarkdownText`, while `hasNativeSelectableMarkdownText()` selects the fallback `Markdown` renderer that ignores this prop. Pass the image renderer into the fallback `Markdown` renderers as well (for example via `createMarkdownRenderers`).
Also found in 3 other location(s):
- apps/mobile/src/features/files/FileMarkdownPreview.tsx:215 -- `renderImage` is passed only to `SelectableMarkdownText`, but `hasNativeSelectableMarkdownText()` is false on the default/Android implementation, so Android renders the fallback `Markdown` component without any image override. Consequently workspace-relative images remain broken on Android, including the README/favicon case this code path is intended to support.
- apps/mobile/src/features/threads/ThreadFeed.tsx:1436 -- `renderWorkspaceImage` is passed only into `useMarkdownStyles`, where it is stored as `styles.renderImage` for `SelectableMarkdownText`; the active fallback at lines 969–977 renders `Markdown` without an image renderer. Since `hasNativeSelectableMarkdownText()` currently always returns `false`, workspace images in thread messages still use the default renderer and the newly added signed-asset flow is never reached.
- apps/mobile/src/features/threads/ThreadFeed.tsx:708 -- `renderImage` is stored only on the `MarkdownStyleSet` for `SelectableMarkdownText`, but it is not added to the Nitro Markdown `renderers`. On Android, `hasNativeSelectableMarkdownText()` is always false, so messages use `<Markdown renderers={styles.renderers}>` and never invoke this workspace image renderer; workspace-relative images therefore remain broken on Android. Add an image renderer to `createMarkdownRenderers` (or otherwise pass the override into the fallback renderer).
|
|
||
| /** `C:/...` and `C:\...` hrefs read as a "c:" scheme, so URL filters drop them. */ | ||
| export function isWindowsDrivePathHref(href: string): boolean { | ||
| return WINDOWS_DRIVE_PATH_PATTERN.test(href); |
There was a problem hiding this comment.
🟡 Medium src/markdown-links.ts:239
isWindowsDrivePathHref returns false for backslash paths encoded as C:%5Crepo%5Cfile.ts, so markdownUrlTransform passes them to defaultUrlTransform, which treats c: as a disallowed scheme and clears the href. Decode the href before applying WINDOWS_DRIVE_PATH_PATTERN so Windows file links are preserved.
| return WINDOWS_DRIVE_PATH_PATTERN.test(href); | |
| return WINDOWS_DRIVE_PATH_PATTERN.test(safeDecode(href)); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/markdown-links.ts around line 239:
`isWindowsDrivePathHref` returns `false` for backslash paths encoded as `C:%5Crepo%5Cfile.ts`, so `markdownUrlTransform` passes them to `defaultUrlTransform`, which treats `c:` as a disallowed scheme and clears the href. Decode the href before applying `WINDOWS_DRIVE_PATH_PATTERN` so Windows file links are preserved.
| function isSanitizerStrippedImageSrc(src: string): boolean { | ||
| const decoded = safeDecode(src); | ||
| return WINDOWS_DRIVE_PATH_PATTERN.test(decoded) || decoded.toLowerCase().startsWith("file:"); | ||
| } |
There was a problem hiding this comment.
🟡 Medium src/markdown-links.ts:248
isSanitizerStrippedImageSrc preserves file://server/share/logo.png, so raw HTML sends it through parseFileUrlHref, which drops the hostname and resolves it as the local path /share/logo.png; the signed workspace flow can therefore display an unrelated local file instead of the referenced network file. Restrict preserved file: URLs to empty or localhost hosts, or retain UNC-host semantics before resolving them.
-function isSanitizerStrippedImageSrc(src: string): boolean {
- const decoded = safeDecode(src);
- return WINDOWS_DRIVE_PATH_PATTERN.test(decoded) || decoded.toLowerCase().startsWith("file:");
-}
+function isSanitizerStrippedImageSrc(src: string): boolean {
+ const decoded = safeDecode(src);
+ if (WINDOWS_DRIVE_PATH_PATTERN.test(decoded)) return true;
+ if (!decoded.toLowerCase().startsWith("file:")) return false;
+ try {
+ const parsed = new URL(decoded);
+ return parsed.hostname === "" || parsed.hostname.toLowerCase() === "localhost";
+ } catch {
+ return false;
+ }
+}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/markdown-links.ts around lines 248-251:
`isSanitizerStrippedImageSrc` preserves `file://server/share/logo.png`, so raw HTML sends it through `parseFileUrlHref`, which drops the hostname and resolves it as the local path `/share/logo.png`; the signed workspace flow can therefore display an unrelated local file instead of the referenced network file. Restrict preserved `file:` URLs to empty or `localhost` hosts, or retain UNC-host semantics before resolving them.
1 participant
The problem
The solution
UI Changes
Below is the relevant HTML snippet for these screenshots, with the favicon at
public/icon-512.png:Before
After
Built with Fable 5 and GPT 5.6 Sol in T3 Code
Fix markdown previews to render workspace-relative images
resolveMarkdownImagePathin markdown-links.ts to normalize image src values and resolve them against a base directory, returning an absolute workspace path or nullrehypePreserveLocalImageSrcplugin that copies local image srcs (Windows drive paths,file:URLs) intodata-local-srcbefore sanitization so they survive the sanitizerMarkdownWorkspaceImagecomponent in ChatMarkdown.tsx that requests a signed asset URL for the resolved path and falls back to the original src on failureChatMarkdownaccepts an optionalimageBaseDirprop;RenderedMarkdownSurfacein FilePreviewPanel.tsx passes the document's own directory so relative image paths resolve correctlyCHAT_MARKDOWN_SANITIZE_SCHEMAnow permitsdata-local-srconimgelements; external URLs and non-workspace paths render unchanged📊 Macroscope summarized 9112ccf. 10 files reviewed, 4 issues evaluated, 3 issues filtered, 1 comment posted
🗂️ Filtered Issues
apps/mobile/src/features/files/FileMarkdownPreview.tsx — 0 comments posted, 1 evaluated, 1 filtered
renderImageis passed only toSelectableMarkdownText, buthasNativeSelectableMarkdownText()is false on the default/Android implementation, so Android renders the fallbackMarkdowncomponent without any image override. Consequently workspace-relative images remain broken on Android, including the README/favicon case this code path is intended to support. [ Cross-file consolidated ]apps/mobile/src/features/threads/ThreadFeed.tsx — 0 comments posted, 2 evaluated, 2 filtered
renderImageis stored only on theMarkdownStyleSetforSelectableMarkdownText, but it is not added to the Nitro Markdownrenderers. On Android,hasNativeSelectableMarkdownText()is always false, so messages use<Markdown renderers={styles.renderers}>and never invoke this workspace image renderer; workspace-relative images therefore remain broken on Android. Add an image renderer tocreateMarkdownRenderers(or otherwise pass the override into the fallback renderer). [ Cross-file consolidated ]renderWorkspaceImageis passed only intouseMarkdownStyles, where it is stored asstyles.renderImageforSelectableMarkdownText; the active fallback at lines 969–977 rendersMarkdownwithout an image renderer. SincehasNativeSelectableMarkdownText()currently always returnsfalse, workspace images in thread messages still use the default renderer and the newly added signed-asset flow is never reached. [ Cross-file consolidated ]