(Blocknote): Add external link capture with screen reader hints for blank link targets#22696
(Blocknote): Add external link capture with screen reader hints for blank link targets#22696
Conversation
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.
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.
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.
Previously, aria-describedby was skipped inside contenteditable because directly mutating link DOM attributes outside ProseMirror's knowledge triggered infinite re-render loops (PM strips unknown attributes on re-render, which fires the mutation observer again). This adds a new ExternalLinkA11yExtension that uses ProseMirror Decorations to apply aria-describedby to external link text. Decorations are ProseMirror's sanctioned mechanism for adding DOM attributes without modifying the document model, so no re-render loop occurs. The extension is registered unconditionally (not gated behind capture_external_links), since the screen reader hint is an accessibility concern independent of phishing capture. Implementation details: - Extract isHrefExternal(string) from isLinkExternal(HTMLAnchorElement) so the decoration plugin can check externality from ProseMirror mark attrs (plain href strings) without needing a DOM element - Decoration.inline wraps text nodes in a <span> with the attribute rather than adding it to the <a> element (standard PM behavior) - The referenced description element (open-blank-target-link-description) is already cloned into the BlockNote shadow DOM by block-note-element.ts Co-authored-by: Claude <noreply@anthropic.com>
isHrefExternal treated mailto:, tel:, and javascript: URLs as external because new URL() yields origin "null" for non-web protocols, which never matches window.location.origin. This meant the a11y decoration plugin would incorrectly add the "opens in new tab" hint to non-web links. Add an explicit protocol check so only http/https URLs are considered external. This mirrors the existing guard in isExternalLinkCandidate but applies at the lower level so all callers are safe. Co-authored-by: Claude <noreply@anthropic.com>
akabiru
changed the title
There was a problem hiding this comment.
Pull request overview
This PR moves external link handling for the BlockNote (ProseMirror) editor into BlockNote-native extensions to avoid DOM mutation loops, while keeping phishing-mitigation (/external_redirect) and improving accessibility for screen reader users.
Changes:
- Add BlockNote extensions to (a) intercept external-link clicks for
/external_redirectcapture and (b) addaria-describedbyvia ProseMirror decorations. - Extract shared external-link utilities into a reusable frontend helper and reuse it in the existing
external-linksStimulus controller. - Add/extend Selenium feature specs and a helper for simulating link pastes into the editor.
Reviewed changes
Copilot reviewed 9 out of 9 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 paste_links helper to simulate clipboard paste of one/multiple links into BlockNote in feature specs. |
| modules/documents/spec/features/external_links_in_block_note_spec.rb | New feature spec coverage for “no freeze”, target/rel behavior, a11y hint, and captured-click redirect behavior. |
| frontend/src/stimulus/helpers/external-link-helpers.ts | New shared helper functions for external link detection/candidate checks and building /external_redirect URLs. |
| frontend/src/stimulus/controllers/external-links.controller.ts | Refactors controller to use the shared helper utilities. |
| frontend/src/react/OpBlockNoteContainer.tsx | Plumbs new captureExternalLinks prop down to the editor. |
| frontend/src/react/components/OpBlockNoteEditor.tsx | Registers the new external link extensions, feature-gating capture by prop. |
| frontend/src/react/extensions/external-link-capture.ts | Adds a ProseMirror mousedown interception plugin to route external clicks through /external_redirect. |
| frontend/src/react/extensions/external-link-a11y.ts | Adds ProseMirror decorations to apply aria-describedby hints for external links in the editor DOM. |
| frontend/src/elements/block-note-element.ts | Passes capture setting into React via document.body.dataset.externalLinksEnabledValue. |
BlockNote core team confirmed _tiptapOptions will be removed in a future version. Convert both editor extensions (ExternalLinkCapture, ExternalLinkA11y) from TipTap Extension.create() to BlockNote createExtension(), and register via the extensions option instead of _tiptapOptions.extensions. ProseMirror plugin code is unchanged.
akabiru
force-pushed
the
feature/blocknote-native-extensions
branch
from
bdd7481 to
9a7243c
Compare
| state: { | ||
| init(_, { doc }) { | ||
| return buildDecorations(doc); | ||
| }, | ||
| apply(tr, oldDecos) { | ||
| if (tr.docChanged) { | ||
| return buildDecorations(tr.doc); | ||
| } | ||
| return oldDecos.map(tr.mapping, tr.doc); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
| state: { | |
| init(_, { doc }) { | |
| return buildDecorations(doc); | |
| }, | |
| apply(tr, oldDecos) { | |
| if (tr.docChanged) { | |
| return buildDecorations(tr.doc); | |
| } | |
| return oldDecos.map(tr.mapping, tr.doc); | |
| }, | |
| }, | |
| state: { | |
| init(_, { doc }) { | |
| return buildDecorations(doc); // full build once on editor mount | |
| }, | |
| apply(tr, oldDecos) { | |
| if (tr.docChanged) { | |
| return buildDecorations(tr.doc); // rebuild if doc changed | |
| } | |
| return oldDecos.map(tr.mapping, tr.doc); // cheap remap otherwise | |
| }, | |
| }, |
There was a problem hiding this comment.
🤖 explainer ✨
initruns once when the editor boots — does the first full scanapplyruns on every transaction:tr.docChangedistrueonly when actual content changed (not cursor moves, not selection changes)- On doc change: full rebuild — because a link could have been added, removed, or its href changed
- Otherwise:
.map()shifts existing decoration positions through the transaction's mapping — O(decorations) not O(doc)
One potential improvement here: rebuilding on every docChanged is slightly conservative. If only a non-link node changed (e.g. someone typed in a paragraph with no links), you're still doing a full scan. For most documents this is fine. For very large documents you could add a heuristic:
apply(tr, oldDecos) {
if (!tr.docChanged) return oldDecos.map(tr.mapping, tr.doc);
// Only rebuild if a link mark was actually affected
const affectsLinks = tr.steps.some(step => /* check step ranges touch link marks */);
return affectsLinks ? buildDecorations(tr.doc) : oldDecos.map(tr.mapping, tr.doc);
}But that's an optimisation you'd only need at scale — the current approach is correct and plenty fast for normal document sizes.
| function buildDecorations(doc:PmNode):DecorationSet { | ||
| const decorations:Decoration[] = []; | ||
| doc.descendants((node, pos) => { | ||
| for (const mark of node.marks) { | ||
| if (mark.type.name === 'link' && isHrefExternal(String(mark.attrs.href ?? ''))) { | ||
| decorations.push( | ||
| Decoration.inline(pos, pos + node.nodeSize, { | ||
| 'aria-describedby': 'open-blank-target-link-description', | ||
| }), | ||
| ); | ||
| break; | ||
| } | ||
| } | ||
| }); | ||
| return DecorationSet.create(doc, decorations); | ||
| } | ||
|
|
There was a problem hiding this comment.
🤖 explainer ✨
doc.descendants()walks every text node in the document- For each node it checks its marks (inline formatting) for one named
'link' - If found, it calls
isHrefExternal()to decide if the href points outside the app - If external: pushes a
Decoration.inline()spanning exactly that text node (postopos + node.nodeSize) - The decoration adds a single DOM attribute —
aria-describedby— to the rendered element breakafter the first matching link mark is a nice micro-optimisation — a node won't have twolinkmarks simultaneously, so no need to keep iterating
One subtlety worth noting: Decoration.inline() here spans the text node, not the <a> tag boundary. In ProseMirror, a link mark can span multiple adjacent text nodes (e.g. bold + linked text). This means if a single <a> spans multiple text nodes, each gets its own decoration. That's fine for aria-describedby since the attribute ends up on each rendered span within the link — but worth being aware of.
| * opens in a new tab. It lives in the main layout (`base.html.erb`) and is | ||
| * cloned into the BlockNote shadow DOM by `block-note-element.ts`. | ||
| */ | ||
| export const ExternalLinkA11yExtension = createExtension({ |
There was a problem hiding this comment.
🤖 explainer ✨
createExtension from @blocknote/core is BlockNote's thin wrapper around Tiptap extensions. Using prosemirrorPlugins here is the correct integration point — it maps directly to Tiptap's addProseMirrorPlugins() under the hood. The plugin is registered once and participates in ProseMirror's normal plugin lifecycle.
|
Hi @akabiru |
|
Thanks @bsatarnejad - I'll take a look at that; a bit surprised as the chrome accessibility tree shows the correct result. But perhaps there are nuances with shadow dom and screenreaders. |
Hi @akabiru Initially, I assumed this might be due to This suggests that the issue is not just where the attribute is applied, but that the Given that “opens in new tab” is important interaction information, it would ideally be conveyed directly on the link in a way that is reliably announced by assistive technologies. It might be worth considering an alternative approach (e.g. a more robust semantic solution or a programmatic announcement) rather than relying solely on |
Ticket
What are you trying to accomplish?
Adds external link handling inside the BlockNote editor — both phishing capture (routing clicks through
/external_redirect) and screen reader accessibility (aria-describedbyhints on external links). Previously, the body-levelExternalLinksControllerhandled this by rewriting DOM attributes, but that causes infinite re-render loops inside ProseMirror'scontenteditable.Supersedes #22689 and #22694.
ext-links-capture-docs.mp4
What approach did you choose and why?
We use BlockNote native extensions (
createExtension()withprosemirrorPlugins) instead of DOM manipulation:mousedownbefore ProseMirror's internal click chain, so only our redirect window opens (no double-window from TipTap's Link extension)aria-describedbyvia ProseMirror Decorations, which modify the rendered DOM without touching the document modelNote
These are application-level concerns (not generic enough for op-blocknote-extensions yet).
Both avoid the DOMObserver mutation loop that caused the browser freeze. Shared link utilities are extracted into
external-link-helpers.tsfor reuse with the body-level controller.We initially used TipTap's
_tiptapOptions.extensions, but the BlockNote team confirmed it was an internal API - so we migrated to BlockNote's nativecreateExtension()framework before merging.Merge checklist