Add click-interception for external links in BlockNote editor#22689
Draft
Add click-interception for external links in BlockNote editor#22689
Conversation
akabiru
force-pushed
the
feature/external-links-click-interception-blocknote
branch
3 times, most recently
from
cec1ab7 to
73c5985
Compare
ProseMirror's internal DOMObserver re-parses and re-renders any node whose attributes change, creating infinite loops when the body-level ExternalLinksController writes target, rel, aria-describedby, or rewrites href on links inside the editor. Instead of modifying the DOM, a standalone ProseMirrorExternalLinksController intercepts clicks on external links and routes them through /external_redirect via window.open. The document model retains original URLs, Yjs collaboration is unaffected, and no re-render loops occur. TipTap's Link extension already renders target="_blank" and rel="noopener noreferrer nofollow" from its mark schema defaults, so those attributes are handled natively by ProseMirror. Shared link utilities (isLinkExternal, shouldProcessLink, buildRedirectUrl) are extracted into link-handling helpers so both controllers use a single source of truth without inheritance coupling.
akabiru
force-pushed
the
feature/external-links-click-interception-blocknote
branch
from
73c5985 to
3586e4b
Compare
3 tasks
TipTap's Link extension handles clicks via ProseMirror's mouseup-based handleClick, which fires before any DOM click event. The Stimulus controller's click interception couldn't prevent TipTap from also opening a window, resulting in two windows on every external link click. Replace the Stimulus click interception with a TipTap extension that uses handleDOMEvents.mousedown. Returning true from mousedown prevents ProseMirror from creating its internal MouseDown tracker, so the entire handleClick chain never fires. Only our redirect window opens. The extension is conditionally registered via _tiptapOptions only when external link capture is enabled. When disabled, TipTap's default openOnClick behavior handles link clicks natively.
Handle Text node event targets by normalizing to parentElement
before calling closest('a'). Guard against clicks on anchors
outside the editor content with view.dom.contains(). Replace
brittle sleep with rspec-wait polling for window count.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds external-link capture behavior to the BlockNote (ProseMirror/TipTap) collaborative editor so that external link clicks route through /external_redirect (phishing mitigation), while avoiding DOM mutation loops that previously froze the browser.
Changes:
- Extracted shared external-link utilities (
isLinkExternal,isExternalLinkCandidate,buildExternalRedirectUrl) for reuse across Stimulus and editor code. - Added a TipTap/ProseMirror plugin to intercept link clicks in the editor and open the redirect URL without rewriting DOM attributes inside
contenteditable. - Added Selenium feature coverage for pasting multiple links (freeze regression) and for captured-click routing behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/support/form_fields/primerized/block_note_editor_input.rb | Adds a helper to paste one or multiple links into BlockNote via a synthetic ClipboardEvent. |
| modules/documents/spec/features/external_links_in_block_note_spec.rb | New feature specs covering editor interactivity, link attributes, and captured-click redirect behavior. |
| frontend/src/stimulus/helpers/external-link-helpers.ts | Introduces shared helper functions for determining/capturing external links and building redirect URLs. |
| frontend/src/stimulus/controllers/external-links.controller.ts | Refactors controller to use the extracted shared helpers and centralizes redirect URL building. |
| frontend/src/react/OpBlockNoteContainer.tsx | Threads captureExternalLinks through the BlockNote React container into the editor. |
| frontend/src/react/extensions/external-link-capture.ts | New TipTap extension that intercepts mousedown on external links and opens /external_redirect. |
| frontend/src/react/components/OpBlockNoteEditor.tsx | Conditionally registers the capture extension via _tiptapOptions based on captureExternalLinks. |
| frontend/src/elements/block-note-element.ts | Reads data-external-links-enabled-value from <body> and passes it into the React BlockNote container. |
Narrow closest('a') result with instanceof HTMLAnchorElement for
type safety. Skip non-web protocols (mailto:, tel:, etc.) in
isExternalLinkCandidate to match body-level controller behavior.
Pass element reference to paste_links JS instead of global querySelector.
3 tasks
Member
Author
|
Blocknote core team recommends using blocknote extensions in place of the low-level |
3 tasks
Member
Author
|
Superseded by #22696 |
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.
Ticket
https://community.openproject.org/work_packages/71111
What are you trying to accomplish?
This PR adds external link capture inside the BlockNote editor and fixes the browser freeze that got the original PR reverted.
When capture external links is enabled, clicking an external link in a collaborative document now routes through
/external_redirectfor phishing mitigation — matching how links behave in the rest of the application.ext-links-capture-docs.mp4
What approach did you choose and why?
The body-level
ExternalLinksControllerrewrites link attributes in the DOM, but ProseMirror'sDOMObserverwatches all mutations insidecontenteditableand re-renders against its schema — creating an infinite loop that freezes the browser. We needed a different approach for the editor.We tried a few things:
Stimulus click interception — ProseMirror processes clicks through its own
mousedown→handleClickchain, separate from DOMclickevents. TipTap's Link extension fires via this chain, so a DOM click handler can't prevent it from also opening a window. Result: two windows per click.Overriding TipTap Link's
openOnClickvia_tiptapOptions— BlockNote appends custom extensions after built-ins, and ProseMirror'ssomePropreturns on the first truthy result (first-registered wins). Our override never takes precedence.TipTap extension with
handleDOMEvents.mousedown— this fires before ProseMirror creates its internalMouseDowntracker, preventing TipTap's Link from ever callingwindow.open. Only our redirect window opens, and zero DOM attributes are modified inside the editor. This is a stable, first-class TipTap API backed by ProseMirror's near-frozenPlugincontract.We went with option 3. Shared link utilities (
isLinkExternal,isExternalLinkCandidate,buildExternalRedirectUrl) are extracted intoexternal-link-helpers.tsso both the body-level controller and the editor plugin reuse the same logic.Known trade-off
aria-describedby(the "opens in new tab" screen reader hint) is skipped insidecontenteditablebecause ProseMirror strips unknown attributes on re-render, triggering the same infinite loop. A proper fix via ProseMirror decorations is tracked as a follow-up #22694.Merge checklist