Bug 950732 - Make the show-all-nodes button clickable again in the markup-view, r=jwalker

2014-01-13 16:32:19 +01:00 · 2014-01-13 16:32:19 +01:00 · b9157b42b7
--- a/browser/devtools/markupview/markup-view.js
+++ b/browser/devtools/markupview/markup-view.js
@ -1268,9 +1268,6 @@ function MarkupContainer(aMarkupView, aNode, aInspector) {
  this._onMouseDown = this._onMouseDown.bind(this);
  this.elt.addEventListener("mousedown", this._onMouseDown, false);

-  this._onClick = this._onClick.bind(this);
-  this.elt.addEventListener("click", this._onClick, false);
-
  // Prepare the image preview tooltip data if any
  this._prepareImagePreview();
 }
@ -1394,16 +1391,9 @@ MarkupContainer.prototype = {
  },

  _onMouseDown: function(event) {
-    if (event.target.nodeName !== "a") {
-      this.hovered = false;
-      this.markup.navigate(this);
-      event.stopPropagation();
-    }
-  },
-
-  _onClick: function(event) {
    let target = event.target;

+    // Target may be a resource link (generated by the output-parser)
    if (target.nodeName === "a") {
      event.stopPropagation();
      event.preventDefault();
@ -1411,6 +1401,13 @@ MarkupContainer.prototype = {
                           .tab.ownerDocument.defaultView;
      browserWin.openUILinkIn(target.href, "tab");
    }
+    // Or it may be the "show more nodes" button (which already has its onclick)
+    // Else, it's the container itself
+    else if (target.nodeName !== "button") {
+      this.hovered = false;
+      this.markup.navigate(this);
+      event.stopPropagation();
+    }
  },

  /**
@ -1544,7 +1541,11 @@ MarkupContainer.prototype = {
    // Recursively destroy children containers
    let firstChild;
    while (firstChild = this.children.firstChild) {
-      firstChild.container.destroy();
+      // Not all children of a container are containers themselves
+      // ("show more nodes" button is one example)
+      if (firstChild.container) {
+        firstChild.container.destroy();
+      }
      this.children.removeChild(firstChild);
    }

@ -1553,7 +1554,6 @@ MarkupContainer.prototype = {
    this.elt.removeEventListener("mouseover", this._onMouseOver, false);
    this.elt.removeEventListener("mouseout", this._onMouseOut, false);
    this.elt.removeEventListener("mousedown", this._onMouseDown, false);
-    this.elt.removeEventListener("click", this._onClick, false);
    this.expander.removeEventListener("click", this._onToggle, false);

    // Destroy my editor
--- a/browser/devtools/markupview/test/browser.ini
+++ b/browser/devtools/markupview/test/browser.ini
@ -6,6 +6,7 @@ support-files =
  browser_inspector_markup_navigation.html
  browser_inspector_markup_subset.html
  browser_inspector_markup_765105_tooltip.png
+  browser_inspector_markup_950732.html
  head.js

 [browser_bug896181_css_mixed_completion_new_attribute.js]
@ -19,3 +20,4 @@ skip-if = true
 [browser_inspector_markup_navigation.js]
 [browser_inspector_markup_subset.js]
 [browser_inspector_markup_765105_tooltip.js]
+[browser_inspector_markup_950732.js]
--- a/browser/devtools/markupview/test/browser_inspector_markup_950732.html
+++ b/browser/devtools/markupview/test/browser_inspector_markup_950732.html
@ -0,0 +1,33 @@
+<!DOCTYPE html>
+
+<html class="html">
+  <body class="body">
+    <ul>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+        <li>some content</li>
+    </ul>
+  </body>
+</html>
--- a/browser/devtools/markupview/test/browser_inspector_markup_950732.js
+++ b/browser/devtools/markupview/test/browser_inspector_markup_950732.js
@ -0,0 +1,105 @@
+/* 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.
+ */
+
+let {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+let promise = devtools.require("sdk/core/promise");
+let {Task} = Cu.import("resource://gre/modules/Task.jsm", {});
+
+// Make sure nodes are hidden when there are more than 5 in a row
+registerCleanupFunction(function() {
+  Services.prefs.clearUserPref("devtools.markup.pagesize");
+});
+Services.prefs.setIntPref("devtools.markup.pagesize", 5);
+
+function test() {
+  waitForExplicitFinish();
+
+  let doc;
+  let inspector;
+  let markup;
+
+  gBrowser.selectedTab = gBrowser.addTab();
+  gBrowser.selectedBrowser.addEventListener("load", function onload() {
+    gBrowser.selectedBrowser.removeEventListener("load", onload, true);
+    doc = content.document;
+    waitForFocus(runTests, content);
+  }, true);
+  content.location = "http://mochi.test:8888/browser/browser/devtools/markupview/test/browser_inspector_markup_950732.html";
+
+  function runTests() {
+    Task.spawn(function() {
+      yield openMarkupView();
+      yield selectUL();
+      yield reloadPage();
+      yield showAllNodes();
+
+      assertAllNodesAreVisible();
+      finishUp();
+    }).then(null, Cu.reportError);
+  }
+
+  function openMarkupView() {
+    let deferred = promise.defer();
+
+    var target = TargetFactory.forTab(gBrowser.selectedTab);
+    let toolbox = gDevTools.showToolbox(target, "inspector").then(function(toolbox) {
+      inspector = toolbox.getCurrentPanel();
+      markup = inspector.markup;
+      inspector.once("inspector-updated", deferred.resolve);
+    });
+
+    return deferred.promise;
+  }
+
+  function selectUL() {
+    let deferred = promise.defer();
+
+    let container = getContainerForRawNode(markup, doc.querySelector("ul"));
+    let win = container.elt.ownerDocument.defaultView;
+
+    EventUtils.sendMouseEvent({type: "mousedown"}, container.elt, win);
+    inspector.once("inspector-updated", deferred.resolve);
+
+    return deferred.promise;
+  }
+
+  function reloadPage() {
+    let deferred = promise.defer();
+
+    inspector.once("new-root", () => {
+      doc = content.document;
+      markup = inspector.markup;
+      markup._waitForChildren().then(deferred.resolve);
+    });
+    content.location.reload();
+
+    return deferred.promise;
+  }
+
+  function showAllNodes() {
+    let container = getContainerForRawNode(markup, doc.querySelector("ul"));
+    let button = container.elt.querySelector("button");
+    let win = button.ownerDocument.defaultView;
+
+    EventUtils.sendMouseEvent({type: "click"}, button, win);
+    return markup._waitForChildren();
+  }
+
+  function assertAllNodesAreVisible() {
+    let ul = doc.querySelector("ul");
+    let container = getContainerForRawNode(markup, ul);
+    ok(!container.elt.querySelector("button"), "All nodes button isn't here");
+    is(container.children.childNodes.length, ul.children.length);
+  }
+
+  function finishUp() {
+    doc = inspector = markup = null;
+    gBrowser.removeCurrentTab();
+    finish();
+  }
+}
--- a/browser/devtools/markupview/test/browser_inspector_markup_subset.js
+++ b/browser/devtools/markupview/test/browser_inspector_markup_subset.js
@ -132,7 +132,10 @@ function test() {
    // Make sure that clicking the "more" button loads all the nodes.
    let container = getContainerForRawNode(markup, doc.querySelector("body"));
    let button = container.elt.querySelector("button");
-    button.click();
+    let win = button.ownerDocument.defaultView;
+
+    EventUtils.sendMouseEvent({type: "click"}, button, win);
+
    markup._waitForChildren().then(() => {
      assertChildren("abcdefghijklmnopqrstuvwxyz");
      finishUp();