Bug 1333714 - Allow highlighter to be hidden even if current node is not valid;r=zer0 draft
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 08 Mar 2017 22:03:46 +0100
changeset 495437 0619eadbad730fa0123ad62b651a54a59b6b08e3
parent 494679 a8d5f142c025a938b6af1656443b9eac20020e94
child 495438 8ab40106fe2e3b905b97606ee7facc3ff16d32f1
push id48338
push userjdescottes@mozilla.com
push dateWed, 08 Mar 2017 21:04:47 +0000
reviewerszer0
bugs1333714
milestone55.0a1
Bug 1333714 - Allow highlighter to be hidden even if current node is not valid;r=zer0 When a highlighter was created for a node that has been removed, it can no longer be hidden due to a check performed in the hide method of the basic auto-refresh class. While it makes sense not to display a highlighter for an invalid node, hiding a highlighter should always remain possible. MozReview-Commit-ID: ChkmecJeqy9
devtools/client/commandline/test/browser_cmd_highlight_01.js
devtools/server/actors/highlighters/auto-refresh.js
devtools/server/actors/highlighters/css-grid.js
devtools/server/actors/highlighters/geometry-editor.js
--- a/devtools/client/commandline/test/browser_cmd_highlight_01.js
+++ b/devtools/client/commandline/test/browser_cmd_highlight_01.js
@@ -80,11 +80,19 @@ function* spawnTest() {
         status: "VALID"
       },
       exec: {
         output: "1 node highlighted"
       }
     }
   ]);
 
+  yield helpers.audit(options, {
+    cmd: "unhighlight",
+    check: {},
+    exec: {
+      output: "highlighters hidden"
+    }
+  });
+
   yield helpers.closeToolbar(options);
   yield helpers.closeTab(options);
 }
--- a/devtools/server/actors/highlighters/auto-refresh.js
+++ b/devtools/server/actors/highlighters/auto-refresh.js
@@ -114,17 +114,17 @@ AutoRefreshHighlighter.prototype = {
     }
     return shown;
   },
 
   /**
    * Hide the highlighter
    */
   hide: function () {
-    if (!this._isNodeValid(this.currentNode)) {
+    if (Cu.isDeadWrapper(this.highlighterEnv.window)) {
       return;
     }
 
     this._hide();
     this._stopRefreshLoop();
     this.currentNode = null;
     this.currentQuads = {};
     this.options = null;
--- a/devtools/server/actors/highlighters/css-grid.js
+++ b/devtools/server/actors/highlighters/css-grid.js
@@ -524,17 +524,17 @@ CssGridHighlighter.prototype = extend(Au
     }
 
     this._showGrid();
     this._showGridElements();
 
     root.setAttribute("style",
       `position:absolute; width:${width}px;height:${height}px; overflow:hidden`);
 
-    setIgnoreLayoutChanges(false, this.highlighterEnv.window.document.documentElement);
+    setIgnoreLayoutChanges(false, this.highlighterEnv.document.documentElement);
     return true;
   },
 
   /**
    * Update the grid information displayed in the grid area info bar.
    *
    * @param  {GridArea} area
    *         The grid area object.
@@ -913,17 +913,17 @@ CssGridHighlighter.prototype = extend(Au
    * Hide the highlighter, the canvas and the infobars.
    */
   _hide() {
     setIgnoreLayoutChanges(true);
     this._hideGrid();
     this._hideGridElements();
     this._hideGridAreaInfoBar();
     this._hideGridCellInfoBar();
-    setIgnoreLayoutChanges(false, this.highlighterEnv.window.document.documentElement);
+    setIgnoreLayoutChanges(false, this.highlighterEnv.document.documentElement);
   },
 
   _hideGrid() {
     this.getElement("canvas").setAttribute("hidden", "true");
   },
 
   _showGrid() {
     this.getElement("canvas").removeAttribute("hidden");
--- a/devtools/server/actors/highlighters/geometry-editor.js
+++ b/devtools/server/actors/highlighters/geometry-editor.js
@@ -507,17 +507,17 @@ GeometryEditorHighlighter.prototype = ex
     this.updateOffsetParent();
     this.updateCurrentNode();
     this.updateArrows();
 
     // Avoid zooming the arrows when content is zoomed.
     let node = this.currentNode;
     this.markup.scaleRootElement(node, this.ID_CLASS_PREFIX + "root");
 
-    setIgnoreLayoutChanges(false, node.ownerDocument.documentElement);
+    setIgnoreLayoutChanges(false, this.highlighterEnv.document.documentElement);
     return true;
   },
 
   /**
    * Update the offset parent rectangle.
    * There are 3 different cases covered here:
    * - the node is absolutely/fixed positioned, and an offsetParent is defined
    *   (i.e. it's not just positioned in the viewport): the offsetParent node
@@ -587,18 +587,17 @@ GeometryEditorHighlighter.prototype = ex
 
     this.getElement("root").setAttribute("hidden", "true");
     this.getElement("current-node").setAttribute("hidden", "true");
     this.getElement("offset-parent").setAttribute("hidden", "true");
     this.hideArrows();
 
     this.definedProperties.clear();
 
-    setIgnoreLayoutChanges(false,
-      this.currentNode.ownerDocument.documentElement);
+    setIgnoreLayoutChanges(false, this.highlighterEnv.document.documentElement);
   },
 
   hideArrows: function () {
     for (let side of GeoProp.SIDES) {
       this.getElement("arrow-" + side).setAttribute("hidden", "true");
       this.getElement("label-" + side).setAttribute("hidden", "true");
       this.getElement("handler-" + side).setAttribute("hidden", "true");
     }