Bug 1333714 - Allow highlighter to be hidden even if current node is not valid;r=zer0 draft
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 10 Mar 2017 22:25:01 +0100
changeset 496893 6dfbbdf78f7b59ab8264397f837fa61601bf72ee
parent 496654 5a03382283ae0a020b2a2d84bbbc91ff13cb2130
child 496894 77eae80dd8f610a5cb303d367d114be0cfa6bc4b
push id48736
push userjdescottes@mozilla.com
push dateFri, 10 Mar 2017 21:28:21 +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,17 @@ function* spawnTest() {
         status: "VALID"
       },
       exec: {
         output: "1 node highlighted"
       }
     }
   ]);
 
+  // Hide all highlighters before finishing the test.
+  yield helpers.audit(options, [{
+    setup: "unhighlight",
+    exec: {}
+  }]);
+
   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
@@ -550,17 +550,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.
@@ -999,17 +999,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");
     }