Add aria-describedby screen reader hint for external links in BlockNote#22694
Draft
Conversation
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>
3 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds back the “opens in new tab” screen reader hint for external links inside the BlockNote editor without triggering ProseMirror mutation loops, by applying aria-describedby via ProseMirror decorations.
Changes:
- Introduce a TipTap/ProseMirror decoration-based extension to apply
aria-describedbyfor external links in the editor. - Extract
isHrefExternal(href)fromisLinkExternal(link)so externality can be checked from mark attrs (string hrefs). - Update the feature spec to assert the decoration-wrapped DOM structure in the editor shadow root.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modules/documents/spec/features/external_links_in_block_note_spec.rb | Updates feature spec to assert the aria-describedby hint is present via decoration wrapper. |
| frontend/src/stimulus/helpers/external-link-helpers.ts | Adds isHrefExternal() and refactors isLinkExternal() to delegate to it. |
| frontend/src/react/extensions/external-link-a11y.ts | New TipTap extension that builds/remaps DecorationSet to add aria-describedby for external links. |
| frontend/src/react/components/OpBlockNoteEditor.tsx | Registers the new a11y extension (and conditionally the capture extension) in _tiptapOptions. |
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>
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
Member
Author
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/wp/73721
What are you trying to accomplish?
The parent PR (#22689) had to skip
aria-describedbyinside BlockNote's contenteditable because mutating link attributes from outside ProseMirror triggers an infinite re-render loop. This means screen reader users don't hear "opens in new tab" when focused on an external link inside the editor.This PR adds the hint back using ProseMirror Decorations — the correct mechanism for adding DOM attributes without touching the document model. No mutation loop, no document corruption, just a well-behaved
<span aria-describedby="...">wrapping the link text.What approach did you choose and why?
📖 https://prosemirror.net/docs/ref/#view.Decorations
ProseMirror inline decorations create a wrapper inside the mark's element, they do not modify the mark-rendered tag itself.
A new
ExternalLinkA11yExtension(TipTap extension) that lives alongside the existingExternalLinkCaptureExtension. It's separate because the a11y hint should always be present, while capture is feature-gated.The plugin uses the
state.init/applypattern so decorations are only rebuilt when the document changes — on selection changes and other non-doc transactions, positions are cheaply remapped viaDecorationSet.map().Also extracted
isHrefExternal(string)fromisLinkExternal(HTMLAnchorElement)so the decoration plugin can check externality from ProseMirror mark attrs (plain href strings) without needing a DOM element.Merge checklist