Bug 1507101 - Use StyleChildrenIterator instead of custom frame tree walking code to handle NAC inside shadow dom. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 16 Nov 2018 20:31:50 +0000
changeset 503352 5322e59f933edee69fa4a848e1709d7c21a4552b
parent 503351 7e6b465b73dc86606661742771441bceea83e310
child 503353 a92c330bac8bce0c51099d630349aecbb71d7c35
child 503359 51abb63ca2464f588229752a5c6070f459a74e6b
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1507101
milestone65.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 1507101 - Use StyleChildrenIterator instead of custom frame tree walking code to handle NAC inside shadow dom. r=smaug There are lots of frames which create anonymous content (like scrollframes, which generate scrollbars) but for which this code was wrong. Use StyleChildrenIterator which has a defined order between NAC and flattened tree. I've verified this doesn't break tabbing through input type="date" with UA widget disabled, fwiw. Differential Revision: https://phabricator.services.mozilla.com/D12152
dom/base/nsFocusManager.cpp
dom/base/test/file_bug1453693.html
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -3095,17 +3095,17 @@ private:
 void
 ScopedContentTraversal::Next()
 {
   MOZ_ASSERT(mCurrent);
 
   // Get mCurrent's first child if it's in the same scope.
   if (!(mCurrent->GetShadowRoot() || mCurrent->IsHTMLElement(nsGkAtoms::slot)) ||
       mCurrent == mOwner) {
-    FlattenedChildIterator iter(mCurrent);
+    StyleChildrenIterator iter(mCurrent);
     nsIContent* child = iter.GetNextChild();
     if (child) {
       SetCurrent(child);
       return;
     }
   }
 
   // If mOwner has no children, END traversal
@@ -3113,22 +3113,21 @@ ScopedContentTraversal::Next()
     SetCurrent(nullptr);
     return;
   }
 
   nsIContent* current = mCurrent;
   while (1) {
     // Create parent's iterator and move to current
     nsIContent* parent = current->GetFlattenedTreeParent();
-    FlattenedChildIterator parentIter(parent);
+    StyleChildrenIterator parentIter(parent);
     parentIter.Seek(current);
 
     // Get next sibling of current
-    nsIContent* next = parentIter.GetNextChild();
-    if (next) {
+    if (nsIContent* next = parentIter.GetNextChild()) {
       SetCurrent(next);
       return;
     }
 
     // If no next sibling and parent is mOwner, END traversal
     if (parent == mOwner) {
       SetCurrent(nullptr);
       return;
@@ -3142,40 +3141,40 @@ void
 ScopedContentTraversal::Prev()
 {
   MOZ_ASSERT(mCurrent);
 
   nsIContent* parent;
   nsIContent* last;
   if (mCurrent == mOwner) {
     // Get last child of mOwner
-    FlattenedChildIterator ownerIter(mOwner, false /* aStartAtBeginning */);
+    StyleChildrenIterator ownerIter(mOwner, false /* aStartAtBeginning */);
     last = ownerIter.GetPreviousChild();
 
     parent = last;
   } else {
     // Create parent's iterator and move to mCurrent
     parent = mCurrent->GetFlattenedTreeParent();
-    FlattenedChildIterator parentIter(parent);
+    StyleChildrenIterator parentIter(parent);
     parentIter.Seek(mCurrent);
 
     // Get previous sibling
     last = parentIter.GetPreviousChild();
   }
 
   while (last) {
     parent = last;
     if (parent->GetShadowRoot() ||
         parent->IsHTMLElement(nsGkAtoms::slot)) {
       // Skip contents in other scopes
       break;
     }
 
     // Find last child
-    FlattenedChildIterator iter(parent, false /* aStartAtBeginning */);
+    StyleChildrenIterator iter(parent, false /* aStartAtBeginning */);
     last = iter.GetPreviousChild();
   }
 
   // If parent is mOwner and no previous sibling remains, END traversal
   SetCurrent(parent == mOwner ? nullptr : parent);
 }
 
 nsIContent*
@@ -3303,82 +3302,16 @@ nsFocusManager::GetNextTabbableContentIn
       } else {
         nsIFrame* frame = iterContent->GetPrimaryFrame();
         if (!frame) {
           continue;
         }
         frame->IsFocusable(&tabIndex, 0);
       }
       if (tabIndex < 0 || !(aIgnoreTabIndex || tabIndex == aCurrentTabIndex)) {
-        // If the element has native anonymous content, we may need to
-        // focus some NAC element, even if the element itself isn't focusable.
-        // This happens for example with <input type="date">.
-        // So, try to find NAC and then traverse the frame tree to find elements
-        // to focus.
-        // Yet, even if the frame is a nsIAnonymousContentCreator, don't
-        // traverse into the element again when the element is in a UA Widget,
-        // because there isn't any NAC to focus.
-        nsIFrame* possibleAnonOwnerFrame = iterContent->GetPrimaryFrame();
-        nsIAnonymousContentCreator* anonCreator =
-          do_QueryFrame(possibleAnonOwnerFrame);
-        bool isIterContentInUAWidgetShadow =
-          iterContent->GetContainingShadow() &&
-          iterContent->GetContainingShadow()->IsUAWidget();
-        if (anonCreator &&
-            !isIterContentInUAWidgetShadow &&
-            !iterContent->IsInNativeAnonymousSubtree()) {
-          nsIFrame* frame = nullptr;
-          // Find the first or last frame in tree order so that
-          // we can scope frame traversing to NAC.
-          if (aForward) {
-            frame = possibleAnonOwnerFrame->PrincipalChildList().FirstChild();
-          } else {
-            frame = possibleAnonOwnerFrame->PrincipalChildList().LastChild();
-            nsIFrame* last = frame;
-            while (last) {
-              frame = last;
-              last = frame->PrincipalChildList().LastChild();
-            }
-          };
-
-          nsCOMPtr<nsIFrameEnumerator> frameTraversal;
-          nsresult rv = NS_NewFrameTraversal(getter_AddRefs(frameTraversal),
-                                             iterContent->OwnerDoc()->
-                                               GetShell()->GetPresContext(),
-                                             frame,
-                                             ePreOrder,
-                                             false, // aVisual
-                                             false, // aLockInScrollView
-                                             true, // aFollowOOFs
-                                             true  // aSkipPopupChecks
-                                             );
-          if (NS_SUCCEEDED(rv)) {
-            nsIFrame* frame =
-              static_cast<nsIFrame*>(frameTraversal->CurrentItem());
-            while (frame) {
-              int32_t tabIndex;
-              frame->IsFocusable(&tabIndex, 0);
-              if (tabIndex >= 0 &&
-                  (aIgnoreTabIndex || aCurrentTabIndex == tabIndex)) {
-                return frame->GetContent();
-              }
-
-              if (aForward) {
-                frameTraversal->Next();
-              } else {
-                frameTraversal->Prev();
-              }
-              frame = static_cast<nsIFrame*>(frameTraversal->CurrentItem());
-              if (frame == possibleAnonOwnerFrame) {
-                break;
-              }
-            }
-          }
-        }
-
         continue;
       }
 
       if (!IsHostOrSlot(iterContent)) {
         nsCOMPtr<nsIContent> elementInFrame;
         bool checkSubDocument = true;
         if (aForDocumentNavigation &&
             TryDocumentNavigation(iterContent, &checkSubDocument,
@@ -3976,17 +3909,17 @@ nsFocusManager::GetNextTabbableMapArea(b
 }
 
 int32_t
 nsFocusManager::GetNextTabIndex(nsIContent* aParent,
                                 int32_t aCurrentTabIndex,
                                 bool aForward)
 {
   int32_t tabIndex, childTabIndex;
-  FlattenedChildIterator iter(aParent);
+  StyleChildrenIterator iter(aParent);
 
   if (aForward) {
     tabIndex = 0;
     for (nsIContent* child = iter.GetNextChild();
          child;
          child = iter.GetNextChild()) {
       // Skip child's descendants if child is a shadow host or slot, as they are
       // in the focus navigation scope owned by child's shadow root
--- a/dom/base/test/file_bug1453693.html
+++ b/dom/base/test/file_bug1453693.html
@@ -375,15 +375,17 @@
 
       function init() {
         SimpleTest.waitForFocus(runTest);
       }
     </script>
     <style>
     </style>
     <template id="template">
-      <p tabindex="0" id="p1">component</p>
-      <p tabindex="0" id="p2">/component</p>
+      <div style="overflow: hidden">
+        <p tabindex="0" id="p1">component</p>
+        <p tabindex="0" id="p2">/component</p>
+      </div>
     </template>
   </head>
   <body onload="init()">
   </body>
 </html>