Bug 796006 - Don't fully expand nodes with large amounts of children in markup panel. r=jwalker

browser/devtools/markupview/MarkupView.jsm

@@ -12,6 +12,7 @@ const Ci = Components.interfaces;
 const PAGE_SIZE = 10;
 
 const PREVIEW_AREA = 700;
+const DEFAULT_MAX_CHILDREN = 100;
 
 this.EXPORTED_SYMBOLS = ["MarkupView"];
 
@@ -20,6 +21,7 @@ Cu.import("resource:///modules/devtools/CssRuleView.jsm");
 Cu.import("resource:///modules/devtools/Templater.jsm");
 Cu.import("resource:///modules/devtools/Undo.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 /**
  * Vocabulary for the purposes of this file:
@@ -46,6 +48,12 @@ this.MarkupView = function MarkupView(aInspector, aFrame, aControllerWindow)
   this.doc = this._frame.contentDocument;
   this._elt = this.doc.querySelector("#root");
 
+  try {
+    this.maxChildren = Services.prefs.getIntPref("devtools.markup.pagesize");
+  } catch(ex) {
+    this.maxChildren = DEFAULT_MAX_CHILDREN;
+  }
+
   this.undo = new UndoStack();
   this.undo.installController(aControllerWindow);
 
@@ -69,7 +77,7 @@ this.MarkupView = function MarkupView(aInspector, aFrame, aControllerWindow)
 MarkupView.prototype = {
   _selectedContainer: null,
 
-  template: function MT_template(aName, aDest, aOptions)
+  template: function MT_template(aName, aDest, aOptions={stack: "markup-view.xhtml"})
   {
     let node = this.doc.getElementById("template-" + aName).cloneNode(true);
     node.removeAttribute("id");
@@ -288,7 +296,6 @@ MarkupView.prototype = {
     let walker = documentWalker(aNode);
     let parent = walker.parentNode();
     if (parent) {
-      // Make sure parents of this node are imported too.
       var container = new MarkupContainer(this, aNode);
     } else {
       var container = new RootContainer(this, aNode);
@@ -298,12 +305,15 @@ MarkupView.prototype = {
         // Fake a childList mutation here.
         this._mutationObserver([{target: aEvent.target, type: "childList"}]);
       }.bind(this), true);
-
     }
 
     this._containers.set(aNode, container);
+    // FIXME: set an expando to prevent the the wrapper from disappearing
+    // See bug 819131 for details.
+    aNode.__preserveHack = true;
     container.expanded = aExpand;
 
+    container.childrenDirty = true;
     this._updateChildren(container);
 
     if (parent) {
@@ -327,6 +337,7 @@ MarkupView.prototype = {
       if (mutation.type === "attributes" || mutation.type === "characterData") {
         container.update();
       } else if (mutation.type === "childList") {
+        container.childrenDirty = true;
         this._updateChildren(container);
       }
     }
@@ -339,10 +350,12 @@ MarkupView.prototype = {
    */
   showNode: function MT_showNode(aNode, centered)
   {
-    this.importNode(aNode);
+    let container = this.importNode(aNode);
+    this._updateChildren(container);
     let walker = documentWalker(aNode);
     let parent;
     while (parent = walker.parentNode()) {
+      this._updateChildren(this.getContainer(parent));
       this.expandNode(parent);
     }
     LayoutHelpers.scrollIntoViewIfNeeded(this._containers.get(aNode).editor.elt, centered);
@@ -421,9 +434,33 @@ MarkupView.prototype = {
       this._selectedContainer.selected = true;
     }
 
+    this._ensureSelectionVisible();
+    this._selectedContainer.focus();
+
     return true;
   },
 
+  /**
+   * Make sure that every ancestor of the selection are updated
+   * and included in the list of visible children.
+   */
+  _ensureSelectionVisible: function MT_ensureSelectionVisible()
+  {
+    let node = this._selectedContainer.node;
+    let walker = documentWalker(node);
+    while (node) {
+      let container = this._containers.get(node);
+      let parent = walker.parentNode();
+      if (!container.elt.parentNode) {
+        let parentContainer = this._containers.get(parent);
+        parentContainer.childrenDirty = true;
+        this._updateChildren(parentContainer, node);
+      }
+
+      node = parent;
+    }
+  },
+
   /**
    * Unmark selected node (no node selected).
    */
@@ -448,29 +485,139 @@ MarkupView.prototype = {
   /**
    * Make sure all children of the given container's node are
    * imported and attached to the container in the right order.
+   * @param aCentered If provided, this child will be included
+   *        in the visible subset, and will be roughly centered
+   *        in that list.
    */
-  _updateChildren: function MT__updateChildren(aContainer)
+  _updateChildren: function MT__updateChildren(aContainer, aCentered)
   {
+    if (!aContainer.childrenDirty) {
+      return false;
+    }
+
     // Get a tree walker pointing at the first child of the node.
     let treeWalker = documentWalker(aContainer.node);
     let child = treeWalker.firstChild();
     aContainer.hasChildren = !!child;
-    if (aContainer.expanded) {
-      let lastContainer = null;
-      while (child) {
+
+    if (!aContainer.expanded) {
+      return;
+    }
+
+    aContainer.childrenDirty = false;
+
+    let children = this._getVisibleChildren(aContainer, aCentered);
+    let fragment = this.doc.createDocumentFragment();
+
+    for (child of children.children) {
       let container = this.importNode(child, false);
-
-        // Make sure children are in the right order.
-        let before = lastContainer ? lastContainer.nextSibling : aContainer.children.firstChild;
-        aContainer.children.insertBefore(container.elt, before);
-        lastContainer = container.elt;
-        child = treeWalker.nextSibling();
+      fragment.appendChild(container.elt);
     }
 
-      while (aContainer.children.lastChild != lastContainer) {
-        aContainer.children.removeChild(aContainer.children.lastChild);
+    while (aContainer.children.firstChild) {
+      aContainer.children.removeChild(aContainer.children.firstChild);
+    }
+
+    if (!(children.hasFirst && children.hasLast)) {
+      let data = {
+        showing: this.strings.GetStringFromName("markupView.more.showing"),
+        showAll: this.strings.formatStringFromName(
+                  "markupView.more.showAll",
+                  [aContainer.node.children.length.toString()], 1),
+        allButtonClick: function() {
+          aContainer.maxChildren = -1;
+          aContainer.childrenDirty = true;
+          this._updateChildren(aContainer);
+        }.bind(this)
+      };
+
+      if (!children.hasFirst) {
+        let span = this.template("more-nodes", data);
+        fragment.insertBefore(span, fragment.firstChild);
+      }
+      if (!children.hasLast) {
+        let span = this.template("more-nodes", data);
+        fragment.appendChild(span);
       }
     }
+
+    aContainer.children.appendChild(fragment);
+
+    return true;
+  },
+
+  /**
+   * Return a list of the children to display for this container.
+   */
+  _getVisibleChildren: function MV__getVisibleChildren(aContainer, aCentered)
+  {
+    let maxChildren = aContainer.maxChildren || this.maxChildren;
+    if (maxChildren == -1) {
+      maxChildren = Number.MAX_VALUE;
+    }
+    let firstChild = documentWalker(aContainer.node).firstChild();
+    let lastChild = documentWalker(aContainer.node).lastChild();
+
+    if (!firstChild) {
+      // No children, we're done.
+      return { hasFirst: true, hasLast: true, children: [] };
+    }
+
+    // By default try to put the selected child in the middle of the list.
+    let start = aCentered || firstChild;
+
+    // Start by reading backward from the starting point....
+    let nodes = [];
+    let backwardWalker = documentWalker(start);
+    if (backwardWalker.previousSibling()) {
+      let backwardCount = Math.floor(maxChildren / 2);
+      let backwardNodes = this._readBackward(backwardWalker, backwardCount);
+      nodes = backwardNodes;
+    }
+
+    // Then read forward by any slack left in the max children...
+    let forwardWalker = documentWalker(start);
+    let forwardCount = maxChildren - nodes.length;
+    nodes = nodes.concat(this._readForward(forwardWalker, forwardCount));
+
+    // If there's any room left, it means we've run all the way to the end.
+    // In that case, there might still be more items at the front.
+    let remaining = maxChildren - nodes.length;
+    if (remaining > 0 && nodes[0] != firstChild) {
+      let firstNodes = this._readBackward(backwardWalker, remaining);
+
+      // Then put it all back together.
+      nodes = firstNodes.concat(nodes);
+    }
+
+    return {
+      hasFirst: nodes[0] == firstChild,
+      hasLast: nodes[nodes.length - 1] == lastChild,
+      children: nodes
+    };
+  },
+
+  _readForward: function MV__readForward(aWalker, aCount)
+  {
+    let ret = [];
+    let node = aWalker.currentNode;
+    do {
+      ret.push(node);
+      node = aWalker.nextSibling();
+    } while (node && --aCount);
+    return ret;
+  },
+
+  _readBackward: function MV__readBackward(aWalker, aCount)
+  {
+    let ret = [];
+    let node = aWalker.currentNode;
+    do {
+      ret.push(node);
+      node = aWalker.previousSibling();
+    } while(node && --aCount);
+    ret.reverse();
+    return ret;
   },
 
   /**
@@ -618,9 +765,7 @@ function MarkupContainer(aMarkupView, aNode)
   this.expander = null;
   this.codeBox = null;
   this.children = null;
-  let options = { stack: "markup-view.xhtml" };
-  this.markup.template("container", this, options);
-
+  this.markup.template("container", this);
   this.elt.container = this;
 
   this.expander.addEventListener("click", function() {
@@ -734,7 +879,7 @@ MarkupContainer.prototype = {
     if (focusable) {
       focusable.focus();
     }
-  }
+  },
 }
 
 /**
@@ -841,13 +986,12 @@ function ElementEditor(aContainer, aNode)
   this.attrList = null;
   this.newAttr = null;
   this.closeElt = null;
-  let options = { stack: "markup-view.xhtml" };
 
   // Create the main editor
-  this.template("element", this, options);
+  this.template("element", this);
 
   // Create the closing tag
-  this.template("elementClose", this, options);
+  this.template("elementClose", this);
 
   // Make the tag name editable (unless this is a document element)
   if (aNode != aNode.ownerDocument.documentElement) {
@@ -927,8 +1071,7 @@ ElementEditor.prototype = {
       let data = {
         attrName: aAttr.name,
       };
-      let options = { stack: "markup-view.xhtml" };
-      this.template("attribute", data, options);
+      this.template("attribute", data);
       var {attr, inner, name, val} = data;
 
       // Figure out where we should place the attribute.
@@ -1260,3 +1403,8 @@ function whitespaceTextFilter(aNode)
       return Ci.nsIDOMNodeFilter.FILTER_ACCEPT;
     }
 }
+
+XPCOMUtils.defineLazyGetter(MarkupView.prototype, "strings", function () {
+  return Services.strings.createBundle(
+          "chrome://browser/locale/devtools/inspector.properties");
+});

browser/devtools/markupview/markup-view.xhtml

@@ -16,6 +16,8 @@
   <div id="templates" style="display:none">
     <ul>
       <li id="template-container" save="${elt}" class="container"><span save="${expander}" class="expander"></span><span save="${codeBox}" class="codebox"><ul save="${children}" class="children"></ul></span></li>
+
+      <li id="template-more-nodes" class="more-nodes devtools-class-comment" save="${elt}"><span>${showing}</span> <button href="#" onclick="${allButtonClick}">${showAll}</button></li>
     </ul>
 
     <span id="template-element" save="${elt}" class="editor"><span>&lt;</span><span save="${tag}" class="tagname devtools-theme-tagname"></span><span save="${attrList}"></span><span save="${newAttr}" class="newattr" tabindex="0"></span>&gt;</span>

browser/devtools/markupview/test/Makefile.in

@@ -18,6 +18,8 @@ _BROWSER_FILES = \
 		browser_inspector_markup_mutation.js \
 		browser_inspector_markup_edit.html \
     browser_inspector_markup_edit.js \
+    browser_inspector_markup_subset.html \
+    browser_inspector_markup_subset.js \
 		head.js \
 		$(NULL)
 

browser/devtools/markupview/test/browser_inspector_markup_subset.html

@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+
+<html class="html">
+  <body class="body">
+    <div id="a"></div>
+    <div id="b"></div>
+    <div id="c"></div>
+    <div id="d"></div>
+    <div id="e"></div>
+    <div id="f"></div>
+    <div id="g"></div>
+    <div id="h"></div>
+    <div id="i"></div>
+    <div id="j"></div>
+    <div id="k"></div>
+    <div id="l"></div>
+    <div id="m"></div>
+    <div id="n"></div>
+    <div id="o"></div>
+    <div id="p"></div>
+    <div id="q"></div>
+    <div id="r"></div>
+    <div id="s"></div>
+    <div id="t"></div>
+    <div id="u"></div>
+    <div id="v"></div>
+    <div id="w"></div>
+    <div id="x"></div>
+    <div id="y"></div>
+    <div id="z"></div>
+  </body>
+</html>

browser/devtools/markupview/test/browser_inspector_markup_subset.js

@@ -0,0 +1,146 @@
+/* Any copyright", " is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that the markup view loads only as many nodes as specified
+ * by the devtools.markup.pagesize preference.
+ */
+
+registerCleanupFunction(function() {
+  Services.prefs.clearUserPref("devtools.markup.pagesize");
+});
+Services.prefs.setIntPref("devtools.markup.pagesize", 5);
+
+
+function test() {
+  waitForExplicitFinish();
+
+  // Will hold the doc we're viewing
+  let doc;
+
+  let inspector;
+
+  // Holds the MarkupTool object we're testing.
+  let markup;
+
+  function assertChildren(expected)
+  {
+    let container = markup.getContainer(doc.querySelector("body"));
+    let found = [];
+    for (let child of container.children.children) {
+      if (child.classList.contains("more-nodes")) {
+        found += "*more*";
+      } else {
+        found += child.container.node.getAttribute("id");
+      }
+    }
+    is(expected, found, "Got the expected children.");
+  }
+
+  function forceReload()
+  {
+    let container = markup.getContainer(doc.querySelector("body"));
+    container.childrenDirty = true;
+  }
+
+  let selections = [
+    {
+      desc: "Select the first item",
+      selector: "#a",
+      before: function() {
+      },
+      after: function() {
+        assertChildren("abcde*more*");
+      }
+    },
+    {
+      desc: "Select the last item",
+      selector: "#z",
+      before: function() {},
+      after: function() {
+        assertChildren("*more*vwxyz");
+      }
+    },
+    {
+      desc: "Select an already-visible item",
+      selector: "#v",
+      before: function() {},
+      after: function() {
+        // Because "v" was already visible, we shouldn't have loaded
+        // a different page.
+        assertChildren("*more*vwxyz");
+      },
+    },
+    {
+      desc: "Verify childrenDirty reloads the page",
+      selector: "#w",
+      before: function() {
+        forceReload();
+      },
+      after: function() {
+        // But now that we don't already have a loaded page, selecting
+        // w should center around w.
+        assertChildren("*more*uvwxy*more*");
+      },
+    },
+  ];
+
+  // Create the helper tab for parsing...
+  gBrowser.selectedTab = gBrowser.addTab();
+  gBrowser.selectedBrowser.addEventListener("load", function onload() {
+    gBrowser.selectedBrowser.removeEventListener("load", onload, true);
+    doc = content.document;
+    waitForFocus(setupTest, content);
+  }, true);
+  content.location = "http://mochi.test:8888/browser/browser/devtools/markupview/test/browser_inspector_markup_subset.html";
+
+  function setupTest() {
+    var target = TargetFactory.forTab(gBrowser.selectedTab);
+    let toolbox = gDevTools.showToolbox(target, "inspector").then(function(toolbox) {
+      toolbox.once("inspector-selected", function SE_selected(id, aInspector) {
+        inspector = aInspector;
+        markup = inspector.markup;
+        runNextSelection();
+      });
+    });
+  }
+
+  function runTests() {
+    inspector.selection.once("new-node", startTests);
+    executeSoon(function() {
+      inspector.selection.setNode(doc.body);
+    });
+  }
+
+  function runNextSelection() {
+    let selection = selections.shift();
+    if (!selection) {
+      clickMore();
+      return;
+    }
+
+    info(selection.desc);
+    selection.before();
+    inspector.selection.once("new-node", function() {
+      selection.after();
+      runNextSelection();
+    });
+    inspector.selection.setNode(doc.querySelector(selection.selector));
+  }
+
+  function clickMore() {
+    info("Check that clicking more loads the whole thing.");
+    // Make sure that clicking the "more" button loads all the nodes.
+    let container = markup.getContainer(doc.querySelector("body"));
+    let button = container.elt.querySelector("button");
+    button.click();
+    assertChildren("abcdefghijklmnopqrstuvwxyz");
+    finishUp();
+  }
+
+  function finishUp() {
+    doc = inspector = null;
+    gBrowser.removeCurrentTab();
+    finish();
+  }
+}

browser/locales/en-US/chrome/browser/devtools/inspector.properties

@@ -33,4 +33,10 @@ nodeMenu.tooltiptext=Node operations
 inspector.label=Inspector
 inspector.commandkey=I
 inspector.accesskey=I
+
+# LOCALIZATION NOTE (markupView.more.*)
+# When there are too many nodes to load at once, we will offer to
+# show all the nodes.
+markupView.more.showing=Some nodes were hidden.
+markupView.more.showAll=Show All %S Nodes
 inspector.tooltip=DOM and Style Inspector

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

@@ -49,6 +49,10 @@ li.container {
   -moz-appearance: treetwistyopen;
 }
 
+.more-nodes {
+  padding-left: 16px;
+}
+
 .styleinspector-propertyeditor {
   border: 1px solid #CCC;
 }

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

@@ -52,6 +52,10 @@ li.container {
   -moz-appearance: treetwistyopen;
 }
 
+.more-nodes {
+  padding-left: 16px;
+}
+
 .styleinspector-propertyeditor {
   border: 1px solid #CCC;
 }

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

@@ -54,6 +54,10 @@ li.container {
   background-image: url("chrome://global/skin/tree/twisty-open.png");
 }
 
+.more-nodes {
+  padding-left: 16px;
+}
+
 .styleinspector-propertyeditor {
   border: 1px solid #CCC;
 }