Moving focus by tab could erroneously focus a non-focusable shadow host
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 May 2016 22:42:23 +0000 (22:42 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 May 2016 22:42:23 +0000 (22:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157585

Reviewed by Antti Koivisto.

Source/WebCore:

The bug was caused by findFocusableElementDescendingDownIntoFrameDocument erroneously returning a shadow host
that contains a focusable element instead of traversing it through to find a focusable element within. Fixed
the bug calling findFocusableElementWithinScope which traverses shadow trees to find a focusable element unlike
findFocusableElementOrScopeOwner which returns a focusable element or a shadow host.

Also done some refactoring for clarity.

Test: fast/shadow-dom/focus-on-iframe.html

* page/FocusController.cpp:
(WebCore::FocusController::findFocusableElementDescendingDownIntoFrameDocument): See above.

(WebCore::FocusController::findFocusableElementAcrossFocusScope): Declare outerScope as late as possible.
(WebCore::nextElementWithGreaterTabIndex): Merged if conditions for clarity.
(WebCore::previousElementWithLowerTabIndex): Removed the check for isNonFocusableShadowHost since
isFocusableOrHasShadowTreeWithoutCustomFocusLogic returns true whenever isNonFocusableShadowHost returns true.

LayoutTests:

Added a regression test for moving focus across iframes.

Also expanded negative-tabindex-on-shadow-host.html to cover reverse traversal.

* fast/shadow-dom/focus-on-iframe-expected.txt: Added.
* fast/shadow-dom/focus-on-iframe.html: Added.
* fast/shadow-dom/negative-tabindex-on-shadow-host-expected.txt:
* fast/shadow-dom/negative-tabindex-on-shadow-host.html:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@200712 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/focus-on-iframe-expected.txt [new file with mode: 0644]
LayoutTests/fast/shadow-dom/focus-on-iframe.html [new file with mode: 0644]
LayoutTests/fast/shadow-dom/negative-tabindex-on-shadow-host-expected.txt
LayoutTests/fast/shadow-dom/negative-tabindex-on-shadow-host.html
Source/WebCore/ChangeLog
Source/WebCore/page/FocusController.cpp

index 478bd18..ef36c5a 100644 (file)
@@ -1,3 +1,19 @@
+2016-05-11  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Moving focus by tab could erroneously focus a non-focusable shadow host
+        https://bugs.webkit.org/show_bug.cgi?id=157585
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test for moving focus across iframes.
+
+        Also expanded negative-tabindex-on-shadow-host.html to cover reverse traversal.
+
+        * fast/shadow-dom/focus-on-iframe-expected.txt: Added.
+        * fast/shadow-dom/focus-on-iframe.html: Added.
+        * fast/shadow-dom/negative-tabindex-on-shadow-host-expected.txt:
+        * fast/shadow-dom/negative-tabindex-on-shadow-host.html:
+
 2016-05-11  Darin Adler  <darin@apple.com>
 
         Change IDBObjectStore.createIndex to take an IDL dictionary
diff --git a/LayoutTests/fast/shadow-dom/focus-on-iframe-expected.txt b/LayoutTests/fast/shadow-dom/focus-on-iframe-expected.txt
new file mode 100644 (file)
index 0000000..5c2b9c9
--- /dev/null
@@ -0,0 +1,21 @@
+Tests for setting a negative tabindex on shadow host. Elements inside such a shadow tree should not be in the sequential navigation order.
+To manually test, press tab key four times. It should traverse focusable elements in the increasing numerical order.
+
+1. First sequentially focusable element
+2. An iframe without a focusable element
+3. First focusable element inside an iframe
+4. Second focusable element inside an iframe
+5. An iframe without a focusable shadow host or shadow content
+6. Focusable content inside a shadow tree
+7. A focusable element with a high tabindex
+8. Focusable content inside the first shadow tree
+9. Focusable content inside the second shadow tree
+8. Focusable content inside the first shadow tree
+7. A focusable element with a high tabindex
+6. Focusable content inside a shadow tree
+5. An iframe without a focusable shadow host or shadow content
+4. Second focusable element inside an iframe
+3. First focusable element inside an iframe
+2. An iframe without a focusable element
+1. First sequentially focusable element
+
diff --git a/LayoutTests/fast/shadow-dom/focus-on-iframe.html b/LayoutTests/fast/shadow-dom/focus-on-iframe.html
new file mode 100644 (file)
index 0000000..8da24b1
--- /dev/null
@@ -0,0 +1,115 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>Tests for setting a negative tabindex on shadow host. Elements inside such a shadow tree should not be in the sequential navigation order.<br>
+To manually test, press tab key four times. It should traverse focusable elements in the increasing numerical order.</p>
+<div id="test-content">
+<div id="first" tabindex="1">1. First sequentially focusable element</div>
+<iframe src="about:blank">2. An iframe without a focusable element</iframe>
+<iframe src="about:blank">An iframe with a focusable element should not be focused</iframe>
+<iframe src="about:blank">5. An iframe without a focusable shadow host or shadow content</iframe>
+<iframe src="about:blank">An iframe with a focusable shadow content should not be focused (1)</iframe>
+<iframe src="about:blank">An iframe with a focusable shadow content should not be focused (2)</iframe>
+</div>
+<pre></pre>
+<script>
+
+var oldActiveElement = null;
+function log()
+{
+    setTimeout(function () {
+        var newActiveElement = document.activeElement;
+
+        if (newActiveElement instanceof HTMLIFrameElement) {
+            var contentDocument = newActiveElement.contentDocument;
+            var activeElementInsideFrame = contentDocument.activeElement;
+            if (activeElementInsideFrame != contentDocument.body && activeElementInsideFrame != contentDocument.documentElement /* Firefox */)
+                newActiveElement = activeElementInsideFrame;
+        }
+
+        if (newActiveElement.shadowRoot) {
+            var activeElementInShadow = newActiveElement.shadowRoot.activeElement;
+            if (activeElementInShadow)
+                newActiveElement = activeElementInShadow;
+        }
+
+        if (oldActiveElement == newActiveElement || newActiveElement == document.body)
+            return;
+        oldActiveElement = newActiveElement;
+        document.querySelector('pre').textContent += newActiveElement.textContent + '\n';
+    }, 0);
+}
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+window.onload = function () {
+    document.onkeydown = log;
+
+    var iframes = document.querySelectorAll('iframe');
+    for (var i = 0; i < iframes.length; i++)
+        iframes[i].contentDocument.onkeydown = log;
+
+    iframes[0].contentDocument.body.innerHTML = 'Content of an iframe without a focusable element should not be focused';
+    iframes[1].contentDocument.body.innerHTML = `
+        <div tabindex="0" onfocus="top.log(this)">3. First focusable element inside an iframe</div>
+        <div tabindex="0" onfocus="top.log(this)">4. Second focusable element inside an iframe</div>`;
+
+    iframes[2].contentDocument.body.innerHTML = '<div>A non-focusable shadow host without a focusable content should not be focused</div>';
+    var host = iframes[2].contentDocument.body.querySelector('div');
+    var shadowRoot = host.attachShadow({mode: 'closed'});
+    shadowRoot.innerHTML = '<slot></slot>';
+
+    iframes[3].contentDocument.body.innerHTML = '<div></div>';
+    var host = iframes[3].contentDocument.querySelector('div');
+    var shadowRoot = host.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<div tabindex="0" onfocus="top.log(this)">6. Focusable content inside a shadow tree</div>';
+
+    iframes[4].contentDocument.body.innerHTML = '<div></div><i tabindex="1">7. A focusable element with a high tabindex</i><span></span>';
+    host = iframes[4].contentDocument.querySelector('div');
+    shadowRoot = host.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<div tabindex="0" onfocus="top.log(this)">8. Focusable content inside the first shadow tree</div>';
+
+    host = iframes[4].contentDocument.querySelector('span');
+    shadowRoot = host.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<div tabindex="0" onfocus="top.log(this)">9. Focusable content inside the second shadow tree</div>';
+
+    document.getElementById('first').focus();
+    document.onkeydown();
+
+    if (window.eventSender)
+        moveFocusForward(8);
+}
+
+function moveFocusForward(focusCount)
+{
+    setTimeout(function () {
+        if (!focusCount)
+            return moveFocusBackward(8);
+        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>
index 1551d4e..2b22b9b 100644 (file)
@@ -7,4 +7,9 @@ To manually test, press tab key four times. It should traverse focusable element
 4.1. Focusable element inside a shadow host with no tabindex
 4.2. Shadow host with no tabindex
 5. Last sequentially focusable element outside shadow trees
+4.1. Focusable element inside a shadow host with no tabindex
+4.2. Shadow host with no tabindex
+3. Focusable element inside a shadow host with a positive tabindex
+2. Shadow host with a positive tabindex
+1. First sequentially focusable element outside shadow trees
 
index 5398607..6c62315 100644 (file)
@@ -40,6 +40,11 @@ if (window.eventSender) {
     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']);
     document.getElementById('test-content').style.display = 'none';
 }
 
index 8493bee..a5f1013 100644 (file)
@@ -1,3 +1,27 @@
+2016-05-11  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Moving focus by tab could erroneously focus a non-focusable shadow host
+        https://bugs.webkit.org/show_bug.cgi?id=157585
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by findFocusableElementDescendingDownIntoFrameDocument erroneously returning a shadow host
+        that contains a focusable element instead of traversing it through to find a focusable element within. Fixed
+        the bug calling findFocusableElementWithinScope which traverses shadow trees to find a focusable element unlike
+        findFocusableElementOrScopeOwner which returns a focusable element or a shadow host.
+
+        Also done some refactoring for clarity.
+
+        Test: fast/shadow-dom/focus-on-iframe.html
+
+        * page/FocusController.cpp:
+        (WebCore::FocusController::findFocusableElementDescendingDownIntoFrameDocument): See above.
+
+        (WebCore::FocusController::findFocusableElementAcrossFocusScope): Declare outerScope as late as possible.
+        (WebCore::nextElementWithGreaterTabIndex): Merged if conditions for clarity.
+        (WebCore::previousElementWithLowerTabIndex): Removed the check for isNonFocusableShadowHost since
+        isFocusableOrHasShadowTreeWithoutCustomFocusLogic returns true whenever isNonFocusableShadowHost returns true.
+
 2016-05-11  Ryan Haddad  <ryanhaddad@apple.com>
 
         Updating bindings tests results after r200699
index 270d4f9..3fa23c4 100644 (file)
@@ -300,8 +300,7 @@ Element* FocusController::findFocusableElementDescendingDownIntoFrameDocument(Fo
         HTMLFrameOwnerElement& owner = downcast<HTMLFrameOwnerElement>(*element);
         if (!owner.contentFrame())
             break;
-        // FIXME: This can return a non-focusable shadow root.
-        Element* foundElement = findFocusableElementOrScopeOwner(direction, FocusNavigationScope::scopeOwnedByIFrame(owner), 0, event);
+        Element* foundElement = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByIFrame(owner), nullptr, event);
         if (!foundElement)
             break;
         ASSERT(element != foundElement);
@@ -433,9 +432,10 @@ Element* FocusController::findFocusableElementAcrossFocusScope(FocusDirection di
     // If there's no focusable node to advance to, move up the focus scopes until we find one.
     Element* owner = scope.owner();
     while (owner) {
-        FocusNavigationScope outerScope = FocusNavigationScope::scopeOf(*owner);
         if (direction == FocusDirectionBackward && isFocusableShadowHost(*owner, *event))
             return findFocusableElementDescendingDownIntoFrameDocument(direction, owner, event);
+
+        auto outerScope = FocusNavigationScope::scopeOf(*owner);
         if (Element* candidateInOuterScope = findFocusableElementWithinScope(direction, outerScope, owner, event))
             return candidateInOuterScope;
         owner = outerScope.owner();
@@ -514,11 +514,9 @@ static Element* nextElementWithGreaterTabIndex(const FocusNavigationScope& scope
             continue;
         Element& candidate = downcast<Element>(*node);
         int candidateTabIndex = candidate.tabIndex();
-        if (isFocusableOrHasShadowTreeWithoutCustomFocusLogic(candidate, event) && candidateTabIndex > tabIndex) {
-            if (!winner || candidateTabIndex < winningTabIndex) {
-                winner = &candidate;
-                winningTabIndex = candidateTabIndex;
-            }
+        if (isFocusableOrHasShadowTreeWithoutCustomFocusLogic(candidate, event) && candidateTabIndex > tabIndex && (!winner || candidateTabIndex < winningTabIndex)) {
+            winner = &candidate;
+            winningTabIndex = candidateTabIndex;
         }
     }
 
@@ -535,7 +533,7 @@ static Element* previousElementWithLowerTabIndex(const FocusNavigationScope& sco
             continue;
         Element& element = downcast<Element>(*node);
         int currentTabIndex = shadowAdjustedTabIndex(element, event);
-        if ((isFocusableOrHasShadowTreeWithoutCustomFocusLogic(element, event) || isNonFocusableShadowHost(element, event)) && currentTabIndex < tabIndex && currentTabIndex > winningTabIndex) {
+        if (isFocusableOrHasShadowTreeWithoutCustomFocusLogic(element, event) && currentTabIndex < tabIndex && currentTabIndex > winningTabIndex) {
             winner = &element;
             winningTabIndex = currentTabIndex;
         }