Bug 911209 - Display hidden nodes differently in the markup-view; r=miker

browser/devtools/markupview/markup-view.js

@@ -85,6 +85,9 @@ function MarkupView(aInspector, aFrame, aControllerWindow) {
   this._boundMutationObserver = this._mutationObserver.bind(this);
   this.walker.on("mutations", this._boundMutationObserver);
 
+  this._boundOnDisplayChange = this._onDisplayChange.bind(this);
+  this.walker.on("display-change", this._boundOnDisplayChange);
+
   this._boundOnNewSelection = this._onNewSelection.bind(this);
   this._inspector.selection.on("new-node-front", this._boundOnNewSelection);
   this._onNewSelection();
@@ -614,6 +617,19 @@ MarkupView.prototype = {
     });
   },
 
+  /**
+   * React to display-change events from the walker
+   * @param {Array} nodes An array of nodeFronts
+   */
+  _onDisplayChange: function(nodes) {
+    for (let node of nodes) {
+      let container = this._containers.get(node);
+      if (container) {
+        container.isDisplayed = node.isDisplayed;
+      }
+    }
+  },
+
   /**
    * Given a list of mutations returned by the mutation observer, flash the
    * corresponding containers to attract attention.
@@ -1110,6 +1126,9 @@ MarkupView.prototype = {
     this.walker.off("mutations", this._boundMutationObserver)
     this._boundMutationObserver = null;
 
+    this.walker.off("display-change", this._boundOnDisplayChange);
+    this._boundOnDisplayChange = null;
+
     this._elt.removeEventListener("mousemove", this._onMouseMove, false);
     this._elt.removeEventListener("mouseleave", this._onMouseLeave, false);
     this._elt = null;
@@ -1268,6 +1287,9 @@ function MarkupContainer(aMarkupView, aNode, aInspector) {
 
   // Prepare the image preview tooltip data if any
   this._prepareImagePreview();
+
+  // Marking the node as shown or hidden
+  this.isDisplayed = this.node.isDisplayed;
 }
 
 MarkupContainer.prototype = {
@@ -1342,6 +1364,16 @@ MarkupContainer.prototype = {
     });
   },
 
+  /**
+   * Show the element has displayed or not
+   */
+  set isDisplayed(isDisplayed) {
+    this.elt.classList.remove("not-displayed");
+    if (!isDisplayed) {
+      this.elt.classList.add("not-displayed");
+    }
+  },
+
   copyImageDataUri: function() {
     // We need to send again a request to gettooltipData even if one was sent for
     // the tooltip, because we want the full-size image

browser/devtools/markupview/test/browser.ini

@@ -6,6 +6,7 @@ support-files =
   doc_markup_flashing.html
   doc_markup_mutation.html
   doc_markup_navigation.html
+  doc_markup_not_displayed.html
   doc_markup_pagesize_01.html
   doc_markup_pagesize_02.html
   doc_markup_search.html
@@ -26,6 +27,8 @@ support-files =
 [browser_markupview_mutation_01.js]
 [browser_markupview_mutation_02.js]
 [browser_markupview_navigation.js]
+[browser_markupview_node_not_displayed_01.js]
+[browser_markupview_node_not_displayed_02.js]
 [browser_markupview_pagesize_01.js]
 [browser_markupview_pagesize_02.js]
 [browser_markupview_search_01.js]

browser/devtools/markupview/test/browser_markupview_node_not_displayed_01.js

@@ -0,0 +1,33 @@
+/* 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";
+
+// Tests that nodes that are not displayed appear differently in the markup-view
+// when these nodes are imported in the view.
+
+// Note that nodes inside a display:none parent are obviously not displayed too
+// but the markup-view uses css inheritance to mark those as hidden instead of
+// having to visit each and every child of a hidden node. So there's no sense
+// testing children nodes.
+
+const TEST_URL = TEST_URL_ROOT + "doc_markup_not_displayed.html";
+const TEST_DATA = [
+  {selector: "#normal-div", isDisplayed: true},
+  {selector: "head", isDisplayed: false},
+  {selector: "#display-none", isDisplayed: false},
+  {selector: "#hidden-true", isDisplayed: false},
+  {selector: "#visibility-hidden", isDisplayed: true}
+];
+
+let test = asyncTest(function*() {
+  let {inspector} = yield addTab(TEST_URL).then(openInspector);
+
+  for (let {selector, isDisplayed} of TEST_DATA) {
+    info("Getting node " + selector);
+    let container = getContainerForRawNode(selector, inspector);
+    is(!container.elt.classList.contains("not-displayed"), isDisplayed,
+      "The container for " + selector + " is marked as displayed " + isDisplayed);
+  }
+});

browser/devtools/markupview/test/browser_markupview_node_not_displayed_02.js

@@ -0,0 +1,132 @@
+/* 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";
+
+// Tests that nodes are marked as displayed and not-displayed dynamically, when
+// their display changes
+
+const TEST_URL = TEST_URL_ROOT + "doc_markup_not_displayed.html";
+const TEST_DATA = [
+  {
+    desc: "Hiding a node by creating a new stylesheet",
+    selector: "#normal-div",
+    before: true,
+    changeStyle: (doc, node) => {
+      let div = doc.createElement("div");
+      div.id = "new-style";
+      div.innerHTML = "<style>#normal-div {display:none;}</style>";
+      doc.body.appendChild(div);
+    },
+    after: false
+  },
+  {
+    desc: "Showing a node by deleting an existing stylesheet",
+    selector: "#normal-div",
+    before: false,
+    changeStyle: (doc, node) => {
+      doc.getElementById("new-style").remove();
+    },
+    after: true
+  },
+  {
+    desc: "Hiding a node by changing its style property",
+    selector: "#display-none",
+    before: false,
+    changeStyle: (doc, node) => {
+      node.style.display = "block";
+    },
+    after: true
+  },
+  {
+    desc: "Showing a node by removing its hidden attribute",
+    selector: "#hidden-true",
+    before: false,
+    changeStyle: (doc, node) => {
+      node.removeAttribute("hidden");
+    },
+    after: true
+  },
+  {
+    desc: "Hiding a node by adding a hidden attribute",
+    selector: "#hidden-true",
+    before: true,
+    changeStyle: (doc, node) => {
+      node.setAttribute("hidden", "true");
+    },
+    after: false
+  },
+  {
+    desc: "Showing a node by changin a stylesheet's rule",
+    selector: "#hidden-via-stylesheet",
+    before: false,
+    changeStyle: (doc, node) => {
+      doc.styleSheets[0].cssRules[0].style.setProperty("display", "inline");
+    },
+    after: true
+  },
+  {
+    desc: "Hiding a node by adding a new rule to a stylesheet",
+    selector: "#hidden-via-stylesheet",
+    before: true,
+    changeStyle: (doc, node) => {
+      doc.styleSheets[0].insertRule(
+        "#hidden-via-stylesheet {display: none;}", 1);
+    },
+    after: false
+  },
+  {
+    desc: "Hiding a node by adding a class that matches an existing rule",
+    selector: "#normal-div",
+    before: true,
+    changeStyle: (doc, node) => {
+      doc.styleSheets[0].insertRule(
+        ".a-new-class {display: none;}", 2);
+      node.classList.add("a-new-class");
+    },
+    after: false
+  }
+];
+
+let test = asyncTest(function*() {
+  let {inspector} = yield addTab(TEST_URL).then(openInspector);
+
+  for (let data of TEST_DATA) {
+    info("Running test case: " + data.desc);
+    yield runTestData(inspector, data);
+  }
+});
+
+function runTestData(inspector, {selector, before, changeStyle, after}) {
+  let def = promise.defer();
+
+  info("Getting the " + selector + " test node");
+  let container = getContainerForRawNode(selector, inspector);
+  is(!container.elt.classList.contains("not-displayed"), before,
+    "The container is marked as " + (before ? "shown" : "hidden"));
+
+  info("Listening for the display-change event");
+  inspector.markup.walker.once("display-change", nodes => {
+    info("Verifying that the list of changed nodes include our container");
+
+    ok(nodes.length, "The display-change event was received with a nodes");
+    let foundContainer = false;
+    for (let node of nodes) {
+      if (inspector.markup.getContainer(node) === container) {
+        foundContainer = true;
+        break;
+      }
+    }
+    ok(foundContainer, "Container is part of the list of changed nodes");
+
+    is(!container.elt.classList.contains("not-displayed"), after,
+      "The container is marked as " + (after ? "shown" : "hidden"));
+    def.resolve();
+  });
+
+  info("Making style changes");
+  changeStyle(content.document, getNode(selector));
+
+  return def.promise;
+}

browser/devtools/markupview/test/doc_markup_not_displayed.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+    #hidden-via-stylesheet {
+      display: none;
+    }
+  </style>
+</head>
+<body>
+  <div id="normal-div"></div>
+  <div id="display-none" style="display:none;"></div>
+  <div id="hidden-true" hidden="true"></div>
+  <div id="visibility-hidden" style="visibility:hidden;"></div>
+  <div id="hidden-via-stylesheet"></div>
+</body>
+</html>

browser/themes/shared/devtools/markup-view.css

@@ -30,6 +30,12 @@
   color: #f5f7fa; /* Light foreground text */
 }
 
+/* In case a node isn't displayed in the page, we fade the syntax highlighting */
+.not-displayed .open,
+.not-displayed .close {
+  opacity: .7;
+}
+
 .tag-line {
   padding-left: 2px;
 }

toolkit/devtools/server/actors/inspector.js

@@ -62,6 +62,8 @@ const {Unknown} = require("sdk/platform/xpcom");
 const {Class} = require("sdk/core/heritage");
 const {PageStyleActor} = require("devtools/server/actors/styles");
 const {HighlighterActor} = require("devtools/server/actors/highlighter");
+const {getLayoutChangesObserver, releaseLayoutChangesObserver} =
+  require("devtools/server/actors/layout");
 
 const FONT_FAMILY_PREVIEW_TEXT = "The quick brown fox jumps over the lazy dog";
 const FONT_FAMILY_PREVIEW_TEXT_SIZE = 20;
@@ -177,6 +179,10 @@ var NodeActor = exports.NodeActor = protocol.ActorClass({
     protocol.Actor.prototype.initialize.call(this, null);
     this.walker = walker;
     this.rawNode = node;
+
+    // Storing the original display of the node, to track changes when reflows
+    // occur
+    this.wasDisplayed = this.isDisplayed;
   },
 
   toString: function() {
@@ -227,6 +233,8 @@ var NodeActor = exports.NodeActor = protocol.ActorClass({
       attrs: this.writeAttrs(),
 
       pseudoClassLocks: this.writePseudoClassLocks(),
+
+      isDisplayed: this.isDisplayed,
     };
 
     if (this.isDocumentElement()) {
@@ -247,6 +255,29 @@ var NodeActor = exports.NodeActor = protocol.ActorClass({
     return form;
   },
 
+  get computedStyle() {
+    if (Cu.isDeadWrapper(this.rawNode) ||
+        this.rawNode.nodeType !== Ci.nsIDOMNode.ELEMENT_NODE ||
+        !this.rawNode.ownerDocument ||
+        !this.rawNode.ownerDocument.defaultView) {
+      return null;
+    }
+    return this.rawNode.ownerDocument.defaultView.getComputedStyle(this.rawNode);
+  },
+
+  /**
+   * Is the node's display computed style value other than "none"
+   */
+  get isDisplayed() {
+    let style = this.computedStyle;
+    if (!style) {
+      // Consider all non-element nodes as displayed
+      return true;
+    } else {
+      return style.display !== "none";
+    }
+  },
+
   writeAttrs: function() {
     if (!this.rawNode.attributes) {
       return undefined;
@@ -548,6 +579,12 @@ let NodeFront = protocol.FrontClass(NodeActor, {
     return this.pseudoClassLocks.some(locked => locked === pseudo);
   },
 
+  get isDisplayed() {
+    // The NodeActor's form contains the isDisplayed information as a boolean
+    // starting from FF32. Before that, the property is missing
+    return "isDisplayed" in this._form ? this._form.isDisplayed : true;
+  },
+
   getNodeValue: protocol.custom(function() {
     if (!this.incompleteValue) {
       return delayedResolve(new ShortLongString(this.shortValue));
@@ -836,6 +873,10 @@ var WalkerActor = protocol.ActorClass({
     },
     "highlighter-hide" : {
       type: "highlighter-hide"
+    },
+    "display-change" : {
+      type: "display-change",
+      nodes: Arg(0, "array:domnode")
     }
   },
 
@@ -875,6 +916,10 @@ var WalkerActor = protocol.ActorClass({
     // Ensure that the root document node actor is ready and
     // managed.
     this.rootNode = this.document();
+
+    this.reflowObserver = getLayoutChangesObserver(this.tabActor);
+    this._onReflows = this._onReflows.bind(this);
+    this.reflowObserver.on("reflows", this._onReflows);
   },
 
   // Returns the JSON representation of this object over the wire.
@@ -894,6 +939,11 @@ var WalkerActor = protocol.ActorClass({
     this.clearPseudoClassLocks();
     this._activePseudoClassLocks = null;
     this.rootDoc = null;
+
+    this.reflowObserver.off("reflows", this._onReflows);
+    this.reflowObserver = null;
+    releaseLayoutChangesObserver(this.tabActor);
+
     events.emit(this, "destroyed");
     protocol.Actor.prototype.destroy.call(this);
   },
@@ -904,7 +954,7 @@ var WalkerActor = protocol.ActorClass({
     if (actor instanceof NodeActor) {
       if (this._activePseudoClassLocks &&
           this._activePseudoClassLocks.has(actor)) {
-        this.clearPsuedoClassLocks(actor);
+        this.clearPseudoClassLocks(actor);
       }
       this._refMap.delete(actor.rawNode);
     }
@@ -928,6 +978,24 @@ var WalkerActor = protocol.ActorClass({
     return actor;
   },
 
+  _onReflows: function(reflows) {
+    // Going through the nodes the walker knows about, see which ones have
+    // had their display changed and send a display-change event if any
+    let changes = [];
+    for (let [node, actor] of this._refMap) {
+      let isDisplayed = actor.isDisplayed;
+      if (isDisplayed !== actor.wasDisplayed) {
+        changes.push(actor);
+        // Updating the original value
+        actor.wasDisplayed = isDisplayed;
+      }
+    }
+
+    if (changes.length) {
+      events.emit(this, "display-change", changes);
+    }
+  },
+
   /**
    * This is kept for backward-compatibility reasons with older remote targets.
    * Targets prior to bug 916443.