Closed Bug 1490820 Opened 1 year ago Closed 1 year ago

Sequential focus handling issue in Shadow DOM


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




Tracking Status
firefox63 --- fixed
firefox64 --- fixed


(Reporter: smaug, Assigned: smaug)




(2 files)

Load the web page.
Sign in (required, but an account is free).
Go to a Family Tree Person Page (for example )
In the "Vitals" section of the page, click "Edit" to the right of "Birth".
In the "Edit Birth" popup, press the <tab> key on your keyboard.
The "X" in the top right corner of the poup gets focus.
Press <tab> again, the Date of Birth gets focus.
Press <tab> again, nothing happens.

This bad behavior is present in 63.0b5 (64-bit) on Windows 10
and in current nightly, but the page works correctly in 62.0
Priority: -- → P2
Assignee: nobody → bugs
Attached file minimal testcase
Ronald, thanks a ton for reporting the issue!

Took a bit time to find the minimal testcase, but fix should be very simple.
We rely elsewhere that focusable element actually has nsIFrame, since for normal DOM we use frame traversal to find next focusable content. But shadow DOM DOM tree traversal is used (per shadow DOM spec).
Attachment #9010936 - Flags: review?(ehsan)
Comment on attachment 9010936 [details] [diff] [review]

Review of attachment 9010936 [details] [diff] [review]:

::: dom/base/test/file_bug1453693.html
@@ +126,5 @@
>          sr.appendChild(shadowInput);
> +        var hiddenShadowButton = document.createElement("button");
> +        hiddenShadowButton.setAttribute("style", "display: none;");
> +        sr.appendChild(hiddenShadowButton);

Is there a wpt test that covers this?  It seems like having a wpt test for this would be valuable...
Attachment #9010936 - Flags: review?(ehsan) → review+
I don't know how to write wpt tests dealing with native input events.
There are some ways, but last time I asked, it was still limited.
jgraham told that is now possible

I guess the test should be converted to wpt
Pushed by
elements without frames shouldn't be focusable in shadow DOM, r=ehsan
Comment on attachment 9010936 [details] [diff] [review]

Approval Request Comment
[Feature/Bug causing the regression]: New feature, shadow dom, bug 1205323
[User impact if declined]: sequential focus handling may be broken (tab key)
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: just landed
[Needs manual test from QE? If yes, steps to reproduce]: I don't think so 
[List of other uplifts needed for the feature/fix]: NA
[Is the change risky?]: this shouldn't be very risky
[Why is the change risky/not risky?]: ... because this just skips elements which aren't visible when looking for the next element to focus in shadow DOM
[String changes made/needed]: NA
Attachment #9010936 - Flags: approval-mozilla-beta?
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment on attachment 9010936 [details] [diff] [review]

Low risk patch for a bug in a new 63 feature, uplift approved for 63 beta 9, thanks.
Attachment #9010936 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1518121
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.