Bug 911209 - Display hidden nodes differently in the markup-view; r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 05 Jun 2014 14:50:03 +0200
changeset 206177 62385fdd6577156b0e855b1355eb7d491983ec12
parent 206176 eddaac20ab8579256ff1cb4e845b6611bfaed47e
child 206178 65b0d08a49ac78eb5e729a69a4e8812231ab5593
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs911209
milestone32.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 911209 - Display hidden nodes differently in the markup-view; r=miker
browser/devtools/markupview/markup-view.js
browser/devtools/markupview/test/browser.ini
browser/devtools/markupview/test/browser_markupview_node_not_displayed_01.js
browser/devtools/markupview/test/browser_markupview_node_not_displayed_02.js
browser/devtools/markupview/test/doc_markup_not_displayed.html
browser/themes/shared/devtools/markup-view.css
toolkit/devtools/server/actors/inspector.js
--- a/browser/devtools/markupview/markup-view.js
+++ b/browser/devtools/markupview/markup-view.js
@@ -80,16 +80,19 @@ function MarkupView(aInspector, aFrame, 
   this.undo = new UndoStack();
   this.undo.installController(aControllerWindow);
 
   this._containers = new Map();
 
   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();
 
   this._boundKeyDown = this._onKeyDown.bind(this);
   this._frame.contentWindow.addEventListener("keydown", this._boundKeyDown, false);
 
   this._boundFocus = this._onFocus.bind(this);
@@ -610,16 +613,29 @@ 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.
    */
   _flashMutatedNodes: function(aMutations) {
     let addedOrEditedContainers = new Set();
     let removedContainers = new Set();
 
     for (let {type, target, added, removed} of aMutations) {
@@ -1105,16 +1121,19 @@ MarkupView.prototype = {
     this._boundKeyDown = null;
 
     this._inspector.selection.off("new-node-front", this._boundOnNewSelection);
     this._boundOnNewSelection = null;
 
     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;
 
     for (let [key, container] of this._containers) {
       container.destroy();
     }
     this._containers = null;
@@ -1263,16 +1282,19 @@ function MarkupContainer(aMarkupView, aN
   // Appending the editor element and attaching event listeners
   this.tagLine.appendChild(this.editor.elt);
 
   this._onMouseDown = this._onMouseDown.bind(this);
   this.elt.addEventListener("mousedown", this._onMouseDown, false);
 
   // Prepare the image preview tooltip data if any
   this._prepareImagePreview();
+
+  // Marking the node as shown or hidden
+  this.isDisplayed = this.node.isDisplayed;
 }
 
 MarkupContainer.prototype = {
   toString: function() {
     return "[MarkupContainer for " + this.node + "]";
   },
 
   isPreviewable: function() {
@@ -1337,16 +1359,26 @@ MarkupContainer.prototype = {
 
     return this.tooltipData.data.then(({data, size}) => {
       tooltip.setImageContent(data, size);
     }, () => {
       tooltip.setBrokenImageContent();
     });
   },
 
+  /**
+   * 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
     this.node.getImageData().then(data => {
       data.data.string().then(str => {
         clipboardHelper.copyString(str, this.markup.doc);
       });
     });
--- a/browser/devtools/markupview/test/browser.ini
+++ b/browser/devtools/markupview/test/browser.ini
@@ -1,16 +1,17 @@
 [DEFAULT]
 skip-if = e10s # Bug ?????? - devtools tests disabled with e10s
 subsuite = devtools
 support-files =
   doc_markup_edit.html
   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
   doc_markup_toggle.html
   doc_markup_tooltip.png
   head.js
   helper_attributes_test_runner.js
   helper_outerhtml_test_runner.js
@@ -21,16 +22,18 @@ support-files =
 [browser_markupview_highlight_hover_02.js]
 [browser_markupview_html_edit_01.js]
 [browser_markupview_html_edit_02.js]
 [browser_markupview_html_edit_03.js]
 [browser_markupview_image_tooltip.js]
 [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_markupview_tag_edit_01.js]
 [browser_markupview_tag_edit_02.js]
 [browser_markupview_tag_edit_03.js]
 [browser_markupview_tag_edit_04.js]
 [browser_markupview_tag_edit_05.js]
new file mode 100644
--- /dev/null
+++ b/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);
+  }
+});
new file mode 100644
--- /dev/null
+++ b/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;
+}
new file mode 100644
--- /dev/null
+++ b/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>
\ No newline at end of file
--- a/browser/themes/shared/devtools/markup-view.css
+++ b/browser/themes/shared/devtools/markup-view.css
@@ -25,16 +25,22 @@
 .theme-selected ~ .editor .theme-fg-color3,
 .theme-selected ~ .editor .theme-fg-color4,
 .theme-selected ~ .editor .theme-fg-color5,
 .theme-selected ~ .editor .theme-fg-color6,
 .theme-selected ~ .editor .theme-fg-color7 {
   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;
 }
 
 /* Preview */
 
 #previewbar {
   position: fixed;
--- a/toolkit/devtools/server/actors/inspector.js
+++ b/toolkit/devtools/server/actors/inspector.js
@@ -57,16 +57,18 @@ const {Arg, Option, method, RetVal, type
 const {LongStringActor, ShortLongString} = require("devtools/server/actors/string");
 const {Promise: promise} = Cu.import("resource://gre/modules/Promise.jsm", {});
 const object = require("sdk/util/object");
 const events = require("sdk/event/core");
 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;
 const PSEUDO_CLASSES = [":hover", ":active", ":focus"];
 const HIDDEN_CLASS = "__fx-devtools-hide-shortcut__";
 const XHTML_NS = "http://www.w3.org/1999/xhtml";
 const IMAGE_FETCHING_TIMEOUT = 500;
 // The possible completions to a ':' with added score to give certain values
@@ -172,16 +174,20 @@ exports.setValueSummaryLength = function
  */
 var NodeActor = exports.NodeActor = protocol.ActorClass({
   typeName: "domnode",
 
   initialize: function(walker, node) {
     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() {
     return "[NodeActor " + this.actorID + " for " + this.rawNode.toString() + "]";
   },
 
   /**
    * Instead of storing a connection object, the NodeActor gets its connection
@@ -222,16 +228,18 @@ var NodeActor = exports.NodeActor = prot
       // doctype attributes
       name: this.rawNode.name,
       publicId: this.rawNode.publicId,
       systemId: this.rawNode.systemId,
 
       attrs: this.writeAttrs(),
 
       pseudoClassLocks: this.writePseudoClassLocks(),
+
+      isDisplayed: this.isDisplayed,
     };
 
     if (this.isDocumentElement()) {
       form.isDocumentElement = true;
     }
 
     if (this.rawNode.nodeValue) {
       // We only include a short version of the value if it's longer than
@@ -242,16 +250,39 @@ var NodeActor = exports.NodeActor = prot
       } else {
         form.shortValue = this.rawNode.nodeValue;
       }
     }
 
     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;
     }
     return [{namespace: attr.namespace, name: attr.name, value: attr.value }
             for (attr of this.rawNode.attributes)];
   },
 
@@ -543,16 +574,22 @@ let NodeFront = protocol.FrontClass(Node
 
   get attributes() this._form.attrs,
 
   get pseudoClassLocks() this._form.pseudoClassLocks || [],
   hasPseudoClassLock: function(pseudo) {
     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));
     } else {
       return this._getNodeValue();
     }
   }, {
     impl: "_getNodeValue"
@@ -831,16 +868,20 @@ var WalkerActor = protocol.ActorClass({
       type: "pickerNodeHovered",
       node: Arg(0, "disconnectedNode")
     },
     "highlighter-ready" : {
       type: "highlighter-ready"
     },
     "highlighter-hide" : {
       type: "highlighter-hide"
+    },
+    "display-change" : {
+      type: "display-change",
+      nodes: Arg(0, "array:domnode")
     }
   },
 
   /**
    * Create the WalkerActor
    * @param DebuggerServerConnection conn
    *    The server connection.
    */
@@ -870,16 +911,20 @@ var WalkerActor = protocol.ActorClass({
     this.onFrameUnload = this.onFrameUnload.bind(this);
 
     events.on(tabActor, "will-navigate", this.onFrameUnload);
     events.on(tabActor, "navigate", this.onFrameLoad);
 
     // 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.
   form: function() {
     return {
       actor: this.actorID,
       root: this.rootNode.form()
     }
@@ -889,27 +934,32 @@ var WalkerActor = protocol.ActorClass({
     return "[WalkerActor " + this.actorID + "]";
   },
 
   destroy: function() {
     this._hoveredNode = null;
     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);
   },
 
   release: method(function() {}, { release: true }),
 
   unmanage: function(actor) {
     if (actor instanceof NodeActor) {
       if (this._activePseudoClassLocks &&
           this._activePseudoClassLocks.has(actor)) {
-        this.clearPsuedoClassLocks(actor);
+        this.clearPseudoClassLocks(actor);
       }
       this._refMap.delete(actor.rawNode);
     }
     protocol.Actor.prototype.unmanage.call(this, actor);
   },
 
   _ref: function(node) {
     let actor = this._refMap.get(node);
@@ -923,16 +973,34 @@ var WalkerActor = protocol.ActorClass({
     this._refMap.set(node, actor);
 
     if (node.nodeType === Ci.nsIDOMNode.DOCUMENT_NODE) {
       this._watchDocument(actor);
     }
     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.
    *
    * pick/cancelPick are used to pick a node on click on the content
    * document. But in their implementation prior to bug 916443, they don't allow
    * highlighting on hover.
    * The client-side now uses the highlighter actor's pick and cancelPick