Skip to content

fix: #4356 support custom protocol links - #4416

Closed
Phdigital33 wants to merge 1 commit into
marktext:developfrom
Phdigital33:codex/issue-4356-custom-protocol-links-v2
Closed

Phdigital33 wants to merge 1 commit into
marktext:developfrom
Phdigital33:codex/issue-4356-custom-protocol-links-v2

Conversation

@Phdigital33

Copy link
Copy Markdown

Closes #4356

Summary

  • Allow non-web custom protocol links such as sambesi://... to open through Electron's external shell path.
  • Keep unsafe protocols (file:, javascript:, data:, vbscript:) out of openExternal.
  • Avoid treating Windows drive paths such as C:/Users/example/note.md as external protocols.
  • Make the renderer link-click payload tolerate href: null and fall back to text for links reported by the editor.

Type of change

  • Bug fix (non-breaking, fixes an issue)
  • New feature (non-breaking, adds functionality)
  • Breaking change (causes existing functionality to change)
  • Documentation update

Test plan

  • New tests added
  • Manually tested on: macOS

Validation run:

pnpm --filter marktext typecheck
pnpm --filter marktext build
pnpm --filter marktext exec eslint src/main/menu/actions/file.ts src/renderer/src/store/editor.ts test/e2e/issue-4356.spec.ts
pnpm --filter marktext exec playwright test test/e2e/issue-4356.spec.ts --config test/e2e/playwright.config.ts

Notes:

  • Targeted ESLint completed with warning-only existing non-null assertion warnings in the touched files; no lint errors were reported.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@FurkaanBoraa

Copy link
Copy Markdown
Contributor

Hi — I came at this issue from the crash side (#4473) and traced the renderer paths while testing, so one heads-up in case it's useful: I believe the new isExternalProtocolUrl branch may be unreachable from the actual UI flows. The popover's jump button sends only { href } (editor.vue jumpClick), and for a custom-protocol link muya sanitizes the href away, so the payload arrives as { href: null } with no text and the handler's early return drops it. The Cmd/Ctrl+click path is guarded inside muya (linkMouseEvents.ts returns early when linkInfo.href is null), so it never emits at all. The e2e tests pass because they send the IPC payload directly with text populated, which the UI never does for these links. Making this work end-to-end probably also needs the renderer to forward text (or muya to expose the unsanitized href). Happy to be proven wrong!

@Jocs

Jocs commented Jun 15, 2026

Copy link
Copy Markdown
Member

@Phdigital33 still Draft status?

@Phdigital33

Copy link
Copy Markdown
Author

Thanks @Jocs and @FurkaanBoraa. Yes, this should remain Draft for now.

@FurkaanBoraa, I think your read is correct. The current test exercises the main-process IPC path directly, but it does not prove the real editor/popover flow can reach that branch. For custom protocols, Muya appears to sanitize the rendered href to null, and the current jumpClick path only forwards { href }, so the proposed isExternalProtocolUrl handling may not be reachable from normal UI interaction.

I'll rework this before marking it ready for review. My plan is to make the actual renderer/Muya path preserve or forward the original link target in a safe way, then replace/augment the direct IPC test with an end-to-end test that starts from a real markdown link, opens the popover, clicks jump, and verifies the expected external-open behavior.

I'll also revisit the security boundary around opening custom protocols. If maintainers prefer landing the narrower crash fix first via #4473 and treating custom-protocol support as a separate feature with confirmation/preference behavior, I'm happy to adapt this PR accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link with custom protocoll does not work

3 participants