1430020 - Follow-up of bug 1413834: Handle focus navigation on <iframe> in shadow DOM

Status: RESOLVED FIXED

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

Description

[Ben Tian (inactive)]

Bug 1413834 implements focus navigation for shadow DOM [1] but <iframe> in shadow DOM is not handled properly. Need to focus into it.

[1] https://w3c.github.io/webcomponents/spec/shadow/#sequential-focus-navigation

Comment 1

[Ben Tian (inactive)]

Neil, do you know if document navigation in bug 1132518 should navigate into/out from shadow DOM? Also is there spec for document navigation with F6?

Comment 2

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

User doesn't know whether some area is in Shadow DOM or not, so if there isn't a spec for some feature, IMO, we should try to keep the behavior light DOM has.

Perhaps Marco has some opinion here.

Comment 3

[Marco Zehe (:MarcoZ)]

My opinion is that, whether dealing with shadow DOM or not, should be completely ubiquitous to end users. Shadow DOM is a technicality we as web developers and browser makers have to deal with, but the end user does not have to care about, and we must not force them to just for our convenience.

Comment 4

[Ben Tian (inactive)]

Seems we should ignore shadow boundary for document navigation. Thanks for the input.

Comment 5

[Neil Deakin]

F6 should navigate and focus the root element of <browser>, <editor> and frames in a <frameset>, but just traverse inside but not stop on <iframe>. The order should be the same as tab navigation.

Comment 7

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

Attachment: shadow_frame_navigation.diff

(I'll ask someone else to review if I see you won't have time)

remote: View your changes here:
remote:   https://hg.mozilla.org/try/rev/9faff1f08a8467b0e3f8897f4ac19b6d4b687c4d
remote:   https://hg.mozilla.org/try/rev/d798e276905dfc79ee15bcdc0d5b57c59f8a4c2e
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=d798e276905dfc79ee15bcdc0d5b57c59f8a4c2e
remote: recorded changegroup in replication log in 0.094s

Comment 8

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

The patch is too small, but the actual fix is.
The change to nsContentUtils::IsSubDocumentTabbable and the 
@@ -3353,24 +3355,43 @@ nsFocusManager::GetNextTabbableContentIn part.
Rest is just adding right params and refactoring code to TryDocumentNavigation and TryToMoveFocusToSubDocument methods.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8971369 [details] [diff] [review]
shadow_frame_navigation.diff

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

::: dom/base/nsFocusManager.cpp
@@ +3382,5 @@
>        }
>  
>        // Search in scope owned by iterContent
>        nsIContent* contentToFocus =
> +        GetNextTabbableContentInScope(iterContent, iterContent, 

Uber nit: trailing space.

Comment 10

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

Attachment: shadow_frame_navigation.diff

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bde59ad4082
let sequential focus navigation in shadow DOM enter iframes, r=mrbkap

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5bde59ad4082