Bug 1497181 - Display sizing info for text nodes too; r=gl
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 18 Oct 2018 17:23:32 +0000
changeset 490327 b786be7eb3b5fc1acf0cb38d85cf7104ae2f274c
parent 490326 d7795bc645ac0e3aa47cd4e1112b89887e941bf1
child 490328 50cc3c94b20d7ba104fc24b887e15064cad69d41
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersgl
bugs1497181
milestone64.0a1
Bug 1497181 - Display sizing info for text nodes too; r=gl Differential Revision: https://phabricator.services.mozilla.com/D8976
devtools/client/inspector/flexbox/test/browser.ini
devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_text_nodes.js
devtools/client/inspector/flexbox/test/browser_flexbox_text_nodes_are_listed.js
devtools/client/inspector/flexbox/test/doc_flexbox_text_nodes.html
devtools/server/actors/layout.js
--- a/devtools/client/inspector/flexbox/test/browser.ini
+++ b/devtools/client/inspector/flexbox/test/browser.ini
@@ -1,23 +1,26 @@
 [DEFAULT]
 tags = devtools
 subsuite = devtools
 support-files =
   doc_flexbox_simple.html
   doc_flexbox_pseudos.html
+  doc_flexbox_text_nodes.html
   head.js
   !/devtools/client/inspector/test/head.js
   !/devtools/client/inspector/test/shared-head.js
   !/devtools/client/shared/test/shared-head.js
   !/devtools/client/shared/test/telemetry-test-helpers.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
 
 [browser_flexbox_highlighter_color_picker_on_ESC.js]
 [browser_flexbox_highlighter_color_picker_on_RETURN.js]
 [browser_flexbox_item_outline_exists.js]
 [browser_flexbox_item_outline_has_correct_layout.js]
 [browser_flexbox_item_outline_rotates_for_column.js]
 [browser_flexbox_pseudo_elements_are_listed.js]
 [browser_flexbox_sizing_info_exists.js]
 [browser_flexbox_sizing_info_for_pseudos.js]
+[browser_flexbox_sizing_info_for_text_nodes.js]
 [browser_flexbox_sizing_info_has_correct_sections.js]
+[browser_flexbox_text_nodes_are_listed.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_text_nodes.js
@@ -0,0 +1,38 @@
+/* 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 the flex item sizing UI also appears for text nodes.
+
+const TEST_URI = URL_ROOT + "doc_flexbox_text_nodes.html";
+
+add_task(async function() {
+  await addTab(TEST_URI);
+  const { inspector, flexboxInspector } = await openLayoutView();
+  const { document: doc } = flexboxInspector;
+
+  info("Select the first text node in the flex container");
+  const containerNode = await getNodeFront(".container", inspector);
+  const { nodes } = await inspector.walker.children(containerNode);
+  const firstTextNode = nodes[0];
+
+  const onFlexItemSizingRendered = waitForDOM(doc, "ul.flex-item-sizing");
+  const onFlexItemOutlineRendered = waitForDOM(doc, ".flex-outline-container");
+  await selectNode(firstTextNode, inspector);
+  const [flexSizingContainer] = await onFlexItemSizingRendered;
+  const [flexOutlineContainer] = await onFlexItemOutlineRendered;
+
+  ok(flexSizingContainer, "The flex sizing exists in the DOM");
+  ok(flexOutlineContainer, "The flex outline exists in the DOM");
+
+  info("Check that the various sizing sections are displayed");
+  const allSections = [...flexSizingContainer.querySelectorAll(".section")];
+  ok(allSections.length, "Sizing sections are displayed");
+
+  info("Check that the various parts of the outline are displayed");
+  const [basis, final] = [...flexOutlineContainer.querySelectorAll(
+    ".flex-outline-basis, .flex-outline-final")];
+  ok(basis && final, "The final and basis parts of the outline exist");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_text_nodes_are_listed.js
@@ -0,0 +1,27 @@
+/* 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 text nodes that are flex items do appear in the list of items.
+
+const TEST_URI = URL_ROOT + "doc_flexbox_text_nodes.html";
+
+add_task(async function() {
+  await addTab(TEST_URI);
+  const { inspector, flexboxInspector } = await openLayoutView();
+  const { document: doc } = flexboxInspector;
+
+  // Select the flex container in the inspector.
+  const onItemsListRendered = waitForDOM(doc,
+    "#layout-flexbox-container .flex-item-list");
+  await selectNode(".container", inspector);
+  const [flexItemList] = await onItemsListRendered;
+
+  const items = [...flexItemList.querySelectorAll("li")];
+  is(items.length, 3, "There are 3 items displayed in the list");
+
+  is(items[0].textContent, "#text", "The first item is a text node");
+  is(items[2].textContent, "#text", "The third item is a text node");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/flexbox/test/doc_flexbox_text_nodes.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<style>
+.container {
+  width: 400px;
+  display: flex;
+}
+.container div {
+  flex-basis: 100px;
+  flex-shrink: 0;
+  background: #f06;
+  align-self: stretch;
+}
+</style>
+<div class="container">
+  A text node will be wrapped into an anonymous block container
+  <div></div>
+  Here is yet another text node
+</div>
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -310,33 +310,39 @@ const LayoutActor = ActorClassWithSpec(l
     if (node.rawNode) {
       node = node.rawNode;
     }
 
     const treeWalker = this.walker.getDocumentWalker(node, SHOW_ELEMENT);
     let currentNode = treeWalker.currentNode;
     let displayType = this.walker.getNode(currentNode).displayType;
 
-    if (!displayType) {
-      return null;
-    }
-
-    if (type == "flex") {
-      if (displayType == "inline-flex" || displayType == "flex") {
-        return new FlexboxActor(this, currentNode);
-      } else if (onlyLookAtCurrentNode) {
+    // If the node is an element, check first if it is itself a flex or a grid.
+    if (currentNode.nodeType === currentNode.ELEMENT_NODE) {
+      if (!displayType) {
         return null;
       }
-    } else if (type == "grid" &&
-               (displayType == "inline-grid" || displayType == "grid")) {
-      return new GridActor(this, currentNode);
+
+      if (type == "flex") {
+        if (displayType == "inline-flex" || displayType == "flex") {
+          return new FlexboxActor(this, currentNode);
+        } else if (onlyLookAtCurrentNode) {
+          return null;
+        }
+      } else if (type == "grid" &&
+                 (displayType == "inline-grid" || displayType == "grid")) {
+        return new GridActor(this, currentNode);
+      }
     }
 
     // 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
+    // anonymous containers, so even if their displayType getter returns null we still
+    // want to walk up the chain to find their container.
     while ((currentNode = treeWalker.parentNode())) {
       if (!currentNode) {
         break;
       }
 
       displayType = this.walker.getNode(currentNode).displayType;
 
       if (type == "flex" &&