Bug 1504743 - Combine the box-model highlighter and the flexbox highlighter when highlighting flex containers/items r=gl
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Thu, 17 Jan 2019 17:42:04 +0000
changeset 514292 c63fdb43c80ceed8e580b5760a2e632844c30f5e
parent 514291 10b6c352897a7bc2607abd5f66e96730cf74afcd
child 514293 0f7827253b525066d50ad05a3cfe7d18f4df0124
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1504743
milestone66.0a1
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
Bug 1504743 - Combine the box-model highlighter and the flexbox highlighter when highlighting flex containers/items r=gl Differential Revision: https://phabricator.services.mozilla.com/D12046
devtools/server/actors/highlighters.css
devtools/server/actors/highlighters/box-model.js
devtools/server/actors/highlighters/flexbox.js
devtools/server/actors/layout.js
devtools/shared/layout/utils.js
--- a/devtools/server/actors/highlighters.css
+++ b/devtools/server/actors/highlighters.css
@@ -62,30 +62,40 @@
 }
 
 :-moz-native-anonymous .highlighter-container.box-model {
   /* Make the box-model container have a z-index other than auto so it always sits above
      other highlighters. */
   z-index: 1;
 }
 
+:-moz-native-anonymous .highlighter-container.flexbox {
+  /* Make the flexbox highlighter have a z-index greater than the box-model so
+     it always sits above it. */
+  z-index: 2;
+}
+
 :-moz-native-anonymous .highlighter-container [hidden] {
   display: none;
 }
 
 :-moz-native-anonymous .highlighter-container [dragging] {
   cursor: grabbing;
 }
 
 /* Box Model Highlighter */
 
 :-moz-native-anonymous .box-model-regions {
   opacity: 0.6;
 }
 
+:-moz-native-anonymous .box-model-regions [half-faded] {
+  opacity: 0.5;
+}
+
 /* Box model regions can be faded (see the onlyRegionArea option in
    highlighters.js) in order to only display certain regions. */
 :-moz-native-anonymous .box-model-regions [faded] {
   display: none;
 }
 
 :-moz-native-anonymous .box-model-content {
   fill: var(--highlighter-content-color);
@@ -140,16 +150,19 @@
 
   border-radius: 3px;
   background: var(--highlighter-bubble-background-color) no-repeat padding-box;
 
   color: var(--highlighter-bubble-text-color);
   text-shadow: none;
 
   border: 1px solid var(--highlighter-bubble-border-color);
+
+  /* The infobar should always be above every other highlighter when it is visible */
+  z-index: 2147483647;
 }
 
 /* Arrows */
 
 :-moz-native-anonymous [class$=infobar-container] > [class$=infobar]:before {
   left: calc(50% - var(--highlighter-bubble-arrow-size));
   border: var(--highlighter-bubble-arrow-size) solid var(--highlighter-bubble-border-color);
 }
--- a/devtools/server/actors/highlighters/box-model.js
+++ b/devtools/server/actors/highlighters/box-model.js
@@ -10,22 +10,26 @@ const {
   createNode,
   createSVGNode,
   getBindingElementAndPseudo,
   hasPseudoClassLock,
   isNodeValid,
   moveInfobar,
 } = require("./utils/markup");
 const {
+  findFlexOrGridParentContainerForNode,
+  getCurrentZoom,
   setIgnoreLayoutChanges,
-  getCurrentZoom,
  } = require("devtools/shared/layout/utils");
 const { getNodeDisplayName } = require("devtools/server/actors/inspector/utils");
 const nodeConstants = require("devtools/shared/dom-node-constants");
 
+loader.lazyRequireGetter(this, "FlexboxHighlighter",
+  "devtools/server/actors/highlighters/flexbox", true);
+
 // Note that the order of items in this array is important because it is used
 // for drawing the BoxModelHighlighter's path elements correctly.
 const BOX_MODEL_REGIONS = ["margin", "border", "padding", "content"];
 const BOX_MODEL_SIDES = ["top", "right", "bottom", "left"];
 // Width of boxmodelhighlighter guides
 const GUIDE_STROKE_WIDTH = 1;
 // FIXME: add ":visited" and ":link" after bug 713106 is fixed
 const PSEUDO_CLASSES = [":hover", ":active", ":focus", ":focus-within"];
@@ -269,19 +273,33 @@ class BoxModelHighlighter extends AutoRe
     this.highlighterEnv.off("will-navigate", this.onWillNavigate);
 
     const { pageListenerTarget } = this.highlighterEnv;
     if (pageListenerTarget) {
       pageListenerTarget.removeEventListener("pagehide", this.onPageHide);
     }
 
     this.markup.destroy();
+
+    if (this._flexboxHighlighter) {
+      this._flexboxHighlighter.destroy();
+      this._flexboxHighlighter = null;
+    }
+
     AutoRefreshHighlighter.prototype.destroy.call(this);
   }
 
+  get flexboxHighlighter() {
+    if (!this._flexboxHighlighter) {
+      this._flexboxHighlighter = new FlexboxHighlighter(this.highlighterEnv);
+    }
+
+    return this._flexboxHighlighter;
+  }
+
   getElement(id) {
     return this.markup.getElement(this.ID_CLASS_PREFIX + id);
   }
 
   /**
    * Override the AutoRefreshHighlighter's _isNodeValid method to also return true for
    * text nodes since these can also be highlighted.
    * @param {DOMNode} node
@@ -325,59 +343,152 @@ class BoxModelHighlighter extends AutoRe
   }
 
   /**
    * Update the highlighter on the current highlighted node (the one that was
    * passed as an argument to show(node)).
    * Should be called whenever node size or attributes change
    */
   _update() {
+    const node = this.currentNode;
     let shown = false;
     setIgnoreLayoutChanges(true);
 
+    // We need to set this option before calling _updateBoxModel().
+    this.options.isFlexboxContainer =
+      !!(node && node.getAsFlexContainer && node.getAsFlexContainer());
+
     if (this._updateBoxModel()) {
       // Show the infobar only if configured to do so and the node is an element or a text
       // node.
       if (!this.options.hideInfoBar && (
-          this.currentNode.nodeType === this.currentNode.ELEMENT_NODE ||
-          this.currentNode.nodeType === this.currentNode.TEXT_NODE)) {
+          node.nodeType === node.ELEMENT_NODE ||
+          node.nodeType === node.TEXT_NODE)) {
         this._showInfobar();
       } else {
         this._hideInfobar();
       }
       this._showBoxModel();
       shown = true;
     } else {
       // Nothing to highlight (0px rectangle like a <script> tag for instance)
       this._hide();
     }
 
+    this._updateFlexboxHighlighter();
+
     setIgnoreLayoutChanges(false, this.highlighterEnv.window.document.documentElement);
 
     return shown;
   }
 
+  /**
+   * Update the flexbox highlighter on the current highlighted node. Show it
+   * if the current node is a flexbox container or flexbox item.
+   */
+  _updateFlexboxHighlighter() {
+    this._hideFlexboxHighlighter();
+
+    if (!this.currentNode) {
+      return;
+    }
+
+    const options = {};
+    let node = this.currentNode;
+    let showFlexboxHighlighter = false;
+
+    // If the current node is a flexbox container then remove the box model
+    // content region and make the other regions a little more transparent so
+    // that the flexbox highlighting is emphasized.
+    if (this.options.isFlexboxContainer) {
+      for (const region of BOX_MODEL_REGIONS) {
+        const box = this.getElement(region);
+
+        if (region === "content") {
+          // Hide the content region.
+          box.removeAttribute("d");
+        } else {
+          // Make the non-content regions a little more transparent.
+          box.setAttribute("half-faded", "");
+        }
+      }
+
+      // Stop the flexbox highlighter from showing an outline (the guides do a
+      // great job of that themselves).
+      options.noContainerOutline = true;
+
+      // Toggle the flag to show the flexbox highlighter.
+      showFlexboxHighlighter = true;
+    } else {
+      // The highlighted element is not a flexbox container so we need to check
+      // if it is a flex item.
+      const container = findFlexOrGridParentContainerForNode(node, "flex");
+
+      if (container) {
+        for (const region of BOX_MODEL_REGIONS) {
+          const box = this.getElement(region);
+
+          // Ensure that the box model regions are not faded. The content region
+          // will reappear because it is regenerated by the highlighter.
+          box.setAttribute("half-faded", "");
+        }
+
+        // Hide the guides because we are only interested in the flex item's
+        // box model regions in relation to the flexbox overlay (and to make
+        // things less ugly).
+        this._hideGuides();
+
+        node = container;
+
+        // Toggle the flag to show the flexbox highlighter.
+        showFlexboxHighlighter = true;
+      }
+    }
+
+    if (showFlexboxHighlighter) {
+      // If the flag is set then show the flexbox highlighter.
+      this.flexboxHighlighter.show(node, options);
+    } else {
+      // Otherwise ensure that the box model regions are not faded.
+      for (const region of BOX_MODEL_REGIONS) {
+        const box = this.getElement(region);
+
+        box.removeAttribute("half-faded");
+      }
+    }
+  }
+
   _scrollUpdate() {
     this._moveInfobar();
   }
 
   /**
    * Hide the highlighter, the outline and the infobar.
    */
   _hide() {
     setIgnoreLayoutChanges(true);
 
     this._untrackMutations();
     this._hideBoxModel();
     this._hideInfobar();
+    this._hideFlexboxHighlighter();
 
     setIgnoreLayoutChanges(false, this.highlighterEnv.window.document.documentElement);
   }
 
   /**
+   * Hide the Flexbox highlighter.
+   */
+  _hideFlexboxHighlighter() {
+    if (this._flexboxHighlighter) {
+      this.flexboxHighlighter.hide();
+    }
+  }
+
+  /**
    * Hide the infobar
    */
   _hideInfobar() {
     this.getElement("infobar-container").setAttribute("hidden", "true");
   }
 
   /**
    * Show the infobar
@@ -518,19 +629,21 @@ class BoxModelHighlighter extends AutoRe
     const rootId = this.ID_CLASS_PREFIX + "elements";
     this.markup.scaleRootElement(this.currentNode, rootId);
 
     return true;
   }
 
   _getBoxPathCoordinates(boxQuad, nextBoxQuad) {
     const {p1, p2, p3, p4} = boxQuad;
+    const isFlexboxContainer = this.options.isFlexboxContainer;
 
     let path;
-    if (!nextBoxQuad || !this.options.onlyRegionArea) {
+    if ((isFlexboxContainer && !nextBoxQuad) ||
+        (!isFlexboxContainer && (!nextBoxQuad || !this.options.onlyRegionArea))) {
       // If this is the content box (inner-most box) or if we're not being asked
       // to highlight only region areas, then draw a simple rectangle.
       path = "M" + p1.x + "," + p1.y + " " +
              "L" + p2.x + "," + p2.y + " " +
              "L" + p3.x + "," + p3.y + " " +
              "L" + p4.x + "," + p4.y;
     } else {
       // Otherwise, just draw the region itself, not a filled rectangle.
--- a/devtools/server/actors/highlighters/flexbox.js
+++ b/devtools/server/actors/highlighters/flexbox.js
@@ -61,16 +61,19 @@ const JUSTIFY_CONTENT = "justify-content
  *
  * Available Options:
  * - color(colorValue)
  *     @param  {String} colorValue
  *     The color that should be used to draw the highlighter for this flexbox.
  * - showAlignment(isShown)
  *     @param  {Boolean} isShown
  *     Shows the alignment in the flexbox highlighter.
+ * - noCountainerOutline(isShown)
+ *     @param  {Boolean} noContainerOutline
+ *     Prevent drawing an outline around the flex container.
  *
  * Structure:
  * <div class="highlighter-container">
  *   <div id="flexbox-root" class="flexbox-root">
  *     <canvas id="flexbox-canvas"
  *             class="flexbox-canvas"
  *             width="4096"
  *             height="4096"
@@ -108,17 +111,17 @@ class FlexboxHighlighter extends AutoRef
     // on a page that has scrolled already.
     updateCanvasPosition(this._canvasPosition, this._scroll, this.win,
       this._winDimensions);
   }
 
   _buildMarkup() {
     const container = createNode(this.win, {
       attributes: {
-        "class": "highlighter-container",
+        "class": "highlighter-container flexbox",
       },
     });
 
     const root = createNode(this.win, {
       parent: container,
       attributes: {
         "id": "root",
         "class": "root",
@@ -519,16 +522,19 @@ class FlexboxHighlighter extends AutoRef
     const { width, height } = containerQuad.getBounds();
 
     this.setupCanvas({
       lineDash: FLEXBOX_LINES_PROPERTIES.alignItems.lineDash,
       lineWidthMultiplier: 2,
     });
 
     this.ctx.fillStyle = this.getFlexContainerPattern(devicePixelRatio);
+    this.ctx.strokeStyle =
+      this.options.noContainerOutline ? "transparent" : this.color;
+
     drawRect(this.ctx, 0, 0, width, height, this.currentMatrix);
 
     // Find current angle of outer flex element by measuring the angle of two arbitrary
     // points, then rotate canvas, so the hash pattern stays 45deg to the boundary.
     const p1 = apply(this.currentMatrix, [0, 0]);
     const p2 = apply(this.currentMatrix, [1, 0]);
     const angleRad = Math.atan2(p2[1] - p1[1], p2[0] - p1[0]);
     this.ctx.rotate(angleRad);
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -485,20 +485,16 @@ function findFlexOrGridParentContainerFo
   const treeWalker = walker.getDocumentWalker(node, SHOW_ELEMENT);
   let currentNode = treeWalker.currentNode;
 
   const flexType = type === "flex";
   const gridType = type === "grid";
 
   try {
     while ((currentNode = treeWalker.parentNode())) {
-      if (!currentNode) {
-        break;
-      }
-
       const displayType = walker.getNode(currentNode).displayType;
       if (!displayType) {
         break;
       }
 
       if (flexType && displayType.includes("flex")) {
         if (isNodeAFlexItemInContainer(node, currentNode, walker)) {
           return currentNode;
--- a/devtools/shared/layout/utils.js
+++ b/devtools/shared/layout/utils.js
@@ -901,8 +901,89 @@ function getUntransformedQuad(node, regi
     quad.p4.y += topOffset;
 
     node = node.offsetParent;
   }
 
   return quad;
 }
 exports.getUntransformedQuad = getUntransformedQuad;
+
+/**
+ * If the provided node is a grid of flex item, then return its parent grid or flex
+ * container.
+ *
+ * @param  {DOMNode} node
+ *         The node that is supposedly a grid or flex item.
+ * @param  {String} type
+ *         The type of container/item to look for: "flex" or "grid".
+ * @return {DOMNode|null}
+ *         The parent grid or flex container if found, null otherwise.
+ */
+function findFlexOrGridParentContainerForNode(node, type) {
+  const doc = node.ownerDocument;
+  const win = doc.defaultView;
+  const treeWalker = doc.createTreeWalker(doc.body, NodeFilter.SHOW_ELEMENT);
+  const flexType = type === "flex";
+  const gridType = type === "grid";
+  let currentNode = null;
+
+  treeWalker.currentNode = node;
+
+  try {
+    while ((currentNode = treeWalker.parentNode())) {
+      const displayType = win.getComputedStyle(currentNode).display;
+      if (!displayType) {
+        break;
+      }
+
+      if (flexType && displayType.includes("flex")) {
+        if (isNodeAFlexItemInContainer(node, currentNode)) {
+          return currentNode;
+        }
+      } else if (gridType && displayType.includes("grid")) {
+        return currentNode;
+      } else if (displayType === "contents") {
+        // Continue walking up the tree since the parent node is a content element.
+        continue;
+      }
+
+      break;
+    }
+  } catch (e) {
+    // Getting the parentNode can fail when the supplied node is in shadow DOM.
+  }
+
+  return null;
+}
+exports.findFlexOrGridParentContainerForNode = findFlexOrGridParentContainerForNode;
+
+/**
+ * Returns whether or not the given node is actually considered a flex item by its
+ * container.
+ *
+ * @param  {DOMNode} supposedItem
+ *         The node that might be a flex item of its container.
+ * @param  {DOMNode} container
+ *         The node's container.
+ * @return {Boolean}
+ *         Whether or not the node we are looking at is a flex item of its container.
+ */
+function isNodeAFlexItemInContainer(supposedItem, container) {
+  const doc = container.ownerDocument;
+  const win = doc.defaultView;
+  const containerDisplayType = win.getComputedStyle(container).display;
+
+  if (containerDisplayType.includes("flex")) {
+    const containerFlex = container.getAsFlexContainer();
+
+    for (const line of containerFlex.getLines()) {
+      for (const item of line.getItems()) {
+        if (item.node === supposedItem) {
+          return true;
+        }
+      }
+    }
+  }
+
+  return false;
+}
+exports.isNodeAFlexItemInContainer = isNodeAFlexItemInContainer;