Bug 1479508 Part 2: Change GetElementsWithGrid to use a more conservative traversal that skips subtrees without frames. draft
authorBrad Werth <bwerth@mozilla.com>
Wed, 01 Aug 2018 15:33:06 -0700
changeset 826358 9cd4f463cd2c081cf25b8e09c903e4211798a215
parent 826357 a9065d571214c7a43c931a3f542a99a09c8d3ab3
child 826359 f4824780786a12f7e28ef0fd8ccd3592463483d0
push id118307
push userbwerth@mozilla.com
push dateFri, 03 Aug 2018 18:02:24 +0000
bugs1479508
milestone63.0a1
Bug 1479508 Part 2: Change GetElementsWithGrid to use a more conservative traversal that skips subtrees without frames. MozReview-Commit-ID: 1hTQD8LoLQw
dom/base/Element.cpp
dom/base/Element.h
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1550,46 +1550,41 @@ already_AddRefed<nsIHTMLCollection>
 Element::GetElementsByClassName(const nsAString& aClassNames)
 {
   return nsContentUtils::GetElementsByClassName(this, aClassNames);
 }
 
 void
 Element::GetElementsWithGrid(nsTArray<RefPtr<Element>>& aElements)
 {
-  // This helper function is passed to GetElementsByMatching()
-  // to identify elements with styling which will cause them to
-  // generate a nsGridContainerFrame during layout.
-  auto IsDisplayGrid = [](Element* aElement) -> bool
-  {
-    RefPtr<ComputedStyle> computedStyle =
-      nsComputedDOMStyle::GetComputedStyle(aElement, nullptr);
-    if (computedStyle) {
-      const nsStyleDisplay* display = computedStyle->StyleDisplay();
-      return (display->mDisplay == StyleDisplay::Grid ||
-              display->mDisplay == StyleDisplay::InlineGrid);
+  nsINode* cur = this;
+  while (cur) {
+    if (cur->IsElement()) {
+      Element* elem = cur->AsElement();
+
+      if (elem->GetPrimaryFrame()) {
+        // See if this has a GridContainerFrame. Use the same method that
+        // nsGridContainerFrame uses, which deals with some edge cases.
+        if (nsGridContainerFrame::GetGridContainerFrame(elem->GetPrimaryFrame())) {
+          aElements.AppendElement(elem);
+        }
+
+        // This element has a frame, so allow the traversal to go through
+        // the children.
+        cur = cur->GetNextNode(this);
+        continue;
+      }
     }
-    return false;
-  };
-
-  GetElementsByMatching(IsDisplayGrid, aElements);
-}
-
-void
-Element::GetElementsByMatching(nsElementMatchFunc aFunc,
-                               nsTArray<RefPtr<Element>>& aElements)
-{
-  for (nsINode* cur = this; cur; cur = cur->GetNextNode(this)) {
-    if (cur->IsElement() && aFunc(cur->AsElement())) {
-      aElements.AppendElement(cur->AsElement());
-    }
+
+    // Either this isn't an element, or it has no frame. Continue with the
+    // traversal but ignore all the children.
+    cur = cur->GetNextNonChildNode(this);
   }
 }
 
-
 /**
  * Returns the count of descendants (inclusive of aContent) in
  * the uncomposed document that are explicitly set as editable.
  */
 static uint32_t
 EditableInclusiveDescendantCount(nsIContent* aContent)
 {
   auto htmlElem = nsGenericHTMLElement::FromNode(aContent);
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1162,35 +1162,23 @@ public:
     static_assert(sizeof(CSSPseudoElementType) <= sizeof(uintptr_t),
                   "Need to be able to store this in a void*");
     MOZ_ASSERT(aPseudo != CSSPseudoElementType::NotPseudo);
     SetProperty(nsGkAtoms::pseudoProperty, reinterpret_cast<void*>(aPseudo));
   }
 
   /**
    * Return an array of all elements in the subtree rooted at this
-   * element that are styled as grid containers. This includes
-   * elements that don't actually generate any frames (by virtue of
-   * being in a 'display:none' subtree), but this does not include
+   * element that have grid container frames. This does not include
    * pseudo-elements.
    */
   void GetElementsWithGrid(nsTArray<RefPtr<Element>>& aElements);
 
 private:
   /**
-   * Define a general matching function that can be passed to
-   * GetElementsByMatching(). Each Element being considered is
-   * passed in.
-   */
-  typedef bool (*nsElementMatchFunc)(Element* aElement);
-
-  void GetElementsByMatching(nsElementMatchFunc aFunc,
-                             nsTArray<RefPtr<Element>>& aElements);
-
-  /**
    * Implement the algorithm specified at
    * https://dom.spec.whatwg.org/#insert-adjacent for both
    * |insertAdjacentElement()| and |insertAdjacentText()| APIs.
    */
   nsINode* InsertAdjacent(const nsAString& aWhere,
                           nsINode* aNode,
                           ErrorResult& aError);