Skip to content

Commit

Permalink
Focus ordering should respect slot elements
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=151379

Reviewed by Antti Koivisto.

Source/WebCore:

Implemented the sequential focus navigation ordering as discussed on
WICG/webcomponents#375

New behavior treats each shadow root and slot as a "focus scope". The focus navigation ordering
is defined within each "focus scope" using tabindex, treating any "focus scope owner"
(e.g. shadow host or a slot) as if it was having tabindex=0 if it wasn't itself focusable.

This patch modifies FocusNavigationScope to support a focus scope defined for a slot element in
addition to the one defined for a shadow tree and a document as previously supported.

Tests: fast/shadow-dom/focus-across-details-element.html
       fast/shadow-dom/focus-navigation-across-slots.html

* dom/Node.cpp:
(WebCore::parentShadowRoot): Extracted from assignedSlot.
(WebCore::Node::assignedSlot):
(WebCore::Node::assignedSlotForBindings): Added.
* dom/Node.h:
* dom/NonDocumentTypeChildNode.idl:
* html/HTMLDetailsElement.h:
(HTMLDetailsElement::hasCustomFocusLogic): Added. Don't treat details element as a "focus scope".
* html/HTMLSummaryElement.h:
(HTMLSummaryElement::hasCustomFocusLogic): Ditto for summary element.
* page/FocusController.cpp:
(WebCore::hasCustomFocusLogic): Moved.
(WebCore::isFocusScopeOwner): Added. Returns true on a shadow host without a custom focus logic or
on a slot inside a shadow tree whose shadow host doesn't have a custom focus logic.
(WebCore::FocusNavigationScope::firstChildInScope): Now takes a reference. Call isFocusScopeOwner
to check for both slots and shadow roots instead of just the latter. This fixes a subtle bug that
focus may never get out of textarea in some cases due to its failure to check hasCustomFocusLogic.
(WebCore::FocusNavigationScope::lastChildInScope): Ditto.
(WebCore::FocusNavigationScope::parentInScope): Made this a member function since it needs to check
against m_slotElement inside the focus scope of a slot.
(WebCore::FocusNavigationScope::nextSiblingInScope): Added. Finds the next assigned node in a slot
in the focus scope defined for a slot. Just calls nextSibling() in the focus scope for shadow tree
and document.
(WebCore::FocusNavigationScope::previousSiblingInScope): Ditto for finding the previous sibling.
(WebCore::FocusNavigationScope::firstNodeInScope): Added. This function replaces rootNode() which
doesn't exist for the focus scope of a slot element.
(WebCore::FocusNavigationScope::lastNodeInScope): Ditto for the last node.
(WebCore::FocusNavigationScope::nextInScope):
(WebCore::FocusNavigationScope::previousInScope):
(WebCore::FocusNavigationScope::FocusNavigationScope): Added a variant that takes HTMLSlotElement.
(WebCore::FocusNavigationScope::owner): Added the support for slot elements.
(WebCore::FocusNavigationScope::scopeOf): Ditto.
(WebCore::FocusNavigationScope::scopeOwnedByScopeOwner): Ditto.
(WebCore::isFocusableElementOrScopeOwner): Added the support for slot elements and renamed from
isFocusableOrHasShadowTreeWithoutCustomFocusLogic.
(WebCore::isNonFocusableScopeOwner): Ditto. Renamed from isNonFocusableShadowHost.
(WebCore::isFocusableScopeOwner): Ditto. Renamed from isFocusableShadowHost.
(WebCore::shadowAdjustedTabIndex): Added the support for slot elements.
(WebCore::FocusController::findFocusableElementAcrossFocusScope):
(WebCore::FocusController::nextFocusableElementWithinScope):
(WebCore::FocusController::previousFocusableElementWithinScope):
(WebCore::FocusController::findElementWithExactTabIndex):
(WebCore::nextElementWithGreaterTabIndex): Call firstNodeInScope() instead of rootNode() here since
there is no root node for the focus scope defined for a slot element.
(WebCore::previousElementWithLowerTabIndex): Ditto for scope.lastNodeInScope().
(WebCore::FocusController::nextFocusableElementOrScopeOwner):
(WebCore::FocusController::previousFocusableElementOrScopeOwner):
(WebCore::parentInScope): Deleted.
(WebCore::FocusNavigationScope::rootNode): Deleted.
(WebCore::FocusNavigationScope::scopeOwnedByShadowHost): Deleted.
(WebCore::isNonFocusableShadowHost): Deleted.
(WebCore::isFocusableShadowHost): Deleted.
(WebCore::isFocusableOrHasShadowTreeWithoutCustomFocusLogic): Deleted.

LayoutTests:

Added regression tests for moving focus by tab and shift+tab across
user-defined shadow trees with slots and details element.

* fast/shadow-dom/focus-across-details-element-expected.txt: Added.
* fast/shadow-dom/focus-across-details-element.html: Added.
* fast/shadow-dom/focus-navigation-across-slots-expected.txt: Added.
* fast/shadow-dom/focus-navigation-across-slots.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@200964 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa@webkit.org committed May 16, 2016
1 parent caf9068 commit 38df02f
Show file tree
Hide file tree
Showing 13 changed files with 541 additions and 81 deletions.
15 changes: 15 additions & 0 deletions LayoutTests/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
2016-05-16 Ryosuke Niwa <rniwa@webkit.org>

Focus ordering should respect slot elements
https://bugs.webkit.org/show_bug.cgi?id=151379

Reviewed by Antti Koivisto.

Added regression tests for moving focus by tab and shift+tab across
user-defined shadow trees with slots and details element.

* fast/shadow-dom/focus-across-details-element-expected.txt: Added.
* fast/shadow-dom/focus-across-details-element.html: Added.
* fast/shadow-dom/focus-navigation-across-slots-expected.txt: Added.
* fast/shadow-dom/focus-navigation-across-slots.html: Added.

2016-05-16 Ryan Haddad <ryanhaddad@apple.com>

Rebaseline tests for ios-simulator
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Tests for moving focus across details element. The existence of shadow tree on details element should not affect the focus sequential navigation ordering.
To manually test, press tab key five times and then shift+tab five times. It should traverse focusable elements in the increasing numerical order and then in the reverse order.

1. Focusable element with tabindex=1
2. Focusable element in details with tabindex=2
3. Focusable element in details with tabindex=3
4. Focusable element in summary with tabindex=4
5. Focusable element in details with tabindex=5
6. Focusable element in details with tabindex=6
5. Focusable element in details with tabindex=5
4. Focusable element in summary with tabindex=4
3. Focusable element in details with tabindex=3
2. Focusable element in details with tabindex=2
1. Focusable element with tabindex=1

49 changes: 49 additions & 0 deletions LayoutTests/fast/shadow-dom/focus-across-details-element.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<!DOCTYPE html>
<html>
<body>
<p>Tests for moving focus across details element.
The existence of shadow tree on details element should not affect the focus sequential navigation ordering.<br>
To manually test, press tab key five times and then shift+tab five times.
It should traverse focusable elements in the increasing numerical order and then in the reverse order.</p>
<div id="test-content">
<div id="first" tabindex="1" onfocus="log(this)">1. Focusable element with tabindex=1</div>
<details open>
<div tabindex="6" onfocus="log(this)">6. Focusable element in details with tabindex=6</div>
<summary><div tabindex="4" onfocus="log(this)">4. Focusable element in summary with tabindex=4</div></summary>
<div tabindex="2" onfocus="log(this)">2. Focusable element in details with tabindex=2</div>
</details>
<details open>
<div tabindex="5" onfocus="log(this)">5. Focusable element in details with tabindex=5</div>
<div tabindex="3" onfocus="log(this)">3. Focusable element in details with tabindex=3</div>
</details>
</div>
<pre></pre>
<script>

function log(element) {
document.querySelector('pre').textContent += element.textContent + '\n';
}

if (window.testRunner)
testRunner.dumpAsText();

document.getElementById('first').focus();

if (window.eventSender) {
eventSender.keyDown('\t');
eventSender.keyDown('\t');
eventSender.keyDown('\t');
eventSender.keyDown('\t');
eventSender.keyDown('\t');

eventSender.keyDown('\t', ['shiftKey']);
eventSender.keyDown('\t', ['shiftKey']);
eventSender.keyDown('\t', ['shiftKey']);
eventSender.keyDown('\t', ['shiftKey']);
eventSender.keyDown('\t', ['shiftKey']);
document.getElementById('test-content').style.display = 'none';
}

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
Tests for moving focus by pressing tab key across shadow boundaries.
To manually test, press tab key sixteen times then shift+tab sixteen times.
It should traverse focusable elements in the increasing numerical order and then in the reverse order.

1. First sequentially focusable element
2. The focusable element in shadow tree with the higehst tabindex
3. The focusable element in shadow tree with the lowest tabindex
4. Slotted content with tabindex=4
5. Slotted content with tabindex=0
6. The focusable element in shadow tree with the higehst tabindex
7. Focusable slot 1
8. Content in slot 1 with tabindex=7
9. Content in slot 1 with tabindex=0
10. Content in slot 2 with tabindex=1
11. Content in slot 2 with tabindex=1
12. Content in slot 2 with tabindex=0
13. Non-focusable slot fallback with tabindex=1
14. Focusable slot element.
15. Shadow content with tabindex=2
16. Non-focusable slot fallback with tabindex=0
17. Focusable slot fallback content with tabindex=0
16. Non-focusable slot fallback with tabindex=0
15. Shadow content with tabindex=2
14. Focusable slot element.
13. Non-focusable slot fallback with tabindex=1
12. Content in slot 2 with tabindex=0
11. Content in slot 2 with tabindex=1
10. Content in slot 2 with tabindex=1
9. Content in slot 1 with tabindex=0
8. Content in slot 1 with tabindex=7
7. Focusable slot 1
6. The focusable element in shadow tree with the higehst tabindex
5. Slotted content with tabindex=0
4. Slotted content with tabindex=4
3. The focusable element in shadow tree with the lowest tabindex
2. The focusable element in shadow tree with the higehst tabindex
1. First sequentially focusable element

150 changes: 150 additions & 0 deletions LayoutTests/fast/shadow-dom/focus-navigation-across-slots.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<!DOCTYPE html>
<html>
<body>
<p>Tests for moving focus by pressing tab key across shadow boundaries.<br>
To manually test, press tab key sixteen times then shift+tab sixteen times.<br>
It should traverse focusable elements in the increasing numerical order and then in the reverse order.</p>
<style>

#test-content > * {
display: block;
width: 350px;
height: 150px;
border: solid 1px black;
margin: 10px;
float: left;
}

pre {
padding-top: 1rem;
clear: both;
}

</style>
<div id="test-content">

<div id="first" tabindex="1">1. First sequentially focusable element</div>

<div id="shadow-without-tabindex">
A non-focusable shadow host should not be focused.
<div slot="slot" tabindex="0">5. Slotted content with tabindex=0</div>
<div slot="slot" tabindex="3">4. Slotted content with tabindex=4</div>
</div>

<div id="shadow-with-multiple-slots">
A non-focusable shadow host should not be focused.
<div slot="slot2"><div tabindex="2">11. Content in slot 2 with tabindex=1</div></div>
<div slot="slot2" tabindex="1">10. Content in slot 2 with tabindex=1</div>
<div slot="slot2" tabindex="0">12. Content in slot 2 with tabindex=0</div>
<div slot="slot1" tabindex="0">9. Content in slot 1 with tabindex=0</div>
<div slot="slot1" tabindex="7">8. Content in slot 1 with tabindex=7</div>
</div>

<div id="shadow-with-slot-fallback">
A non-focusable shadow host should not be focused.
</div>

</div>
<pre></pre>
<script>

var oldActiveElement = null;
function log()
{
setTimeout(function () {
var newActiveElement = document.activeElement;

var shadowRoot = newActiveElement.shadowRoot || newActiveElement.closedShadowRoot;
if (shadowRoot) {
var activeElementInShadow = shadowRoot.activeElement;
if (activeElementInShadow)
newActiveElement = activeElementInShadow;
}

if (oldActiveElement == newActiveElement || newActiveElement == document.body)
return;
oldActiveElement = newActiveElement;
document.querySelector('pre').textContent += newActiveElement.firstChild.textContent.trim() + '\n';
}, 0);
}

if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

window.onload = function () {
document.onkeydown = log;

for (var element of document.querySelectorAll('#test-content *')) {
if (element instanceof HTMLIFrameElement)
element.contentDocument.onkeydown = log;
else
element.onfocus = log;
}

var hostWithoutTabIndex = document.getElementById('shadow-without-tabindex');
hostWithoutTabIndex.closedShadowRoot = hostWithoutTabIndex.attachShadow({mode: 'closed'});
hostWithoutTabIndex.closedShadowRoot.innerHTML = `
<div tabindex="2" onfocus="log(this)">3. The focusable element in shadow tree with the lowest tabindex</div>
<slot name="slot" onfocus="log(this)">Non-focusable slot element should not be focused</slot>
<div tabindex="1" onfocus="log(this)">2. The focusable element in shadow tree with the higehst tabindex</div>`;

var hostWithMultipleSlots = document.getElementById('shadow-with-multiple-slots');
hostWithMultipleSlots.closedShadowRoot = hostWithMultipleSlots.attachShadow({mode: 'closed'});
hostWithMultipleSlots.closedShadowRoot.innerHTML = `
<slot name="slot1" tabindex="2" onfocus="log(this)" style="display:block;">7. Focusable slot 1</slot>
<div tabindex="1" onfocus="log(this)">6. The focusable element in shadow tree with the higehst tabindex</div>
<slot name="slot2" onfocus="log(this)">Non-focusable slot 2 should not be focused</slot>`;

var shadowWithSlotFallback = document.getElementById('shadow-with-slot-fallback');
shadowWithSlotFallback.closedShadowRoot = shadowWithSlotFallback.attachShadow({mode: 'closed'});
shadowWithSlotFallback.closedShadowRoot.innerHTML = `
<slot name="slot1" onfocus="log(this)">
Non-focusable slot should not be focused.
<div tabindex="0">16. Non-focusable slot fallback with tabindex=0</div>
<div tabindex="1">13. Non-focusable slot fallback with tabindex=1</div>
</slot>
<div tabindex="2" onfocus="log(this)">15. Shadow content with tabindex=2</div>
<slot name="slot2" tabindex="1" style="display:block;" onfocus="log(this)">
14. Focusable slot element.
<div tabindex="0">17. Focusable slot fallback content with tabindex=0</div>
</slot>`;

document.getElementById('first').focus();
document.onkeydown();

if (window.eventSender)
moveFocusForward(16);
}

function moveFocusForward(focusCount)
{
setTimeout(function () {
if (!focusCount)
return moveFocusBackward(16);
eventSender.keyDown('\t');
setTimeout(function () {
moveFocusForward(focusCount - 1);
}, 1);
}, 1);
}

function moveFocusBackward(focusCount)
{
setTimeout(function () {
if (!focusCount) {
document.getElementById('test-content').style.display = 'none';
testRunner.notifyDone();
return;
}
eventSender.keyDown('\t', ['shiftKey']);
setTimeout(function () {
moveFocusBackward(focusCount - 1);
}, 1);
}, 1);
}

</script>
</body>
</html>
2 changes: 2 additions & 0 deletions LayoutTests/platform/ios-simulator/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ fast/text/text-disappear-on-deselect.html [ ImageOnlyFailure ]
webkit.org/b/148695 fast/shadow-dom [ Pass ]

# No tab navigation support on iOS
fast/shadow-dom/focus-across-details-element.html [ Failure ]
fast/shadow-dom/focus-navigation-across-slots.html [ Failure ]
fast/shadow-dom/focus-on-iframe.html [ Failure ]
fast/shadow-dom/negative-tabindex-on-shadow-host.html [ Failure ]

Expand Down
74 changes: 74 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,77 @@
2016-05-16 Ryosuke Niwa <rniwa@webkit.org>

Focus ordering should respect slot elements
https://bugs.webkit.org/show_bug.cgi?id=151379

Reviewed by Antti Koivisto.

Implemented the sequential focus navigation ordering as discussed on
https://github.com/w3c/webcomponents/issues/375

New behavior treats each shadow root and slot as a "focus scope". The focus navigation ordering
is defined within each "focus scope" using tabindex, treating any "focus scope owner"
(e.g. shadow host or a slot) as if it was having tabindex=0 if it wasn't itself focusable.

This patch modifies FocusNavigationScope to support a focus scope defined for a slot element in
addition to the one defined for a shadow tree and a document as previously supported.

Tests: fast/shadow-dom/focus-across-details-element.html
fast/shadow-dom/focus-navigation-across-slots.html

* dom/Node.cpp:
(WebCore::parentShadowRoot): Extracted from assignedSlot.
(WebCore::Node::assignedSlot):
(WebCore::Node::assignedSlotForBindings): Added.
* dom/Node.h:
* dom/NonDocumentTypeChildNode.idl:
* html/HTMLDetailsElement.h:
(HTMLDetailsElement::hasCustomFocusLogic): Added. Don't treat details element as a "focus scope".
* html/HTMLSummaryElement.h:
(HTMLSummaryElement::hasCustomFocusLogic): Ditto for summary element.
* page/FocusController.cpp:
(WebCore::hasCustomFocusLogic): Moved.
(WebCore::isFocusScopeOwner): Added. Returns true on a shadow host without a custom focus logic or
on a slot inside a shadow tree whose shadow host doesn't have a custom focus logic.
(WebCore::FocusNavigationScope::firstChildInScope): Now takes a reference. Call isFocusScopeOwner
to check for both slots and shadow roots instead of just the latter. This fixes a subtle bug that
focus may never get out of textarea in some cases due to its failure to check hasCustomFocusLogic.
(WebCore::FocusNavigationScope::lastChildInScope): Ditto.
(WebCore::FocusNavigationScope::parentInScope): Made this a member function since it needs to check
against m_slotElement inside the focus scope of a slot.
(WebCore::FocusNavigationScope::nextSiblingInScope): Added. Finds the next assigned node in a slot
in the focus scope defined for a slot. Just calls nextSibling() in the focus scope for shadow tree
and document.
(WebCore::FocusNavigationScope::previousSiblingInScope): Ditto for finding the previous sibling.
(WebCore::FocusNavigationScope::firstNodeInScope): Added. This function replaces rootNode() which
doesn't exist for the focus scope of a slot element.
(WebCore::FocusNavigationScope::lastNodeInScope): Ditto for the last node.
(WebCore::FocusNavigationScope::nextInScope):
(WebCore::FocusNavigationScope::previousInScope):
(WebCore::FocusNavigationScope::FocusNavigationScope): Added a variant that takes HTMLSlotElement.
(WebCore::FocusNavigationScope::owner): Added the support for slot elements.
(WebCore::FocusNavigationScope::scopeOf): Ditto.
(WebCore::FocusNavigationScope::scopeOwnedByScopeOwner): Ditto.
(WebCore::isFocusableElementOrScopeOwner): Added the support for slot elements and renamed from
isFocusableOrHasShadowTreeWithoutCustomFocusLogic.
(WebCore::isNonFocusableScopeOwner): Ditto. Renamed from isNonFocusableShadowHost.
(WebCore::isFocusableScopeOwner): Ditto. Renamed from isFocusableShadowHost.
(WebCore::shadowAdjustedTabIndex): Added the support for slot elements.
(WebCore::FocusController::findFocusableElementAcrossFocusScope):
(WebCore::FocusController::nextFocusableElementWithinScope):
(WebCore::FocusController::previousFocusableElementWithinScope):
(WebCore::FocusController::findElementWithExactTabIndex):
(WebCore::nextElementWithGreaterTabIndex): Call firstNodeInScope() instead of rootNode() here since
there is no root node for the focus scope defined for a slot element.
(WebCore::previousElementWithLowerTabIndex): Ditto for scope.lastNodeInScope().
(WebCore::FocusController::nextFocusableElementOrScopeOwner):
(WebCore::FocusController::previousFocusableElementOrScopeOwner):
(WebCore::parentInScope): Deleted.
(WebCore::FocusNavigationScope::rootNode): Deleted.
(WebCore::FocusNavigationScope::scopeOwnedByShadowHost): Deleted.
(WebCore::isNonFocusableShadowHost): Deleted.
(WebCore::isFocusableShadowHost): Deleted.
(WebCore::isFocusableOrHasShadowTreeWithoutCustomFocusLogic): Deleted.

2016-05-16 Chris Dumez <cdumez@apple.com>

Use WTF::Optional for ScrollView's m_deferredScrollDelta / m_deferredScrollOffsets
Expand Down
Loading

0 comments on commit 38df02f

Please sign in to comment.