2067629 - showPopover({ source }) drops the popover from forward sequential focus navigation when source is a text input or contenteditable

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Brian Birtles (:birtles)]

Attachment: Repro

Steps to reproduce:

1. Open attached test case.
2. Press Tab.

Actual results:

Focus moves to the "after" button. The "in popover" button is skipped.

Expected results:

Focus moves to the "in popover" button, as it does in Chrome.

Two separate things should each put it there: the popover follows its source in DOM order, and per spec a popover's contents are visited immediately after its source.

Notes:

• Changing <input id="source"> -> <button id="source"> works
• Changing <input id="source"> -> <div id="source" tabindex="0"> works
• Changing <input id="source"> -> <div id="source" contenteditable="true"> fails, and additionally Shift+Tab from "after" DOES land on "in popover". So the popover is present in the backward order while absent from the forward one.
• Changing showPopover({ source }) -> showPopover() works. Naming a source is what removes the popover from the position it would otherwise hold in DOM order.
• Changing popover="manual" -> popover="auto" no difference
• Moving the popover after the "after" button still fails with a text-entry source, still works with a button source.

Regression range:

• 143.0 fails for every source type (bug 1984004)
• 144.0 works for every source type (bug 1984004's fix)
• 154.0.1 / 155 works for <button> and tabindex; fails for <input> and contenteditable

Comment 1

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1955857

:ltenenbaum, since you are the author of the regressor, bug 1955857, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Leo Tenenbaum [:ltenenbaum]]

Oh, the problem is that <input> has a UA shadow root which takes priority over the popover since that's how it works for non-UA shadow roots: https://html.spec.whatwg.org/#associated-focus-navigation-owner.
So probably we need to separately deal with the cases of UA vs non-UA shadow roots.

Comment 3

[Leo Tenenbaum [:ltenenbaum]]

Attachment: test.html

Here is another similar case which we aren't handling correctly currently: if the popover invoker has children, then they are never visited in sequential focus navigation. Well maybe the right solution here is that when we get to a popover invoker, we should visit the popover, then visit its children or shadow root. That seems to be approximately what Chrome does, although they visit the shadow root before the popover… (The spec says we should use shadow-including tree order to decide which to visit first, but I don't think the position of the popover in the tree should affect its position in focus navigation. Maybe worth filing a spec issue about that: https://github.com/whatwg/html/issues/12871.)

Comment 4

[Jim Mathies [:jimm]]

Can we get this bug triaged please? (s/p setting)

Comment 5

[Leo Tenenbaum [:ltenenbaum]]

Attachment: Bug 2067629 - Include popover invoker children/shadow root in sequential focus navigation. r=edgar

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d9a41916fb5f
https://hg.mozilla.org/integration/autoland/rev/35945c4c75ca
Include popover invoker children/shadow root in sequential focus navigation. r=edgar

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/35945c4c75ca

Comment 8

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected, along with ESR.
:ltenenbaum, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta and ESR approvals.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox156 and the ESR status flag(s) to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/62448 for changes under testing/web-platform/tests