Bug 1513500 - Fixed it so getCurrentDisplay also checks if a flex container is actually a flex item. r=pbro a=RyanVM
authorMicah Tigley <mtigley@mozilla.com>
Thu, 13 Dec 2018 15:07:49 +0000
changeset 506227 0d66c5be649b285f6881428dc3029d5281c93018
parent 506226 7e036d607afaca9bda58e90127bb0064dc966345
child 506228 6f8e2bdfd8b61347c2ccd7a29841ef277d055d04
push id10336
push usercsabou@mozilla.com
push dateFri, 14 Dec 2018 21:17:48 +0000
treeherdermozilla-beta@6f8e2bdfd8b6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro, RyanVM
bugs1513500
milestone65.0
Bug 1513500 - Fixed it so getCurrentDisplay also checks if a flex container is actually a flex item. r=pbro a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D14312
devtools/client/inspector/flexbox/test/browser.ini
devtools/client/inspector/flexbox/test/browser_flexbox_grand_parent_flex.js
devtools/server/actors/layout.js
--- a/devtools/client/inspector/flexbox/test/browser.ini
+++ b/devtools/client/inspector/flexbox/test/browser.ini
@@ -20,16 +20,17 @@ support-files =
 
 [browser_flexbox_accordion_state.js]
 [browser_flexbox_container_and_item.js]
 [browser_flexbox_container_and_item_accordion_state.js]
 [browser_flexbox_container_and_item_updates_on_change.js]
 [browser_flexbox_container_element_rep.js]
 [browser_flexbox_container_properties.js]
 [browser_flexbox_empty_state.js]
+[browser_flexbox_grand_parent_flex.js]
 [browser_flexbox_highlighter_color_picker_on_ESC.js]
 [browser_flexbox_highlighter_color_picker_on_RETURN.js]
 [browser_flexbox_highlighter_opened_telemetry.js]
 [browser_flexbox_item_list_01.js]
 [browser_flexbox_item_list_02.js]
 [browser_flexbox_item_list_updates_on_change.js]
 [browser_flexbox_item_outline_exists.js]
 [browser_flexbox_item_outline_has_correct_layout.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_grand_parent_flex.js
@@ -0,0 +1,44 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that a flex container is not shown as a flex item of its grandparent flex
+// container.
+
+const TEST_URI = `
+<style>
+.flex {
+  display: flex;
+}
+</style>
+<div class="flex">
+  <div>
+    <div id="grandchild" class="flex">
+      This is a flex item of a flex container.
+      Its parent isn't a flex container, but its grandparent is.
+    </div>
+  </div>
+</div>
+`;
+
+add_task(async function() {
+  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  const { inspector, flexboxInspector } = await openLayoutView();
+  const { document: doc } = flexboxInspector;
+
+  info("Select the flex container's grandchild.");
+  const onFlexContainerHeaderRendered = waitForDOM(doc, ".flex-header-container-label");
+  await selectNode("#grandchild", inspector);
+  await onFlexContainerHeaderRendered;
+
+  info("Check that only the Flex Container accordion item is showing.");
+  const flexPanes = doc.querySelectorAll(".flex-accordion");
+  is(flexPanes.length, 1, "There should only be one flex accordion item showing.");
+
+  info("Check that the container header shows Flex Container.");
+  const flexAccordionHeader = flexPanes[0].querySelector("._header .truncate");
+  is(flexAccordionHeader.textContent, "Flex Container",
+    "The flexbox pane shows a flex container accordion item.");
+});
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -331,21 +331,23 @@ const LayoutActor = ActorClassWithSpec(l
    * selected node. The current node can be a grid/flex container or grid/flex item.
    * If it is a grid/flex item, returns the parent grid/flex container. Otherwise, returns
    * null if the current or parent node is not a grid/flex container.
    *
    * @param  {Node|NodeActor} node
    *         The node to start iterating at.
    * @param  {String} type
    *         Can be "grid" or "flex", the display type we are searching for.
+   * @param  {Boolean} onlyLookAtContainer
+   *         If true, only look at given node's container and iterate from there.
    * @return {GridActor|FlexboxActor|null}
    *         The GridActor or FlexboxActor of the grid/flex container of the given node.
    *         Otherwise, returns null.
    */
-  getCurrentDisplay(node, type) {
+  getCurrentDisplay(node, type, onlyLookAtContainer) {
     if (isNodeDead(node)) {
       return null;
     }
 
     // Given node can either be a Node or a NodeActor.
     if (node.rawNode) {
       node = node.rawNode;
     }
@@ -356,17 +358,27 @@ const LayoutActor = ActorClassWithSpec(l
 
     // If the node is an element, check first if it is itself a flex or a grid.
     if (node.nodeType === node.ELEMENT_NODE) {
       if (!displayType) {
         return null;
       }
 
       if (flexType && displayType.includes("flex")) {
-        return new FlexboxActor(this, node);
+        if (!onlyLookAtContainer) {
+          return new FlexboxActor(this, node);
+        }
+
+        const container = findFlexOrGridParentContainerForNode(node, type, this.walker);
+
+        if (container) {
+          return new FlexboxActor(this, container);
+        }
+
+        return null;
       } else if (gridType && displayType.includes("grid")) {
         return new GridActor(this, node);
       }
     }
 
     // Otherwise, check if this is a flex/grid item or the parent node is a flex/grid
     // container.
     // Note that text nodes that are children of flex/grid containers are wrapped in
@@ -409,21 +421,17 @@ const LayoutActor = ActorClassWithSpec(l
    *         The node to start iterating at.
    * @param  {Boolean|null} onlyLookAtParents
    *         If true, skip the passed node and only start looking at its parent and up.
    * @return {FlexboxActor|null}
    *         The FlexboxActor of the flex container of the given node. Otherwise, returns
    *         null.
    */
   getCurrentFlexbox(node, onlyLookAtParents) {
-    if (onlyLookAtParents) {
-      node = node.rawNode.parentNode;
-    }
-
-    return this.getCurrentDisplay(node, "flex");
+    return this.getCurrentDisplay(node, "flex", onlyLookAtParents);
   },
 
   /**
    * Returns an array of GridActor objects for all the grid elements contained in the
    * given root node.
    *
    * @param  {Node|NodeActor} node
    *         The root node for grid elements