Repository navigation
fix: #4356 crash when using the link popover on a link with an unsupported protocol - #4473
Conversation
8fd8c2d to
4ec36fb
Compare
|
Rebased onto the latest Just to clarify scope so this is easy to evaluate: this PR only fixes the crash — it stops the popover from offering "jump" when the href was sanitized away, and null-guards Whenever you have a moment, could you take a look / approve the CI run? Happy to adjust anything. |
|
@FurkaanBoraa Pls fix the failed CI. |
A markdown link with an unsupported protocol (e.g. sambesi://) gets its href stripped by muya's sanitizeHyperlink, so the linkTools popover captures href: null and clicking "jump" crashed FORMAT_LINK_CLICK with "TypeError: Cannot read properties of null (reading 'length')". - muya: hide the jump action when linkInfo.href is empty — there is nothing to jump to - desktop: null-guard FORMAT_LINK_CLICK and align the payload types with muya's actual contract (href: string | null) - tests: muya unit specs for the popover filtering; desktop e2e regression spec issue-4356.spec.ts
4ec36fb to
aa86971
Compare
|
Thanks @Jocs — fixed and pushed. The CI failure was in the muya unit spec: after the engine refactor renamed the link-tools private members to the |
2 participants
Closes #4356
Summary
Clicking the "jump" button in the link popover on a link whose URL uses an unsupported protocol (e.g.
sambesi://localhost/node/11164) crashed the renderer withTypeError: Cannot read properties of null (reading 'length').Root cause: muya's
sanitizeHyperlink(DOMPurify's protocol allowlist) strips such hrefs at render time, sogetLinkInfocaptureshref: null, and the popover's jump handler forwarded that toFORMAT_LINK_CLICK, which readdata.href.lengthwithout a guard.The fix is two-layered:
@muyajs/core: the linkTools popover no longer offers "jump" whenlinkInfo.hrefis empty — there is nothing to jump to. This mirrors the existing nothing-to-act-on rule that keeps unresolved reference links out of the popover entirely.FORMAT_LINK_CLICKnull-guardsdata.href(defense in depth), and thejumpClick/ store payload types now match muya's actual contract (href: string | null).Reproduced on macOS against
develop(64e590c) before the fix with the exact stack trace from the issue; the same scenario is clean after the fix.Note on scope: even without the crash, custom-protocol links do not open — the main-process
mt::format-link-clickhandler only openshttp(s)URLs and deliberately swallows other schemes (// Prevent other URLs.). That behavior predates the engine rewrite and is left unchanged here, since opening arbitrary schemes viashell.openExternalis a security decision. Happy to follow up if supporting custom protocols (e.g. behind a confirmation prompt) is wanted — discussed in #4356.Type of change
Test plan
New tests:
packages/muya/src/ui/linkTools/__tests__/linkTools.spec.ts— the popover omits the jump item whenlinkInfo.hrefis null and renders it when an href is present.packages/desktop/test/e2e/issue-4356.spec.ts— Playwright regression spec: the popover on a custom-protocol link offers only "unlink" without renderer errors, and anchor links still offer a working "jump".Verified locally: muya unit tests (726 passed), desktop unit tests (465 passed),
pnpm run lint,pnpm run typecheck,pnpm -C packages/muya lint,pnpm -C packages/muya lint:types, and the desktop e2e suite (97 passed; the one failure,parity-source-undo-saved.spec.ts, fails identically on unmodifieddevelopand is unrelated).Notes for reviewers [optional]
The e2e spec follows the existing
issue-NNNN.spec.tscrash-guard convention and uses thesuppressErrorDialog+expectNoRendererErrorshelpers fromtest/e2e/helpers.ts.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.