1430692 - Follow-up of bug 1413834: Handle focus navigation on NAC in shadow DOM

Status: RESOLVED FIXED

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

Description

[Ben Tian (inactive)]

Per bug 1413834 comment 43:

"The thing with <input type=date> is that in light DOM we traverse frame tree, so we enter the layout objects of that element, including the objects for the native anonymous content.
So, should we in shadow DOM case try to access the primary frame of an element, try to get nsIAnonymousContentCreator from it, and if such is got, use frame tree locally for the native anonymous tree to see if there is something to focus."

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nac_in_shadow_focus_with_test.diff

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/189516f166d9ca51fe9eb9083394b73649a88306
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=189516f166d9ca51fe9eb9083394b73649a88306
remote: recorded changegroup in replication log in 0.098s


Ok, this is a bit painful. Currently we use frame tree for tab navigation in light DOM, since that has been our behavior historically and it actually gives the sanest behavior. But for Shadow DOM we follow the Shadow DOM spec. However, DOM tree traversal doesn't know anything about native anonymous content, so this patch makes us use frame tree again for NAC.

We can get rid of this stuff once we convert date and such elements to use shadow DOM internally.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8970680 [details] [diff] [review]
nac_in_shadow_focus_with_test.diff

oops, this needs rebasing.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nac_in_shadow_focus_with_test_2.diff

rebased

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

remote: View the pushlog for these changes here:
remote:   https://hg.mozilla.org/try/pushloghtml?changeset=2262e39a9d94348e6c8ffbbab6a65be6f2ee924c
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=2262e39a9d94348e6c8ffbbab6a65be6f2ee924c
remote: recorded changegroup in replication log in 0.100s

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8970696 [details] [diff] [review]
nac_in_shadow_focus_with_test_2.diff

Review of attachment 8970696 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsFocusManager.cpp
@@ +3275,5 @@
> +
> +      if (firstNonChromeOnly && firstNonChromeOnly == iterContent) {
> +        // We just broke out from the native anonynous content, so move
> +        // to the previous/next node of the native anonymous owner.
> +        aForward ? contentTraversal.Next() : contentTraversal.Prev();

Please write this as a normal if/else.

@@ +3314,5 @@
> +            nsIFrame* last = frame;
> +            while (last) {
> +              frame = last;
> +              last = frame->PrincipalChildList().LastChild();
> +            };

Nit: exta ; after the }s.

(also, this could be a for loop: for (nsIFrame* last = frame; last; last = frame->PrincipalChildList().LastChild()) frame = last;)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

oops, that while used to be do-while.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nac_in_shadow_focus_with_test_3.diff

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nac_in_shadow_focus_with_test_4.diff

better to compile too

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b51c5cf8035
Handle focus navigation on NAC in shadow DOM, r=mrbkap

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4b51c5cf8035