Make anchored-dialog Tab navigation deterministic on Safari/WebKit

[BorisTyshkevich]

Summary

The shared anchored-dialog primitive does not fully own keyboard traversal. It only handles wrapping at the first and last declared focusable elements and delegates every intermediate Tab step to the browser's native sequential-focus policy.

On WebKit/Safari with the default macOS setting where Tab does not highlight every webpage control, native traversal skips checkboxes and buttons. In the multi-select popover this means keyboard focus never reaches Clear, Cancel, or Apply; it can also leave the modal dialog before the trap sees focus at its declared last element.

The existing WebKit E2E failures are therefore not merely a test-environment preference problem. They expose a product accessibility defect in the shared modal-popover focus trap used by both:

• src/ui/multi-select-field.ts;
• src/ui/time-range-field.ts.

Fix the shared primitive so its declared focus order is deterministic across Chromium, Firefox, and WebKit without requiring a host OS/browser preference.

Observed failure

Current tests/e2e/multi-select.spec.js opens the multi-select dialog, presses Tab until the Apply button should be focused, and then verifies its focus-visible outline in both themes.

WebKit fails both theme cases:

[webkit] › tests/e2e/multi-select.spec.js
  › keeps enabled, hover, disabled, focus, and pressed Apply states distinct in dark theme
  › keeps enabled, hover, disabled, focus, and pressed Apply states distinct in light theme

Error: expected outline not to be "none"

Chromium and Firefox reach Apply and pass. WebKit does not focus Apply, so the focus-visible style is never active.

This reproduces against untouched current main; it is not caused by an open feature branch.

Current implementation

openAnchoredDialog() in src/ui/popover.ts recomputes its focusable set on every Tab press:

function focusableEls(): HTMLElement[] {
  return [...dialog.querySelectorAll<HTMLElement>('input, button')]
    .filter((el) => !el.closest('[hidden]')
      && !(el as HTMLInputElement | HTMLButtonElement).disabled);
}

That dynamic filtering is correct and must remain.

The problem is the traversal logic:

const onTabTrap = (e: KeyboardEvent): void => {
  if (e.key !== 'Tab') return;
  const items = focusableEls();
  if (items.length === 0) return;
  const first = items[0];
  const last = items[items.length - 1];
  const activeEl = d.activeElement as HTMLElement | null;

  if (e.shiftKey) {
    if (!activeEl || activeEl === first || !dialog.contains(activeEl)) {
      e.preventDefault();
      last.focus();
    }
  } else if (!activeEl || activeEl === last || !dialog.contains(activeEl)) {
    e.preventDefault();
    first.focus();
  }
};

It intervenes only at the boundaries. From any middle element it intentionally does nothing and assumes the browser will focus the next member of focusableEls().

The unit suite encodes that assumption explicitly:

it('Tab from a middle element does not trap (browser default order applies)', ...)

That assumption is not portable. WebKit's native sequential-focus policy can skip controls that the primitive itself considers focusable.

Concrete multi-select sequence

When the multi-select popover opens, initialFocus places focus on its search input. The primitive's declared order is approximately:

Search input
Select visible checkbox
Option checkbox(es)
Clear button
Cancel button
Apply button

On affected WebKit/Safari configurations, native Tab traversal does not visit all of those controls. Because Apply is never active:

• the E2E focus-state assertion fails;
• Apply is not keyboard-reachable with ordinary Tab;
• the modal trap can allow focus to escape rather than wrapping from Apply to Search;
• the product behavior differs across supported desktop engines.

The time-range popover has the same structural risk: text inputs, caret/buttons, dynamically rendered recent/constant buttons, Cancel, and Apply all share this primitive.

Required behavior

While an anchored dialog is open:

1. ordinary Tab moves to the next currently eligible element in the primitive's declared DOM order;
2. Shift+Tab moves to the previous currently eligible element;
3. traversal wraps within the dialog in both directions;
4. disabled elements are skipped;
5. elements inside a [hidden] ancestor are skipped;
6. the eligible set is recomputed on every key press because consumers mutate visibility and disabled state while open;
7. if focus is outside the dialog or on a node no longer present in the eligible set:
   • Tab moves to the first eligible element;
   • Shift+Tab moves to the last eligible element;
8. a one-element dialog keeps focus on that element;
9. an empty eligible set preserves the current no-op behavior and lets the browser handle the key;
10. keyboard-driven movement retains a visible :focus-visible indication;
11. Escape, backdrop close, commit ordering, focus return, ARIA state, placement, and lifecycle behavior remain unchanged.

No system preference, browser launch flag, or manual developer setup may be required beyond the existing Playwright browser installation.

Required implementation

Change the shared onTabTrap implementation so it owns every Tab transition rather than only boundary wrapping.

An acceptable implementation is:

const onTabTrap = (e: KeyboardEvent): void => {
  if (e.key !== 'Tab') return;

  const items = focusableEls();
  if (items.length === 0) return;

  const active = d.activeElement as HTMLElement | null;
  const current = active ? items.indexOf(active) : -1;
  const delta = e.shiftKey ? -1 : 1;

  const next = current < 0
    ? (e.shiftKey ? items.length - 1 : 0)
    : (current + delta + items.length) % items.length;

  e.preventDefault();
  items[next].focus();
};

Equivalent code is acceptable, but the behavioral contract above is fixed.

Important constraints

• Keep recomputing the eligible list for every press.
• Keep the current dialog-scoped listener; do not move a global Tab listener onto document.
• Do not call stopPropagation() unless an existing event contract demonstrably requires it; preventing native traversal is sufficient.
• Do not mutate consumer elements with positive tabindex values.
• Do not add permanent tab stops.
• Do not reorder the DOM to influence browser traversal.
• Do not duplicate focus-trap logic in either consumer.
• Keep the current input, button scope unless a concrete existing consumer requires another native element. Broadening the primitive to arbitrary links, selects, textareas, or [tabindex] is outside this bug unless necessary for current rendered content.
• Preserve the current disabled and hidden filtering semantics exactly.

Focus-visible requirement

The existing CSS focus treatment is not the root cause. The style is absent in the failing test because Apply never becomes focused.

The shared keydown handler must move focus synchronously as a result of the keyboard event so the target is treated as keyboard-focused and visibly matches :focus-visible in supported engines.

Do not solve this by:

• changing the product rule from :focus-visible to unconditional :focus;
• adding a permanent outline to Apply;
• directly focusing Apply only inside the test;
• asserting a CSS declaration without proving the button became the active element.

If a supported engine does not apply :focus-visible to focus moved synchronously from the Tab key handler, document that with a minimal reproduction and implement the smallest shared keyboard-modality treatment in the primitive. Do not add a multi-select-only workaround.

Unit tests

Update tests/unit/popover.test.ts so the tests describe deterministic primitive-owned traversal rather than native browser delegation.

Required coverage:

Forward traversal

• Tab from the first element focuses the second.
• Tab from a middle element focuses the next element.
• Tab from the last element wraps to the first.
• The event is preventDefault()-ed for every non-empty traversal.

Backward traversal

• Shift+Tab from the last element focuses the previous element.
• Shift+Tab from a middle element focuses the previous element.
• Shift+Tab from the first element wraps to the last.

Dynamic eligibility

• A disabled middle or last element is skipped immediately on the next press.
• An element inside a hidden ancestor is skipped.
• Re-enabling or unhiding an element makes it eligible again without reopening the dialog.
• A focused element that is removed or disabled before the next key press is treated as no longer in the set:
  • Tab chooses the first;
  • Shift+Tab chooses the last.

Edge cases

• One eligible element retains focus in both directions.
• No eligible elements leave the event unhandled, preserving the current empty-set guard.
• Non-Tab keys do not move focus or become prevented.
• The listener remains scoped to the live dialog and is removed on close.

Replace the current test asserting that middle-element Tab is left to browser default; that behavior is the defect.

Playwright regression coverage

Update tests/e2e/multi-select.spec.js to distinguish focus traversal from visual-state styling.

Keyboard sequence test

In at least one theme, assert the real consumer sequence:

1. open the multi-select dialog;
2. verify initial focus is the Search input;
3. press Tab through the checkbox and footer controls;
4. verify each expected control becomes document.activeElement in DOM order;
5. verify Apply is reached using ordinary Tab;
6. verify Apply matches :focus-visible and its computed outline is not none;
7. press Tab once more and verify focus wraps to Search;
8. press Shift+Tab and verify focus wraps back to Apply;
9. verify focus never leaves the dialog during the sequence.

The test must pass in Chromium, Firefox, and WebKit. Do not browser-gate the core traversal assertions.

Existing visual-state test

Keep coverage that enabled, hover, disabled, focus-visible, and pressed states remain distinct in both dark and light themes. Strengthen its focus step to assert both:

await expect(apply).toBeFocused();
expect(await apply.evaluate((el) => el.matches(':focus-visible'))).toBe(true);

before asserting the computed outline.

A bounded loop may remain only if it fails with a clear assertion that Apply was never reached. Prefer an exact, readable sequence based on the fixture's stable controls.

Shared-consumer regression

Add or extend coverage for the time-range consumer where existing E2E infrastructure makes this straightforward:

• open the time-range dialog;
• traverse from its initial input through at least one dynamically rendered button and the footer actions;
• verify focus stays inside and reaches Apply;
• change the dynamic right-column content and verify the next Tab uses the newly rendered eligible set.

A direct shared-primitive browser fixture is acceptable instead if it is smaller and still proves dynamic hidden/disabled recomputation in WebKit. Do not create duplicate consumer logic solely for the test.

Test-environment and CI requirements

The repository supports current desktop Chromium, Firefox, and Safari/WebKit. playwright.config.js intentionally runs all three projects, and the scheduled/manual/release E2E job installs all three engines.

The resolution must work with the current dependency lock and existing CI model.

Do not:

• skip or mark the WebKit cases expected-to-fail;
• add retries to hide the deterministic failure;
• condition assertions on browserName !== 'webkit';
• require developers or CI to enable a macOS Safari preference;
• pin or downgrade Playwright/WebKit to an older build;
• use Alt+Tab/Option+Tab as the only WebKit path;
• remove Apply from the keyboard-order test;
• convert the E2E check into a unit-only CSS assertion.

The exact WebKit version where the failure was first observed is diagnostic context, not the contract. The product must not depend on one engine build's native sequential-focus preference.

Regression boundaries

The change must not alter:

• initial focus selected by each consumer;
• dialog role, aria-modal, or accessible name;
• aria-expanded lifecycle on the trigger;
• Escape handling;
• backdrop dismissal;
• focus return to the trigger on ordinary close;
• skipFocus behavior;
• close-before-commit ordering;
• keyboard-owner registration and cleanup;
• dynamic busy-state disabling in the multi-select;
• time-range validation or dynamic recent/constant rendering;
• mouse and pointer activation;
• visual enabled, hover, disabled, active, or focus styles;
• focus behavior outside anchored dialogs.

Acceptance criteria

• openAnchoredDialog deterministically handles every Tab and Shift+Tab transition for its current focusable set.
• Buttons and checkboxes remain reachable in WebKit without changing a system/browser preference.
• Focus cannot escape either anchored modal dialog through ordinary forward or backward Tab traversal.
• Hidden, disabled, removed, and newly restored controls are handled from a freshly recomputed set.
• Multi-select Apply is reached by ordinary Tab in Chromium, Firefox, and WebKit.
• Apply visibly matches :focus-visible when reached by keyboard.
• The dark- and light-theme action-state assertions remain green.
• The time-range popover retains correct keyboard traversal with dynamic content.
• Existing Escape, backdrop, focus-return, lifecycle, and commit-order tests remain green.
• No browser-specific skip, direct-test focus shortcut, OS preference, retry, or version pin is introduced.
• The inbox label is removed and the issue remains classified as bug + accessibility.

Verification

The implementation PR must report results for:

npm test
npx tsc --noEmit
npm run build
npx playwright test tests/e2e/multi-select.spec.js --project=chromium
npx playwright test tests/e2e/multi-select.spec.js --project=firefox
npx playwright test tests/e2e/multi-select.spec.js --project=webkit
npm run test:e2e

If a dedicated shared-popover or time-range E2E spec is added, include it explicitly in the focused commands as well.

Related work

• Dashboard time-range control: token-based From/To picker in the filter bar #335 introduced the shared anchored-dialog requirement and requires a real Tab trap for the time-range popover.
• Multi-select Apply button hover state is too faint #386 requires enabled, hover, disabled, focus-visible, and pressed Apply states to remain distinct.
• src/ui/popover.ts is the single shared lifecycle and focus-trap implementation; fix the behavior there rather than in either consumer.