Bug 1544826 - Fix tab navigation when document element is a shadow host. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 17 Apr 2019 10:13:33 +0000
changeset 469810 8d0276626d4f
parent 469809 bcd85ff387fb
child 469811 e0f7fc19d5cb
push id112825
push usercbrindusan@mozilla.com
push dateWed, 17 Apr 2019 15:58:37 +0000
treeherdermozilla-inbound@7bd43da7830c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1544826
milestone68.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 1544826 - Fix tab navigation when document element is a shadow host. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D27736
dom/base/nsFocusManager.cpp
dom/base/test/mochitest.ini
dom/base/test/test_focus_shadow_dom_root.html
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -2987,25 +2987,16 @@ void ScopedContentTraversal::Prev() {
 /**
  * Returns scope owner of aContent.
  * A scope owner is either a document root, shadow host, or slot.
  */
 static nsIContent* FindOwner(nsIContent* aContent) {
   nsIContent* currentContent = aContent;
   while (currentContent) {
     nsIContent* parent = currentContent->GetFlattenedTreeParent();
-    if (!parent) {
-      // Document root
-      Document* doc = currentContent->GetUncomposedDoc();
-      if (doc && doc->GetRootElement() == currentContent) {
-        return currentContent;
-      }
-
-      break;
-    }
 
     // Shadow host / Slot
     if (IsHostOrSlot(parent)) {
       return parent;
     }
 
     currentContent = parent;
   }
@@ -3253,37 +3244,34 @@ nsresult nsFocusManager::GetNextTabbable
         aForward ? 1 : 0, aIgnoreTabIndex, aForDocumentNavigation,
         true /* aSkipOwner */);
     if (contentToFocus) {
       NS_ADDREF(*aResultContent = contentToFocus);
       return NS_OK;
     }
   }
 
-  // If aStartContent is not in a scope owned by the root element
-  // (i.e. aStartContent is already in shadow DOM),
-  // search from scope including aStartContent
-  nsIContent* rootElement = aRootContent->OwnerDoc()->GetRootElement();
-  nsIContent* owner = FindOwner(aStartContent);
-  if (owner && rootElement != owner) {
+  // If aStartContent is in a scope owned by Shadow DOM search from scope
+  // including aStartContent
+  if (nsIContent* owner = FindOwner(aStartContent)) {
     nsIContent* contentToFocus = GetNextTabbableContentInAncestorScopes(
         owner, &aStartContent, aOriginalStartContent, aForward,
         &aCurrentTabIndex, aIgnoreTabIndex, aForDocumentNavigation);
     if (contentToFocus) {
       NS_ADDREF(*aResultContent = contentToFocus);
       return NS_OK;
     }
   }
 
   // If we reach here, it means no next tabbable content in shadow DOM.
   // We need to continue searching in light DOM, starting at the top level
   // shadow host in light DOM (updated aStartContent) and its tabindex
   // (updated aCurrentTabIndex).
-  MOZ_ASSERT(FindOwner(aStartContent) == rootElement,
-             "aStartContent should be owned by the root element at this point");
+  MOZ_ASSERT(!FindOwner(aStartContent),
+             "aStartContent should not be owned by Shadow DOM at this point");
 
   nsPresContext* presContext = aPresShell->GetPresContext();
 
   bool getNextFrame = true;
   nsCOMPtr<nsIContent> iterStartContent = aStartContent;
   // Iterate tab index to find corresponding contents
   while (1) {
     nsIFrame* frame = iterStartContent->GetPrimaryFrame();
@@ -3432,17 +3420,17 @@ nsresult nsFocusManager::GetNextTabbable
 
       // As of now, 2018/04/12, sequential focus navigation is still
       // in the obsolete Shadow DOM specification.
       // http://w3c.github.io/webcomponents/spec/shadow/#sequential-focus-navigation
       // "if ELEMENT is focusable, a shadow host, or a slot element,
       //  append ELEMENT to NAVIGATION-ORDER."
       // and later in "For each element ELEMENT in NAVIGATION-ORDER: "
       // hosts and slots are handled before other elements.
-      if (currentContent && IsHostOrSlot(currentContent)) {
+      if (IsHostOrSlot(currentContent)) {
         bool focusableHostSlot;
         int32_t tabIndex =
             HostOrSlotTabIndexValue(currentContent, &focusableHostSlot);
         // Host or slot itself isn't focusable or going backwards, enter its
         // scope.
         if ((!aForward || !focusableHostSlot) && tabIndex >= 0 &&
             (aIgnoreTabIndex || aCurrentTabIndex == tabIndex)) {
           nsIContent* contentToFocus = GetNextTabbableContentInScope(
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -615,17 +615,17 @@ skip-if = toolkit == 'android'
 [test_bug1375050.html]
 [test_bug1381710.html]
 [test_bug1384661.html]
 [test_bug1399605.html]
 [test_bug1404385.html]
 [test_bug1406102.html]
 [test_bug1421568.html]
 [test_bug1453693.html]
-skip-if = os == "mac"
+skip-if = os == "mac" # Different tab focus behavior on mac
 [test_bug1472427.html]
 [test_bug1499169.html]
 skip-if = toolkit == 'android' # Timeouts on android due to page closing issues with embedded pdf
 [test_caretPositionFromPoint.html]
 [test_change_policy.html]
 [test_clearTimeoutIntervalNoArg.html]
 [test_constructor-assignment.html]
 [test_constructor.html]
@@ -663,16 +663,18 @@ skip-if = toolkit == 'android' && !is_fe
 [test_EventSource_redirects.html]
 [test_eventsource_event_listener_leaks.html]
 [test_explicit_user_agent.html]
 skip-if = (toolkit == 'android') # Android: Bug 775227
 [test_find.html]
 skip-if = (toolkit == 'android') # Android: Bug 1465387
 [test_find_nac.html]
 skip-if = (toolkit == 'android') # Android: Bug 1465387
+[test_focus_shadow_dom_root.html]
+skip-if = os == "mac" # Different tab focus behavior on mac
 [test_getAttribute_after_createAttribute.html]
 [test_getElementById.html]
 [test_getTranslationNodes.html]
 [test_getTranslationNodes_limit.html]
 [test_gsp-qualified.html]
 [test_gsp-quirks.html]
 [test_gsp-standards.html]
 [test_history_document_open.html]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_focus_shadow_dom_root.html
@@ -0,0 +1,36 @@
+<!doctype html>
+<title>Test for bug 1544826</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<div id="host"><a href="#" id="slotted">This is focusable too</a></div>
+<script>
+  const host = document.getElementById("host");
+  const shadow = host.attachShadow({ mode: "open" });
+  shadow.innerHTML = `
+    <a id="shadow-1" href="#">This is focusable</a>
+    <slot></slot>
+    <a id="shadow-2" href="#">So is this</a>
+  `;
+  document.documentElement.remove();
+  document.appendChild(host);
+
+  SimpleTest.waitForExplicitFinish();
+  SimpleTest.waitForFocus(function() {
+    is(document.documentElement, host, "Host is the document element");
+    host.offsetTop;
+    synthesizeKey("KEY_Tab");
+    is(shadow.activeElement.id, "shadow-1", "First link in Shadow DOM is focused");
+    synthesizeKey("KEY_Tab");
+    is(document.activeElement.id, "slotted", "Slotted link is focused");
+    synthesizeKey("KEY_Tab");
+    is(shadow.activeElement.id, "shadow-2", "Second link in Shadow DOM is focused");
+
+    // Now backwards.
+    synthesizeKey("KEY_Tab", {shiftKey: true});
+    is(document.activeElement.id, "slotted", "Backwards: Slotted link is focused");
+    synthesizeKey("KEY_Tab", {shiftKey: true});
+    is(shadow.activeElement.id, "shadow-1", "Backwards: First slotted link is focused");
+
+    SimpleTest.finish();
+  });
+</script>