Bug 178001 – Focus navigation order in slot fallback contents is wrong

• Component: DOM
• Status: RESOLVED FIXED

By running "focus-navigation-across-slots.html" test in WebKit Layout Tests, it seems that the navigation order is not following the spec.
https://w3c.github.io/webcomponents/spec/shadow/#sequential-focus-navigation

According to the spec 5.3 Step 2, in focus navigation, the subtree whose root is the slot element which no element is assigned to is considered as one of the focus navigation scope. Elements inside slot fallback contents scope should be navigated sequential.
In this test, the order should be 14->17->15->13->16.

[Radar WebKit Bug Importer]

<rdar://problem/42842997>

[Ryosuke Niwa]

Created attachment 347762 [details]
WIP

[Ryosuke Niwa]

Created attachment 347765 [details]
Fixes the bug

[Antti Koivisto]

Comment on attachment 347765 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=347765&action=review

> Source/WebCore/page/FocusController.cpp:283
> +        auto slot = makeRef(downcast<HTMLSlotElement>(element));
> +        return FocusNavigationScope(slot, slot->assignedNodes() ? SlotKind::Assigned : SlotKind::Fallback);

makeRef here look unnecessary. FocusNavigationScope doesn't ref the slot so someone better be keeping it alive.

[Ryosuke Niwa]

(In reply to Antti Koivisto from comment #4)
> Comment on attachment 347765 [details]
> Fixes the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347765&action=review
> 
> > Source/WebCore/page/FocusController.cpp:283
> > +        auto slot = makeRef(downcast<HTMLSlotElement>(element));
> > +        return FocusNavigationScope(slot, slot->assignedNodes() ? SlotKind::Assigned : SlotKind::Fallback);
> 
> makeRef here look unnecessary. FocusNavigationScope doesn't ref the slot so
> someone better be keeping it alive.

I'll revert this change. I was trying to adopt Ref/RefPtr everywhere but decided to do it in a separate patch.

[Ryosuke Niwa]

Committed r235191: <https://trac.webkit.org/changeset/235191>