Keep Tab out of image captions - #1224
Open
jeremy wants to merge 1 commit into
Open
Conversation
Tabbing through a message drops the cursor into image captions. The caption is a real <textarea> in the document, so native sequential focus navigation walks into it: with the caret in ordinary text and nothing selected, one Tab lands in the caption of the next image rather than moving past it. tabIndex -1 takes the textarea out of the tab order, which is the whole fix for the reported behaviour. On its own that would trade one accessibility bug for another -- a caption no keyboard user can reach at all -- so KEY_TAB_COMMAND at COMMAND_PRIORITY_HIGH restores a deliberate path: with an attachment selected, Tab moves into that attachment caption. Escape hands focus back to the editor and reselects the attachment, so Tab can step in again. Shift+Tab is untouched and still steps backwards out of the editor. The caption also gains an accessible name. It is now reachable only on purpose, so the label is what tells a screen-reader user where they have landed.
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.
Tabbing through a message drops the cursor into image captions. Reported by a customer in early July; the fix has been sitting inside #1053 since then, behind a review conversation about a different part of that PR.
The bug
The caption is a real
<textarea>in the document, so native sequential focus navigation walks straight into it. Verified onmainwith a probe before touching anything — caret in ordinary text, nothing selected,captionTabIndex: 0, oneTab, anddocument.activeElementis the caption textarea.The fix
tabIndex: -1takes the textarea out of the tab order. That one line is the whole fix for the reported behaviour.On its own it would trade one accessibility bug for another — a caption no keyboard user could reach at all. So
KEY_TAB_COMMANDatCOMMAND_PRIORITY_HIGHrestores a deliberate path: with an attachment selected,Tabmoves into that attachment's caption.Escapehands focus back to the editor and reselects the attachment, soTabcan step in again.Shift+Tabis untouched and still steps backwards out of the editor.The caption also gains an accessible name (
ariaLabel). It is now reachable only on purpose, so the label is what tells a screen-reader user where they have landed.Why this is separable from #1053
Every external symbol it needs is already on
main:$isActionTextAttachmentNode,$createNodeSelectionWith,this.editor, and the existingNodeSelection. Nothing here touchesattachment_toolbar.jsorfake_selection.js— the two files the open review threads on #1053 are actually about.Two deliberate trims against the #1053 version:
ariaHiddentoggling. That belongs to the aria-hidden caption mirror (.attachment__caption-text), which is not in this PR; without the mirror the toggle is dead code.$singleSelectedNodeinlined rather than exported fromlexical_helper.js, so this PR adds no shared surface that Improve attachment accessibility #1053 would then conflict with.#1053 stays open for the toolbar work.
One check worth flagging
The behaviour I could not settle by reading was whether a figure click leaves the contenteditable focused on
main, since #1053 also adds a fake-selection span that parks the DOM range inside the figure. Without that span, doesKEY_TAB_COMMANDfire at all?It does. After a figure click on
main:activeElementis the contenteditable,window.getSelection().rangeCountis0(no DOM range at all — the NodeSelection state), and aTabkeydown still arrives on the contenteditable. The fake-selection span is not load-bearing for this.Tests
Three, in
attachment_caption_focus.test.js. Two are ported from #1053; the first is new — #1053's pair does not actually cover the reported complaint, because neither asserts thatTabfrom a text caret stays out of the caption.Each was checked for non-vacuity by reverting one piece at a time, and each revert fails exactly one test:
tabIndex: -1tabIndex, dropKEY_TAB_COMMANDEscapebranchWorth noting the middle row: with
tabIndexat0the Tab-into-caption test passes without the command, because native focus order reaches the textarea anyway. It only tests the command once the textarea is out of the tab order.9/9 green across Chromium, Firefox and WebKit. Full attachment suite: 103 passed. Four tests reported flaky, all upload-timing;
mainflakes in the same files with a different subset each run, so they are pre-existing and not from this change.