Make scrollable code blocks and cell output keyboard-focusable in HTML output - #14816
Make scrollable code blocks and cell output keyboard-focusable in HTML output#14816cwickham wants to merge 13 commits into
Conversation
…board-focusable When a code block or cell output actually scrolls, keyboard users need to Tab to it and scroll it with the arrow keys (WCAG 2.1.1, axe scrollable-region-focusable). Chrome 132+ and Firefox focus such scrollers natively; this runtime sync adds Safari coverage and an accessible name, and only marks regions while they overflow. Part of #14378
Two new language keys, scrollable-code-label and scrollable-output-label, give keyboard-focusable scrollable regions an accessible name. The after-body script exposes them to the scrollable-regions module via a window global (quarto.js is static, so it cannot be templated directly). Part of #14378
Verified in-browser, two corrections to the sync pass: - An element only scrolls by keyboard when its computed overflow is auto or scroll (axe's own condition). Without this, pre.sourceCode - which bleeds outside its scrolling div.sourceCode parent but is not itself a scroll container - became a second, useless tab stop on every highlighted block. - tabindex="-1" removes an element from the tab order, so Pandoc's per-line anchors (a[href][tabindex="-1"]) must not count as focusable content; they were suppressing the tab stop on every highlighted block. Part of #14378
Unit tests cover the pure helpers (isScrollable geometry + overflow logic, resolveLabels merging). The Playwright spec covers the runtime: attributes present at 390px and absent at 1440px, one tab stop per block, hidden tab panes left alone, removal on resize, Tab reach with a visible focus ring, arrow-key scrolling (chromium/firefox; WebKit routes arrow keys to the page), and code inside <details>. Part of #14378
A `.visually-hidden` block is clipped to 1px and sets `overflow: hidden` on one axis; CSS then computes the other axis to auto, so its full content height reads as overflow. The sync marked it, which gave real quarto-web pages (the get-started screenshots each carry ~9 such blocks) an invisible 1px tab stop labelled "Scrollable code". Require a usable size before marking. Also corrects the copyright header year on the two new files, per .claude/rules/copyright.md. Part of #14378
Pandoc emits a focusable fragment link per numbered line, and #14655 keeps those links focusable on purpose. They satisfy axe's focusable-content condition, so a numbered block passed scrollable-region-focusable while staying unscrollable: every anchor sits at the start of its line, so focusing one leaves scrollLeft at 0 and the clipped content unreachable. Line-number anchors no longer count as focusable content, so a numbered block gets a region tab stop alongside its per-line links. Regions holding genuinely useful focusable content are still skipped. Part of #14378
…special case Opening a <details> re-syncs because it makes the page taller and trips the body ResizeObserver, not because folded code is handled specially. Without the note the test reads as evidence of per-widget support, and the obvious next move is to add a matching hook for tabsets. Part of #14378
|
Filed #14817 for the revealjs side, which this PR does not cover. Same axe rule, different target: there the scroll container is |
These two keys are aria-labels, never shown as visible text, so they belong beside the toggle-* strings rather than next to the code-copy tooltips. Moved in _language.yml (with a comment marking the group), and kept in step in the constants and FormatLanguage declarations. definitions.yml does not list the toggle-* keys, so the schema is unchanged and no artifacts need regenerating. Part of #14378
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
cderv
left a comment
There was a problem hiding this comment.
Nice piece of work, thorough tests and design write-up. Left a few inline comments on things worth fixing before merge.
Two are real correctness issues in scrollable-regions.js: disabled controls are currently counted as focusable content, so a region whose only focusable descendant is a disabled button/input skips its tab stop entirely. And the guard meant to respect author markup checks tabindex/aria-label but not role, so an author-set role gets overwritten then stripped once the region stops overflowing.
There's also a stale-state case: cleanup correctly skips a focused region, but nothing reruns it once focus leaves, so tabindex/role/aria-label can stick around until the next unrelated resize.
Last one is smaller: the two new _language.yml keys are English-only. That's legal, we did the same for #14376, but #14684 already raised the bar by shipping major-locale translations in the same PR, and #14822 exists because we keep letting this drift. Could we add at least the major locales here rather than leaning on #14822 to backfill?
| const kFocusableSelector = | ||
| 'a[href]:not([tabindex="-1"]), button:not([tabindex="-1"]), ' + | ||
| 'input:not([tabindex="-1"]), select:not([tabindex="-1"]), ' + | ||
| 'textarea:not([tabindex="-1"]), [tabindex]:not([tabindex="-1"])'; |
There was a problem hiding this comment.
kFocusableSelector doesn't exclude disabled controls. button:disabled, input:disabled, etc. aren't in the tab order, so hasFocusableContent returns true for a region whose only "focusable" descendant is a disabled control, and syncScrollableRegions skips adding a tab stop there. The region ends up scrollable but unreachable by keyboard. Add :not(:disabled) to the button/input/select/textarea clauses.
| export function syncScrollableRegions(labels) { | ||
| labels = resolveLabels(labels || window.quartoScrollableRegionsLabels); | ||
| for (const el of document.querySelectorAll(kCandidateSelector)) { | ||
| if (!hasUsableSize(el)) { | ||
| continue; | ||
| } | ||
| if (!el.hasAttribute(kMarker)) { | ||
| // respect author markup and regions with their own tab stops | ||
| if ( | ||
| el.hasAttribute("tabindex") || | ||
| el.hasAttribute("aria-label") || | ||
| hasFocusableContent(el) | ||
| ) { | ||
| continue; | ||
| } | ||
| if (isScrollable(el, getComputedStyle(el))) { | ||
| el.setAttribute("tabindex", "0"); | ||
| el.setAttribute("role", "group"); | ||
| el.setAttribute("aria-label", labelFor(el, labels)); | ||
| el.setAttribute(kMarker, ""); | ||
| } | ||
| } else if ( | ||
| !isScrollable(el, getComputedStyle(el)) && | ||
| el !== document.activeElement | ||
| ) { | ||
| el.removeAttribute("tabindex"); | ||
| el.removeAttribute("role"); | ||
| el.removeAttribute("aria-label"); | ||
| el.removeAttribute(kMarker); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The skip guard (lines 110–116) checks tabindex and aria-label but not role. An element with an author-set role (e.g. <pre role="log">) isn't skipped here: it gets role="group" overwritten at line 119, then role is stripped entirely at line 128 once the region stops overflowing. That contradicts the comment above kMarker at the top of the file ("removal never touches author markup") for this one attribute. Add role to the skip condition, or save/restore the original value instead of unconditionally removing it.
| } else if ( | ||
| !isScrollable(el, getComputedStyle(el)) && | ||
| el !== document.activeElement | ||
| ) { | ||
| el.removeAttribute("tabindex"); | ||
| el.removeAttribute("role"); | ||
| el.removeAttribute("aria-label"); | ||
| el.removeAttribute(kMarker); | ||
| } |
There was a problem hiding this comment.
Cleanup is skipped while el === document.activeElement (line 125), which avoids pulling attributes out from under focused content mid-interaction — reasonable. But nothing reruns the check once focus leaves that element. Sequence: Tab into a scrollable region, resize so it no longer overflows, Tab away. tabindex, role, aria-label, and the marker all stay stale until the next unrelated resize or reload. Worth a focusout listener on marked regions that reruns syncScrollableRegions (or just the check for that one element) once focus leaves.
| # Accessible names for scrollable regions (never shown as visible text) | ||
| scrollable-code-label: "Scrollable code" | ||
| scrollable-output-label: "Scrollable output" |
There was a problem hiding this comment.
These two keys only exist in the English _language.yml — none of the 33 locale files got them. That's ok (precedent: #14376 added nav-landmark labels English-only) but it's the exact drift #14822 was filed to track, and the more recent #14684 (skip-to-content link) already raised the bar by shipping translations for the major locales (de/es/fr/it/ja/nl/pt/pt-BR/zh/zh-TW) in the same PR. Worth adding at least those before merge rather than leaning on #14822 to backfill later — it's aria-label-only text, so translation cost is two short strings per locale, not a big lift.
Closes #14378. Part of #8706.
What this does
A code block or cell output that overflows is scrollable, but keyboard users cannot reach or scroll it in Safari. Chrome and Firefox already make such regions focusable natively, so this is a Safari gap for users and a missing accessible name everywhere. axe reports
scrollable-region-focusable(serious, WCAG 2.1.1/2.1.3) in all three, because it reads markup rather than browser behavior. Compatibility under Design notes has the detail.This adds a runtime module to
quarto.js, for bootstrap HTML formats only. While a region overflows it getstabindex="0",role="group", a localizedaria-label, and adata-quarto-scrollablemarker so removal only touches attributes we added. The attributes come off when the region fits again. The sync runs onDOMContentLoadedand from the existing throttled bodyResizeObserver, so it adds no new observers.The candidates are the three scroll containers in rendered HTML:
div.sourceCodediv.sourceCode { overflow: auto }for highlighted codeprepre { overflow: auto }.cell-output-display:not(.no-overflow-x)_quarto-rules.scssWhich regions get marked
A region qualifies when its computed overflow is
autoorscroll, which is axe's own condition, and it overflows by more than 1px. Three kinds of region are skipped:tabindexoraria-label.<details>, an inactive tab pane, or avisually-hiddencode alternative. The last is clipped to 1px, which makes its content height read as overflow. Marking it would add an invisible tab stop, and quarto-web's get-started pages carry about nine each.Pandoc's line anchors are the exception to that third rule. Without line numbers they carry
tabindex="-1", so they are out of the tab order anyway. With line numbers they are real focusable links, kept that way on purpose by #14655. But each one sits at the start of its line, so focusing it leavesscrollLeftat 0 and the clipped content unreachable. A numbered block therefore satisfies axe while staying unscrollable, so it gets its own region tab stop alongside its per-line links.When it re-syncs
One rule, not one per widget: the module re-syncs when the page's layout changes size. Collapsed callouts, folded
<details>, and tab panes of differing heights all make the page taller, so a singleResizeObservercovers them with no mechanism-specific code.The boundary is a reveal that changes no size, and two tab panes of equal height are the clean example. That case keeps today's behavior rather than regressing anything. A
shown.bs.tabhook would fix tabsets while still missing equal-height accordions and overlay panels, trading one documented boundary for a special case plus a boundary.Design notes
Runtime JS, not a static
tabindex. A Lua filter would put a tab stop on every code block, whether or not it scrolls. It would also change every rendered HTML file, breaking downstream snapshot tests and scrapers. Rendered HTML stays byte-identical here.Compatibility. Chrome (since 132, January 2025) and Firefox already make these regions focusable natively (Chrome blog). These tab stops have therefore been live on every Quarto site for over a year, with no reports. That bounds the risk: this polyfills shipped browser behavior rather than inventing a new interaction. What it adds on top is Safari coverage, an accessible name, and axe conformance. Every failure mode leaves an element exactly as it is today.
Labels. Two new language keys,
scrollable-code-labelandscrollable-output-label.quarto.jsis static, so the after-body script passes the resolved strings through a window global. The module falls back to the English defaults.role="group", notregion. Pages carry many code blocks, andregionlandmarks would flood the screen-reader rotor.Tests
tests/unit/scrollable-regions.test.ts): overflow geometry, usable size, the focusable-content rule, label merging.tests/integration/playwright/tests/html-scrollable-regions.spec.ts) on chromium, firefox, and webkit: marking and removal across viewports, the three skips, one tab stop per block, real-Tab reach with a visible focus ring, and arrow-key scrolling. Arrow keys are asserted on chromium and firefox only, because Playwright's WebKit routes them to the page even when a scroller has focus.axe-accessibilitystill passes (116 tests), with its code-line-number fixtures re-rendered.about.html(#cb1) anddocs/troubleshooting/index.html(barepre) are gone, and axe reports none for the rule at 390x844.Checklist
I have (if applicable):
AI-assisted PR