Bug 1499322 - Support pseudo elements in the flexbox inspector; r=rcaliman
authorPatrick Brosset <pbrosset@mozilla.com>
Wed, 17 Oct 2018 14:24:57 +0000
changeset 490068 bfb92e2d55e43b25ee6256c7a037ead4022c3f8d
parent 490067 631545ef79251ea54347ebcb76420b7c1c9ba333
child 490069 1604addc1fc97587040e1b17e6e454ce54328703
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersrcaliman
bugs1499322, 1139937
milestone64.0a1
Bug 1499322 - Support pseudo elements in the flexbox inspector; r=rcaliman On the server, when looking for a flex container for a node, we were bailing out if the displayType of the node was null. It was null for pseudo-elements. This value was returned by the displayType getter in the NodeActor class. Now, the reason for this dates to 4 years ago in bug 1139937 where trying to get the display style of a pseudo-element was done in a way to failed. So we just decided to return null at that point. It doesn't fail anymore, we're able to return, say, "block" if a pseudo-element has a display:block style. So I've removed the checks that returned null and that fixed the issue here. The other part of the fix that was need is in the FlexItemActor class on the server too. This class can be created for a pseudo-element too. It accesses element.style without checking if that property exists. However it does not exist for pseudo-elements. So we needed to add a check for that. It's not a problem to just skip it in this case because pseudo-elements can't have inline styles. Differential Revision: https://phabricator.services.mozilla.com/D8873
devtools/client/inspector/flexbox/test/browser.ini
devtools/client/inspector/flexbox/test/browser_flexbox_pseudo_elements_are_listed.js
devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_pseudos.js
devtools/client/inspector/flexbox/test/doc_flexbox_pseudos.html
devtools/server/actors/inspector/node.js
devtools/server/actors/layout.js
--- a/devtools/client/inspector/flexbox/test/browser.ini
+++ b/devtools/client/inspector/flexbox/test/browser.ini
@@ -1,17 +1,23 @@
 [DEFAULT]
 tags = devtools
 subsuite = devtools
 support-files =
   doc_flexbox_simple.html
+  doc_flexbox_pseudos.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_has_correct_sections.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_pseudo_elements_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 pseudo-elements that are flex items do appear in the list of items.
+
+const TEST_URI = URL_ROOT + "doc_flexbox_pseudos.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, 2, "There are 2 items displayed in the list");
+
+  is(items[0].textContent, "::before", "The first item is ::before");
+  is(items[1].textContent, "::after", "The second item is ::after");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_pseudos.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 pseudo elements.
+
+const TEST_URI = URL_ROOT + "doc_flexbox_pseudos.html";
+
+add_task(async function() {
+  await addTab(TEST_URI);
+  const { inspector, flexboxInspector } = await openLayoutView();
+  const { document: doc } = flexboxInspector;
+
+  info("Select the ::before pseudo-element in the inspector");
+  const containerNode = await getNodeFront(".container", inspector);
+  const { nodes } = await inspector.walker.children(containerNode);
+  const beforeNode = nodes[0];
+
+  const onFlexItemSizingRendered = waitForDOM(doc, "ul.flex-item-sizing");
+  const onFlexItemOutlineRendered = waitForDOM(doc, ".flex-outline-container");
+  await selectNode(beforeNode, 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/doc_flexbox_pseudos.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<style>
+.container {
+  width: 300px;
+  height: 150px;
+  margin: 10px;
+  display: flex;
+}
+
+.container::before,
+.container::after {
+  color: #f06;
+  border: 1px solid #f06;
+  background: gold;
+  padding: 1em;
+}
+
+.container::before {
+  content: "::before pseudo-element item";
+}
+
+.container::after {
+  content: "::after pseudo-element item";
+}
+</style>
+<div class="container"></div>
--- a/devtools/server/actors/inspector/node.js
+++ b/devtools/server/actors/inspector/node.js
@@ -218,19 +218,17 @@ const NodeActor = protocol.ActorClassWit
   },
 
   /**
    * Returns the computed display style property value of the node.
    */
   get displayType() {
     // Consider all non-element nodes as displayed.
     if (InspectorActorUtils.isNodeDead(this) ||
-        this.rawNode.nodeType !== Node.ELEMENT_NODE ||
-        isAfterPseudoElement(this.rawNode) ||
-        isBeforePseudoElement(this.rawNode)) {
+        this.rawNode.nodeType !== Node.ELEMENT_NODE) {
       return null;
     }
 
     const style = this.computedStyle;
     if (!style) {
       return null;
     }
 
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -166,17 +166,18 @@ const FlexItemActor = ActorClassWithSpec
       [`max-${dimension}`]: "",
       [dimension]: ""
     };
 
     if (this.element.nodeType === this.element.ELEMENT_NODE) {
       for (const name in properties) {
         let value = "";
         // Look first on the element style.
-        if (this.element.style[name] && this.element.style[name] !== "auto") {
+        if (this.element.style &&
+            this.element.style[name] && this.element.style[name] !== "auto") {
           value = this.element.style[name];
         } else {
           // And then on the rules that apply to the element.
           // getCSSStyleRules returns rules from least to most specific, so override
           // values as we find them.
           const cssRules = getCSSStyleRules(this.element);
           for (const rule of cssRules) {
             const rulePropertyValue = rule.style.getPropertyValue(name);