Add feature for preview of markdown files#10
Open
rickmoonex wants to merge 1 commit intotermermc:masterfrom
Open
Add feature for preview of markdown files#10rickmoonex wants to merge 1 commit intotermermc:masterfrom
rickmoonex wants to merge 1 commit intotermermc:masterfrom
Conversation
Owner
|
Thanks for your PR, I like the proposed feature and I think it would make the client a lot nicer to use for some people. However, based on my review, there are quite a few things that need to be reworked. Also, one question: did you use AI for this PR? |
Author
|
@termermc , used Claude to draft a PR description since there was no template. I've written the code myself. Would like to know the in depth feedback so I can do the reworking where needed. |
Owner
If you scroll up, you should see my comments on pieces of code, but it can be summed up to the following points:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a full-page markdown preview for
.mdfiles in a share, plus an opt-in "auto-open README.md" setting. Peer-supplied markdown is rendered with the same same-origin / external-link discipline as profile pages, so the trust model matches what users already expect.Why
Clicking a
.mdfile in the share browser previously dumped raw markdown into the small sidebar preview as plain text. Shares that are natural fits for markdown — notes, recipe books, documentation — were unreadable without downloading. A full-page preview makes these shares directly useful; auto-opening aREADME.mdmatches the convention used on GitHub, GitLab, filesystem browsers, etc.What changed
1. Full-page markdown preview
/server/:uuid/md/:username/*path(ServerMdPreviewPage.tsx+.module.css)..md/.markdownfile inServerBrowsePagenow navigates to this route (href, notonClick), so middle-click opens in a new tab and the URL is shareable.marked.parse(..., { gfm: true })→DOMPurify.sanitize(..., strict allowlist)→ DOM-level URL rewriting pass → inject..md→.mdlinks useuseNavigateso they feel like single-page transitions; non-.mdin-share targets get a file-server URL and open in a new tab.2. Auto-open README.md (new client setting)
localStorage(per-browser, synchronous, no RPC/proto changes — it's a pure UI preference).ServerBrowsePage, the check runs inside thegetDirFilesstreaming loop, so navigation fires as soon as a chunk containingREADME.mdarrives — no waiting for the full listing to enumerate.readme.md.navigate(..., { replace: true })so browser-back skips the directory entry rather than immediately re-triggering the preview.?noauto=1, which suppresses auto-open for that one visit (you can always get the listing). Subsequent navigation into other directories auto-opens again.3. Security — aligned with profile-page rules
Peer markdown is untrusted content, so the renderer mirrors the existing profile-page constraints (see docs):
sandbox=""+<script>/onerror/<iframe>rejected<style>,<meta>javascript:URLs/foo)/-prefixedhref/src→data-blockedsrc/hrefmust resolve within the profile rootsrcmust resolve within the current share;http(s)anddata:image URLs stripped; escaping relative paths blocked<a>links needtarget="_blank", not normally clickabletarget="_blank" rel="noopener noreferrer", adds a "Middle-click or right-click to open" tooltip, and swallows plain left-click while letting middle-click / Ctrl-Cmd-Shift-Alt-click pass through.md's directory, and anything whose resolved path escapes the share's first segment is blocked with a tooltip explaining whyIntra-share
.md→.mdlinks get the/md/...preview URL; non-.mdin-share references become file-server URLs opened in a new tab. Fragment-only links (#anchor) scroll within the rendered content.Test plan
npm run checkandnpm run buildclean.README.md, subdirectory.mdfiles with relative links/images, and aspicy_test.mdwith deliberate XSS / escape / absolute-path / external-image / external-link payloads..md→ full-page preview renders; breadcrumb back returns to the directory..md→ opens in a new tab.?noauto=1, no re-trigger.alertpops,<iframe>/<script>/onerror/javascript:/ absolute-path / external-image / escape-link cases all inert.https://example.comlink: plain left-click does nothing; middle-click opens a new tab; right-click copies URL.Non-goals
.content pre codeis the target selector).Previeweror the existingguessFileCategory—.mdis now routed independently.