Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 06 Jan 2016 15:01:24 -0800
changeset 321227 131c9e4a6575
parent 321226 154482173e40
child 512875 c3bc8e7160c5
push id9351
push userjdescottes@mozilla.com
push dateWed, 13 Jan 2016 08:24:50 +0000
reviewersbgrins
bugs1157469
milestone46.0a1
Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins Inspector actor now uses EventListenerService:addListenerChangeListener to propagate event listener changes as mutations with type 'events'. markupview will now trigger an update when receiving a mutation of type 'event', and the event bubble display will be updated
devtools/client/inspector/markup/markup.js
devtools/client/inspector/markup/test/browser_markup_events.js
devtools/client/inspector/markup/test/doc_markup_events.html
devtools/client/inspector/markup/test/helper_events_test_runner.js
devtools/server/actors/inspector.js
devtools/server/tests/mochitest/chrome.ini
devtools/server/tests/mochitest/test_inspector-mutations-events.html
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -836,24 +836,23 @@ MarkupView.prototype = {
       }
 
       let container = this.getContainer(target);
       if (!container) {
         // Container might not exist if this came from a load event for a node
         // we're not viewing.
         continue;
       }
-      if (type === "attributes" || type === "characterData") {
+      if (type === "attributes" || type === "characterData"
+        || type === "events" || type === "pseudoClassLock") {
         container.update();
       } else if (type === "childList" || type === "nativeAnonymousChildList") {
         container.childrenDirty = true;
         // Update the children to take care of changes in the markup view DOM.
         this._updateChildren(container, {flash: true});
-      } else if (type === "pseudoClassLock") {
-        container.update();
       }
     }
 
     this._waitForChildren().then(() => {
       if (this._destroyer) {
         console.warn("Could not fully update after markup mutations, " +
           "the markup-view was destroyed while waiting for children.");
         return;
@@ -2542,17 +2541,16 @@ function ElementEditor(aContainer, aNode
         undoMods.apply();
       });
     }
   });
 
   let tagName = this.node.nodeName.toLowerCase();
   this.tag.textContent = tagName;
   this.closeTag.textContent = tagName;
-  this.eventNode.style.display = this.node.hasEventListeners ? "inline-block" : "none";
 
   this.update();
   this.initialized = true;
 }
 
 ElementEditor.prototype = {
 
   set selected(aValue) {
@@ -2638,16 +2636,20 @@ ElementEditor.prototype = {
         // But not if this is the first time the editor instance has
         // been created.
         if (this.initialized) {
           this.flashAttribute(attr.name);
         }
       }
     }
 
+    // Update the event bubble display
+    this.eventNode.style.display = this.node.hasEventListeners ?
+      "inline-block" : "none";
+
     this.updateTextEditor();
   },
 
   /**
    * Update the inline text editor in case of a single text child node.
    */
   updateTextEditor: function() {
     let node = this.node.singleTextChild;
--- a/devtools/client/inspector/markup/test/browser_markup_events.js
+++ b/devtools/client/inspector/markup/test/browser_markup_events.js
@@ -147,11 +147,47 @@ const TEST_DATA = [
           "DOM2"
         ],
         handler: 'function boundClickHandler(event) {\n' +
                  '  alert("Bound event clicked");\n' +
                  '}'
       }
     ]
   },
+  // #noevents tests check that dynamically added events are properly displayed
+  // in the markupview
+  {
+    selector: "#noevents",
+    expected: []
+  },
+  {
+    selector: "#noevents",
+    beforeTest: function* (inspector, testActor) {
+      let nodeMutated = inspector.once("markupmutation");
+      yield testActor.eval("window.wrappedJSObject.addNoeventsClickHandler();");
+      yield nodeMutated;
+    },
+    expected: [
+      {
+        type: "click",
+        filename: TEST_URL + ":106",
+        attributes: [
+          "Bubbling",
+          "DOM2"
+        ],
+        handler: 'function noeventsClickHandler(event) {\n' +
+                 '  alert("noevents has an event listener");\n' +
+                 '}'
+      }
+    ]
+  },
+  {
+    selector: "#noevents",
+    beforeTest: function* (inspector, testActor) {
+      let nodeMutated = inspector.once("markupmutation");
+      yield testActor.eval("window.wrappedJSObject.removeNoeventsClickHandler();");
+      yield nodeMutated;
+    },
+    expected: []
+  },
 ];
 
 add_task(runEventPopupTests);
--- a/devtools/client/inspector/markup/test/doc_markup_events.html
+++ b/devtools/client/inspector/markup/test/doc_markup_events.html
@@ -97,16 +97,30 @@
         boundhe.addEventListener("click", this);
       }
 
       boundHandleEventClick.prototype = {
         handleEvent: function() {
           alert("boundHandleEvent clicked");
         }
       };
+
+      function noeventsClickHandler(event) {
+        alert("noevents has an event listener");
+      };
+
+      function addNoeventsClickHandler() {
+        let noevents = document.getElementById("noevents");
+        noevents.addEventListener("click", noeventsClickHandler);
+      };
+
+      function removeNoeventsClickHandler() {
+        let noevents = document.getElementById("noevents");
+        noevents.removeEventListener("click", noeventsClickHandler);
+      };
     </script>
   </head>
   <body onload="init();">
     <div id="container">
       <div>1</div>
       <div>2</div>
       <div>3</div>
       <div>4</div>
--- a/devtools/client/inspector/markup/test/helper_events_test_runner.js
+++ b/devtools/client/inspector/markup/test/helper_events_test_runner.js
@@ -2,40 +2,64 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
  * Generator function that runs checkEventsForNode() for each object in the
  * TEST_DATA array.
  */
 function* runEventPopupTests() {
-  let {inspector} = yield addTab(TEST_URL).then(openInspector);
+  let {inspector, testActor} = yield addTab(TEST_URL).then(openInspector);
 
   yield inspector.markup.expandAll();
 
-  for (let {selector, expected} of TEST_DATA) {
-    yield checkEventsForNode(selector, expected, inspector);
+  for (let test of TEST_DATA) {
+    yield checkEventsForNode(test, inspector, testActor);
   }
 
   // Wait for promises to avoid leaks when running this as a single test.
   // We need to do this because we have opened a bunch of popups and don't them
   // to affect other test runs when they are GCd.
   yield promiseNextTick();
 }
 
 /**
  * Generator function that takes a selector and expected results and returns
  * the event info.
  *
- * @param {String} selector
- *        Selector pointing at the node to be inspected
+ * @param {Object} test
+ *  A test object should contain the following properties:
+ *        - selector {String} a css selector targeting the node to edit
+ *        - expected {Array} array of expected event objects
+ *          - type {String} event type
+ *          - filename {String} filename:line where the evt handler is defined
+ *          - attributes {Array} array of event attributes ({String})
+ *          - handler {String} string representation of the handler
+ *        - beforeTest {Function} (optional) a function to execute on the page
+ *        before running the test
+ * @param {InspectorPanel} inspector The instance of InspectorPanel currently
+ * opened
+ * @param {TestActorFront} testActor
  */
-function* checkEventsForNode(selector, expected, inspector) {
+function* checkEventsForNode(test, inspector, testActor) {
+  let {selector, expected, beforeTest} = test;
   let container = yield getContainerForSelector(selector, inspector);
+
+  if (typeof beforeTest === "function") {
+    yield beforeTest(inspector, testActor);
+  }
+
   let evHolder = container.elt.querySelector(".markupview-events");
+
+  if (expected.length === 0) {
+    // if no event is expected, simply check that the event bubble is hidden
+    is(evHolder.style.display, "none", "event bubble should be hidden");
+    return;
+  }
+
   let tooltip = inspector.markup.tooltip;
 
   yield selectNode(selector, inspector);
 
   // Click button to show tooltip
   info("Clicking evHolder");
   EventUtils.synthesizeMouseAtCenter(evHolder, {}, inspector.markup.doc.defaultView);
   yield tooltip.once("shown");
--- a/devtools/server/actors/inspector.js
+++ b/devtools/server/actors/inspector.js
@@ -858,16 +858,18 @@ var NodeFront = protocol.FrontClass(Node
           value: change.newValue
         });
       }
     } else if (change.type === "characterData") {
       this._form.shortValue = change.newValue;
       this._form.incompleteValue = change.incompleteValue;
     } else if (change.type === "pseudoClassLock") {
       this._form.pseudoClassLocks = change.pseudoClassLocks;
+    } else if (change.type === "events") {
+      this._form.hasEventListeners = change.hasEventListeners;
     }
   },
 
   // Some accessors to make NodeFront feel more like an nsIDOMNode
 
   get id() {
     return this.getAttribute("id");
   },
@@ -1343,16 +1345,42 @@ var WalkerActor = protocol.ActorClass({
     // managed.
     this.rootNode = this.document();
 
     this.layoutChangeObserver = getLayoutChangesObserver(this.tabActor);
     this._onReflows = this._onReflows.bind(this);
     this.layoutChangeObserver.on("reflows", this._onReflows);
     this._onResize = this._onResize.bind(this);
     this.layoutChangeObserver.on("resize", this._onResize);
+
+    this._onEventListenerChange = this._onEventListenerChange.bind(this);
+    eventListenerService.addListenerChangeListener(this._onEventListenerChange);
+  },
+
+  /**
+   * Callback for eventListenerService.addListenerChangeListener
+   * @param nsISimpleEnumerator changesEnum
+   *    enumerator of nsIEventListenerChange
+   */
+  _onEventListenerChange: function(changesEnum) {
+    let changes = changesEnum.enumerate();
+    while (changes.hasMoreElements()) {
+      let current = changes.getNext().QueryInterface(Ci.nsIEventListenerChange);
+      let target = current.target;
+
+      if (this._refMap.has(target)) {
+        let actor = this._refMap.get(target);
+        let mutation = {
+          type: "events",
+          target: actor.actorID,
+          hasEventListeners: actor._hasEventListeners
+        };
+        this.queueMutation(mutation);
+      }
+    }
   },
 
   // Returns the JSON representation of this object over the wire.
   form: function() {
     return {
       actor: this.actorID,
       root: this.rootNode.form(),
       traits: {
@@ -1409,16 +1437,19 @@ var WalkerActor = protocol.ActorClass({
 
       this.walkerSearch.destroy();
 
       this.layoutChangeObserver.off("reflows", this._onReflows);
       this.layoutChangeObserver.off("resize", this._onResize);
       this.layoutChangeObserver = null;
       releaseLayoutChangesObserver(this.tabActor);
 
+      eventListenerService.removeListenerChangeListener(
+        this._onEventListenerChange);
+
       this.onMutations = null;
 
       this.tabActor = null;
 
       events.emit(this, "destroyed");
     } catch(e) {
       console.error(e);
     }
--- a/devtools/server/tests/mochitest/chrome.ini
+++ b/devtools/server/tests/mochitest/chrome.ini
@@ -64,16 +64,17 @@ skip-if = buildapp == 'mulet'
 [test_inspector_getImageDataFromURL.html]
 skip-if = buildapp == 'mulet'
 [test_inspector_getImageData-wait-for-load.html]
 skip-if = buildapp == 'mulet'
 [test_inspector_getNodeFromActor.html]
 [test_inspector-hide.html]
 [test_inspector-insert.html]
 [test_inspector-mutations-attr.html]
+[test_inspector-mutations-events.html]
 [test_inspector-mutations-childlist.html]
 [test_inspector-mutations-frameload.html]
 [test_inspector-mutations-value.html]
 [test_inspector-pseudoclass-lock.html]
 [test_inspector-release.html]
 [test_inspector-reload.html]
 [test_inspector-remove.html]
 [test_inspector-resize.html]
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/mochitest/test_inspector-mutations-events.html
@@ -0,0 +1,184 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1157469
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1157469</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  <script type="application/javascript;version=1.8" src="inspector-helpers.js"></script>
+  <script type="application/javascript;version=1.8">
+
+window.onload = function() {
+
+  const Cu = Components.utils;
+  Cu.import("resource://gre/modules/devtools/Loader.jsm");
+  const {InspectorFront} =
+    devtools.require("devtools/server/actors/inspector");
+
+  SimpleTest.waitForExplicitFinish();
+
+  let inspectee = null;
+  let inspector = null;
+  let walker = null;
+  let eventListener1 = function () {};
+  let eventListener2 = function () {};
+  let eventNode1;
+  let eventNode2;
+  let eventFront1;
+  let eventFront2;
+
+  addAsyncTest(function* setup() {
+    info ("Setting up inspector and walker actors.");
+    let url = document.getElementById("inspectorContent").href;
+
+    yield new Promise(resolve => {
+      attachURL(url, function(err, client, tab, doc) {
+        inspectee = doc;
+        inspector = InspectorFront(client, tab);
+        resolve();
+      });
+    });
+
+    walker = yield inspector.getWalker();
+    ok(walker, "getWalker() should return an actor.");
+
+    runNextTest();
+  });
+
+  addAsyncTest(function* setupEventTest() {
+    eventNode1 = inspectee.querySelector("#a")
+    eventNode2 = inspectee.querySelector("#b")
+
+    eventFront1 = yield walker.querySelector(walker.rootNode, "#a");
+    eventFront2 = yield walker.querySelector(walker.rootNode, "#b");
+
+    runNextTest();
+  });
+
+  addAsyncTest(function* testChangeEventListenerOnSingleNode() {
+    checkNodesHaveNoEventListener();
+
+    info("add event listener on a single node");
+    eventNode1.addEventListener("click", eventListener1);
+
+    let mutations = yield waitForMutations();
+    is(mutations.length, 1, "one mutation expected");
+    is(mutations[0].target, eventFront1, "mutation targets eventFront1");
+    is(mutations[0].type, "events", "mutation type is events");
+    is(mutations[0].hasEventListeners, true, "mutation target should have event listeners");
+    is(eventFront1.hasEventListeners, true, "eventFront1 should have event listeners");
+
+    info("remove event listener on a single node");
+    eventNode1.removeEventListener("click", eventListener1);
+
+    mutations = yield waitForMutations();
+    is(mutations.length, 1, "one mutation expected");
+    is(mutations[0].target, eventFront1, "mutation targets eventFront1");
+    is(mutations[0].type, "events", "mutation type is events");
+    is(mutations[0].hasEventListeners, false, "mutation target should have no event listeners");
+    is(eventFront1.hasEventListeners, false, "eventFront1 should have no event listeners");
+
+    info("perform several event listener changes on a single node")
+    eventNode1.addEventListener("click", eventListener1);
+    eventNode1.addEventListener("click", eventListener2);
+    eventNode1.removeEventListener("click", eventListener1);
+    eventNode1.removeEventListener("click", eventListener2);
+
+    mutations = yield waitForMutations();
+    is(mutations.length, 1, "one mutation expected");
+    is(mutations[0].target, eventFront1, "mutation targets eventFront1");
+    is(mutations[0].type, "events", "mutation type is events");
+    is(mutations[0].hasEventListeners, false, "no event listener expected on mutation target");
+    is(eventFront1.hasEventListeners, false, "no event listener expected on node");
+
+    runNextTest();
+  });
+
+  addAsyncTest(function* testChangeEventsOnSeveralNodes() {
+    checkNodesHaveNoEventListener();
+
+    info("add event listeners on both nodes");
+    eventNode1.addEventListener("click", eventListener1);
+    eventNode2.addEventListener("click", eventListener2);
+
+    let mutations = yield waitForMutations();
+    is(mutations.length, 2, "two mutations expected, one for each modified node");
+    // first mutation
+    is(mutations[0].target, eventFront1, "first mutation targets eventFront1");
+    is(mutations[0].type, "events", "mutation type is events");
+    is(mutations[0].hasEventListeners, true, "mutation target should have event listeners");
+    is(eventFront1.hasEventListeners, true, "eventFront1 should have event listeners");
+    // second mutation
+    is(mutations[1].target, eventFront2, "second mutation targets eventFront2");
+    is(mutations[1].type, "events", "mutation type is events");
+    is(mutations[1].hasEventListeners, true, "mutation target should have event listeners");
+    is(eventFront2.hasEventListeners, true, "eventFront1 should have event listeners");
+
+    info("remove event listeners on both nodes");
+    eventNode1.removeEventListener("click", eventListener1);
+    eventNode2.removeEventListener("click", eventListener2);
+
+    mutations = yield waitForMutations();
+    is(mutations.length, 2, "one mutation registered for event listener change");
+    // first mutation
+    is(mutations[0].target, eventFront1, "first mutation targets eventFront1");
+    is(mutations[0].type, "events", "mutation type is events");
+    is(mutations[0].hasEventListeners, false, "mutation target should have no event listeners");
+    is(eventFront1.hasEventListeners, false, "eventFront2 should have no event listeners");
+    // second mutation
+    is(mutations[1].target, eventFront2, "second mutation targets eventFront2");
+    is(mutations[1].type, "events", "mutation type is events");
+    is(mutations[1].hasEventListeners, false, "mutation target should have no event listeners");
+    is(eventFront2.hasEventListeners, false, "eventFront2 should have no event listeners");
+
+    runNextTest();
+  });
+
+  addAsyncTest(function* testRemoveMissingEvent() {
+    checkNodesHaveNoEventListener();
+
+    info("try to remove an event listener not previously added");
+    eventNode1.removeEventListener("click", eventListener1);
+
+    info("set any attribute on the node to trigger a mutation")
+    eventNode1.setAttribute("data-attr", "somevalue");
+
+    let mutations = yield waitForMutations();
+    is(mutations.length, 1, "expect only one mutation");
+    isnot(mutations.type, "events", "mutation type should not be events");
+
+    runNextTest();
+  });
+
+  function checkNodesHaveNoEventListener() {
+    is(eventFront1.hasEventListeners, false, "eventFront1 hasEventListeners should be false");
+    is(eventFront2.hasEventListeners, false, "eventFront2 hasEventListeners should be false");
+  };
+
+  function waitForMutations() {
+    return new Promise(resolve => {
+      walker.once("mutations", mutations => {
+        resolve(mutations);
+      });
+    });
+  }
+
+  runNextTest();
+}
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1157469">Mozilla Bug 1157469</a>
+<a id="inspectorContent" target="_blank" href="inspector-traversal-data.html">Test Document</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>