Backed out 3 changesets (bug 1479508) for devtools failures at devtools/client/inspector/grids/test/browser_grids_no_fragments.js on a CLOSED TREE
authorCoroiu Cristina <ccoroiu@mozilla.com>
Sat, 04 Aug 2018 00:48:56 +0300
changeset 485246 5cb6d926e60bc6e56da2971f167a965983354d5c
parent 485245 741a654c2f4c183a14c717f3367828f5003078e5
child 485247 72be95f82a93c62aab67814e1a5babd37a28ae68
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1479508
milestone63.0a1
backs outcba3360692b8089e5850d1c111a8f57697fa554b
c42fa0ccd335bead0c5e5bcf98149f4b4754ad90
09d7d2f8fe4f1176c93fc3028821a5aec1177430
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
Backed out 3 changesets (bug 1479508) for devtools failures at devtools/client/inspector/grids/test/browser_grids_no_fragments.js on a CLOSED TREE Backed out changeset cba3360692b8 (bug 1479508) Backed out changeset c42fa0ccd335 (bug 1479508) Backed out changeset 09d7d2f8fe4f (bug 1479508)
dom/base/Element.cpp
dom/base/Element.h
dom/base/test/chrome/test_getElementsWithGrid.html
dom/webidl/Element.webidl
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1550,41 +1550,46 @@ already_AddRefed<nsIHTMLCollection>
 Element::GetElementsByClassName(const nsAString& aClassNames)
 {
   return nsContentUtils::GetElementsByClassName(this, aClassNames);
 }
 
 void
 Element::GetElementsWithGrid(nsTArray<RefPtr<Element>>& aElements)
 {
-  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;
-      }
+  // 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);
     }
-
-    // Either this isn't an element, or it has no frame. Continue with the
-    // traversal but ignore all the children.
-    cur = cur->GetNextNonChildNode(this);
+    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());
+    }
   }
 }
 
+
 /**
  * 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,23 +1162,35 @@ 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 have grid container frames. This does not include
+   * 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
    * 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);
 
--- a/dom/base/test/chrome/test_getElementsWithGrid.html
+++ b/dom/base/test/chrome/test_getElementsWithGrid.html
@@ -23,17 +23,17 @@
 'use strict';
 
 SimpleTest.waitForExplicitFinish();
 
 function testTargetsAreInElements(targets, elements) {
   let c = 0;
   for (let target of targets) {
     if (c >= elements.length) {
-      ok(false, "We shouldn't have more targets than elements found.");
+      ok(false, "We have more targets than elements found.");
       break;
     }
     let element = elements[c];
     let isMatching = (target.id == element.id);
     let test_function = (target.todo ? todo : ok);
 
     test_function(isMatching, "Should find " + target.message + ".");
 
@@ -42,51 +42,49 @@ function testTargetsAreInElements(target
     // cascading errors in that case. If we've instead screwed up the target
     // list, then we will get cascading errors.
     if (isMatching) {
       ++c;
     }
   }
 
   // Make sure we don't have any extra elements after going through all the targets.
-  is(c, elements.length, "We shouldn't have more elements than we have targets.");
+  is(c, elements.length, "We found more elements than we have targets.");
 }
 
 function runTests() {
   // Part 1: Look for all the grid elements starting from the document root.
   let elementsFromRoot = document.documentElement.getElementsWithGrid();
+  is(elementsFromRoot.length, 8, "Found expected number of elements within document root.");
 
   // Check that the expected elements were returned.
   // Targets are provided in order we expect them to appear.
   // Has to end in a non-todo element in order for testing logic to work.
   let targetsFromRoot = [
     {id:"root", message:"root with display:grid"},
     {id:"a", message:"'plain' grid container with display:grid"},
     {id:"b", message:"display:subgrid inside display:grid (to be fixed in Bug 1240834)", todo:true},
     {id:"c", message:"'plain' grid container with display:inline-grid"},
     {id:"d", message:"display:subgrid inside display:inline-grid (to be fixed in Bug 1240834)", todo:true},
     {id:"e", message:"grid container with visibility:hidden"},
-    {id:"f", message:"grid container inside an element"},
+    {id:"f", message:"grid container inside a display:none element"},
     {id:"g", message:"overflow:scroll grid container"},
     {id:"h", message:"button as a grid container"},
     {id:"i", message:"fieldset as a grid container"},
-    {id:"k1", message:"grid container containing a grid container"},
-    {id:"k2", message:"grid container inside a grid container"},
   ];
-  is(elementsFromRoot.length, 10, "Found expected number of elements within document root.");
   testTargetsAreInElements(targetsFromRoot, elementsFromRoot);
 
 
   // Part 2: Look for all the grid elements starting from a non-root element.
   let elementsFromNonRoot = document.getElementById("a_non_root_element").getElementsWithGrid();
+  is(elementsFromNonRoot.length, 1, "Found expected number of elements from non-root element.");
 
   let targetsFromNonRoot = [
-    {id:"f", message:"grid container inside an element (from non-root element)"},
+    {id:"f", message:"grid container inside a display:none element (from non-root element)"},
   ];
-  is(elementsFromNonRoot.length, 1, "Found expected number of elements from non-root element.");
   testTargetsAreInElements(targetsFromNonRoot, elementsFromNonRoot);
 
   SimpleTest.finish();
 }
 </script>
 </head>
 <body onLoad="runTests();">
 
@@ -98,24 +96,20 @@ function runTests() {
 <div class="no-match"></div>
 
 <div id="c" class="gi">
   <div id="d" class="s"></div>
 </div>
 
 <div id="e" class="g" style="visibility:hidden"></div>
 
-<div id="a_non_root_element"><div id="f" class="g"></div></div>
+<div id="a_non_root_element" style="display:none"><div id="f" class="g"></div></div>
 
 <div class="no-match"></div>
 
 <div id="g" style="overflow:scroll" class="g"></div>
 
 <button id="h" class="g"></button>
 
 <fieldset id="i" class="g"></fieldset>
 
-<div id="a_display_none_element" style="display:none"><div id="j" class="g"></div></div>
-
-<div id="k1" class="g"><div id="k2" class="g"></div></div>
-
 </body>
 </html>
--- a/dom/webidl/Element.webidl
+++ b/dom/webidl/Element.webidl
@@ -66,17 +66,19 @@ interface Element : Node {
   boolean webkitMatchesSelector(DOMString selector);
 
   [Pure]
   HTMLCollection getElementsByTagName(DOMString localName);
   [Throws, Pure]
   HTMLCollection getElementsByTagNameNS(DOMString? namespace, DOMString localName);
   [Pure]
   HTMLCollection getElementsByClassName(DOMString classNames);
- 
+  [ChromeOnly, Pure]
+  sequence<Element> getElementsWithGrid();
+
   [CEReactions, Throws, Pure]
   Element? insertAdjacentElement(DOMString where, Element element); // historical
 
   [Throws]
   void insertAdjacentText(DOMString where, DOMString data); // historical
 
   /**
    * The ratio of font-size-inflated text font size to computed font
@@ -149,16 +151,35 @@ interface Element : Node {
 
   [ChromeOnly]
   /**
    * Scrolls the element by (dx, dy) CSS pixels without doing any
    * layout flushing.
    */
   boolean scrollByNoFlush(long dx, long dy);
 
+  // Support reporting of Flexbox properties
+  /**
+   * If this element has a display:flex or display:inline-flex style,
+   * this property returns an object with computed values for flex
+   * properties, as well as a property that exposes the flex lines
+   * in this container.
+   */
+  [ChromeOnly, Pure]
+  Flex? getAsFlexContainer();
+
+  // Support reporting of Grid properties
+  /**
+   * If this element has a display:grid or display:inline-grid style,
+   * this property returns an object with computed values for grid
+   * tracks and lines.
+   */
+  [ChromeOnly, Pure]
+  sequence<Grid> getGridFragments();
+
   [ChromeOnly]
   DOMMatrixReadOnly getTransformToAncestor(Element ancestor);
   [ChromeOnly]
   DOMMatrixReadOnly getTransformToParent();
   [ChromeOnly]
   DOMMatrixReadOnly getTransformToViewport();
 };
 
@@ -267,38 +288,8 @@ partial interface Element {
   void mozRequestFullScreen();
 };
 
 // https://w3c.github.io/pointerlock/#extensions-to-the-element-interface
 partial interface Element {
   [NeedsCallerType]
   void requestPointerLock();
 };
-
-// Mozilla-specific additions to support devtools
-partial interface Element {
-  // Support reporting of Flexbox properties
-  /**
-   * If this element has a display:flex or display:inline-flex style,
-   * this property returns an object with computed values for flex
-   * properties, as well as a property that exposes the flex lines
-   * in this container.
-   */
-  [ChromeOnly, Pure]
-  Flex? getAsFlexContainer();
-
-  // Support reporting of Grid properties
-  /**
-   * If this element has a display:grid or display:inline-grid style,
-   * this property returns an object with computed values for grid
-   * tracks and lines.
-   */
-  [ChromeOnly, Pure]
-  sequence<Grid> getGridFragments();
-  
-  /**
-   * Returns a sequence of all the descendent elements of this element
-   * that have display:grid or display:inline-grid style and generate
-   * a frame.
-   */
-  [ChromeOnly, Pure]
-  sequence<Element> getElementsWithGrid();
-};