Bug 1558393 - Don't keep top level scope owner when going backwards. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 25 Jun 2019 10:35:39 +0000
changeset 542854 2c3402f0d512cffd7dfe99f486a4d4363b7822ec
parent 542853 f14d8d4a913ee7ef995f39c3da530969438a860d
child 542855 f779b952db5e751a19f45ed17bc7b2f9bcc6cce4
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1558393
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1558393 - Don't keep top level scope owner when going backwards. r=smaug 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
@@ -3365,17 +3365,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);
       }