Bug 1558393 - Don't keep top level scope owner when going backwards. r=smaug, a=jcristau
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 25 Jun 2019 10:35:39 +0000
changeset 537095 f4a9a2f7f1d49ee2cafb3b7f959bf5fe3039e830
parent 537094 d996ccc0aa7eab043a8b926c3dffcd7d899530b6
child 537096 ab3fb204627cc68b1a3d0b4ec2b2405a84ae772d
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, jcristau
bugs1558393
milestone68.0
Bug 1558393 - Don't keep top level scope owner when going backwards. r=smaug, a=jcristau When going backwards the host is the last thing of the scope we see, not the first, so we need to make sure to properly switch to the document scope. To be clear, the order this is happening when bogusly going backwards from `second` to `first` is: * We're doing frame traversal so start off the <div>. * The text is the first frame in preorder we find, we figure out that the current top-level owner is the host. But it's not focusable so we carry on. * Now we go to the next frame and find the <span>. `oldTopLevelScopeOwner` is still the span itself, and we don't change it (that's what my patch fixes), so we go into the "We're within non-document scope, continue", and thus skip trying to focus the host. With this fix we actually realize that the current top level scope owner has changed and thus don't skip it and try to focus the slot properly. Differential Revision: https://phabricator.services.mozilla.com/D35585
dom/base/nsFocusManager.cpp
dom/base/test/file_bug1453693.html
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -3359,17 +3359,17 @@ nsresult nsFocusManager::GetNextTabbable
     }
 
     // Walk frames to find something tabbable matching aCurrentTabIndex
     while (frame) {
       // Try to find the topmost scope owner, since we want to skip the node
       // that is not owned by document in frame traversal.
       nsIContent* currentContent = frame->GetContent();
       nsIContent* oldTopLevelScopeOwner = currentTopLevelScopeOwner;
-      if (oldTopLevelScopeOwner != currentContent) {
+      if (!aForward || oldTopLevelScopeOwner != currentContent) {
         currentTopLevelScopeOwner = GetTopLevelScopeOwner(currentContent);
       } else {
         currentTopLevelScopeOwner = currentContent;
       }
       if (currentTopLevelScopeOwner) {
         if (currentTopLevelScopeOwner == oldTopLevelScopeOwner) {
           // We're within non-document scope, continue.
           do {
--- a/dom/base/test/file_bug1453693.html
+++ b/dom/base/test/file_bug1453693.html
@@ -826,30 +826,65 @@
         // Remove elements added to body element.
         host1.remove();
         input.remove();
 
         // Tests expect body.firstChild to have focus.
         document.body.firstChild.focus();
       }
 
+      // Bug 1558393
+      function testBackwardsTabbingWithSlotsWithoutFocusableContent() {
+        let first = document.createElement("div");
+        first.tabIndex = 0;
+        let host = document.createElement("div");
+        host.tabIndex = 0;
+        let second = document.createElement("div");
+        second.tabIndex = 0;
+        host.appendChild(document.createTextNode("foo"));
+        host.attachShadow({ mode: "open" }).innerHTML = `<slot></slot>`;
+
+        document.body.appendChild(first);
+        document.body.appendChild(host);
+        document.body.appendChild(second);
+        document.body.offsetLeft;
+
+        first.focus();
+        opener.is(document.activeElement, first, "First light div should have focus");
+        synthesizeKey("KEY_Tab");
+        opener.is(document.activeElement, host, "Host should be focused");
+        synthesizeKey("KEY_Tab");
+        opener.is(document.activeElement, second, "Second light div should be focused");
+
+        // Now backwards
+        synthesizeKey("KEY_Tab", {shiftKey: true});
+        opener.is(document.activeElement, host, "Focus should return to host");
+        synthesizeKey("KEY_Tab", {shiftKey: true});
+        opener.is(document.activeElement, first, "Focus should return to first light div");
+
+        second.remove();
+        host.remove();
+        first.remove();
+      }
+
       function runTest() {
 
         testTabbingThroughShadowDOMWithTabIndexes();
         testTabbingThroughSimpleShadowDOM();
         testTabbingThroughNestedShadowDOM();
         testTabbingThroughDisplayContentsHost();
         testTabbingThroughLightDOMShadowDOMLightDOM();
         testFocusableHost();
         testShiftTabbingThroughFocusableHost();
         testTabbingThroughNestedSlot();
         testTabbingThroughSlotInLightDOM();
         testTabbingThroughFocusableSlotInLightDOM();
         testTabbingThroughScrollableShadowDOM();
         testDeeplyNestedShadowTree();
+        testBackwardsTabbingWithSlotsWithoutFocusableContent();
 
         opener.didRunTests();
         window.close();
       }
 
       function init() {
         SimpleTest.waitForFocus(runTest);
       }