Bug 1333352 - use client-side source map service in markup view event bubble; r=jdescottes
authorTom Tromey <tom@tromey.com>
Fri, 12 May 2017 14:34:26 -0600
changeset 408053 e82c93c18e8d9b2f5b820a672b55ee467f27a985
parent 408052 5b306da6fbb486d5328cb0af4c0c4dcf1ce91bcd
child 408054 d0a029765f699c91d2631b65bd4357e74a8fc755
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1333352
milestone55.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 1333352 - use client-side source map service in markup view event bubble; r=jdescottes MozReview-Commit-ID: D8bF5kkHp2p
.eslintignore
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_events_source_map.js
devtools/client/inspector/markup/test/doc_markup_events-source_map.html
devtools/client/inspector/markup/test/events_bundle.js
devtools/client/inspector/markup/test/events_bundle.js.map
devtools/client/inspector/markup/test/events_original.js
devtools/client/inspector/markup/test/helper_events_test_runner.js
devtools/client/shared/widgets/tooltip/EventTooltipHelper.js
--- a/.eslintignore
+++ b/.eslintignore
@@ -167,16 +167,17 @@ devtools/server/actors/utils/automation-
 
 # Ignore devtools files testing sourcemaps / code style
 devtools/client/debugger/test/mochitest/code_binary_search.js
 devtools/client/debugger/test/mochitest/code_math.min.js
 devtools/client/debugger/test/mochitest/code_math_bogus_map.js
 devtools/client/debugger/test/mochitest/code_ugly*
 devtools/client/debugger/test/mochitest/code_worker-source-map.js
 devtools/client/framework/test/code_ugly*
+devtools/client/inspector/markup/test/events_bundle.js
 devtools/server/tests/unit/babel_and_browserify_script_with_source_map.js
 devtools/server/tests/unit/setBreakpoint*
 devtools/server/tests/unit/sourcemapped.js
 
 # dom/ exclusions
 dom/animation/**
 dom/archivereader/**
 dom/asmjscache/**
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -14,16 +14,17 @@ support-files =
   doc_markup_events_04.html
   doc_markup_events_form.html
   doc_markup_events_jquery.html
   doc_markup_events-overflow.html
   doc_markup_events_react_development_15.4.1.html
   doc_markup_events_react_development_15.4.1_jsx.html
   doc_markup_events_react_production_15.3.1.html
   doc_markup_events_react_production_15.3.1_jsx.html
+  doc_markup_events-source_map.html
   doc_markup_flashing.html
   doc_markup_html_mixed_case.html
   doc_markup_image_and_canvas.html
   doc_markup_image_and_canvas_2.html
   doc_markup_links.html
   doc_markup_mutation.html
   doc_markup_navigation.html
   doc_markup_not_displayed.html
@@ -32,16 +33,19 @@ support-files =
   doc_markup_search.html
   doc_markup_svg_attributes.html
   doc_markup_toggle.html
   doc_markup_tooltip.png
   doc_markup_void_elements.html
   doc_markup_void_elements.xhtml
   doc_markup_whitespace.html
   doc_markup_xul.xul
+  events_bundle.js
+  events_bundle.js.map
+  events_original.js
   head.js
   helper_attributes_test_runner.js
   helper_diff.js
   helper_events_test_runner.js
   helper_markup_accessibility_navigation.js
   helper_outerhtml_test_runner.js
   helper_style_attr_test_runner.js
   lib_babel_6.21.0_min.js
@@ -109,16 +113,17 @@ skip-if = (os == 'linux' && bits == 32 &
 [browser_markup_events_jquery_1.11.1.js]
 [browser_markup_events_jquery_2.1.1.js]
 [browser_markup_events-overflow.js]
 skip-if = true # Bug 1177550
 [browser_markup_events_react_development_15.4.1.js]
 [browser_markup_events_react_development_15.4.1_jsx.js]
 [browser_markup_events_react_production_15.3.1.js]
 [browser_markup_events_react_production_15.3.1_jsx.js]
+[browser_markup_events_source_map.js]
 [browser_markup_events-windowed-host.js]
 [browser_markup_links_01.js]
 [browser_markup_links_02.js]
 [browser_markup_links_03.js]
 [browser_markup_links_04.js]
 subsuite = clipboard
 skip-if = (os == 'linux' && bits == 32 && debug) # bug 1328915, disable linux32 debug devtools for timeouts
 [browser_markup_links_05.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/browser_markup_events_source_map.js
@@ -0,0 +1,58 @@
+/* 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";
+
+// Check that source maps work in the event popup.
+
+const INITIAL_URL = URL_ROOT + "doc_markup_void_elements.html";
+const TEST_URL = URL_ROOT + "doc_markup_events-source_map.html";
+
+/* import-globals-from helper_events_test_runner.js */
+loadHelperScript("helper_events_test_runner.js");
+
+const TEST_DATA = [
+  {
+    selector: "#clicky",
+    isSourceMapped: true,
+    expected: [
+      {
+        type: "click",
+        filename: "webpack:///events_original.js:7",
+        attributes: [
+          "Bubbling",
+          "DOM2"
+        ],
+        handler: `function clickme() {
+  console.log("clickme");
+}`,
+      },
+    ],
+  },
+];
+
+add_task(function* () {
+  // Load some other URL before opening the toolbox, then navigate to
+  // the test URL.  This ensures that source map service will see the
+  // sources as they are loaded, avoiding any races.
+  let {toolbox, inspector, testActor} = yield openInspectorForURL(INITIAL_URL);
+
+  // Ensure the source map service is operating.  This looks a bit
+  // funny, but sourceMapURLService is a getter, and we don't need the
+  // result.
+  toolbox.sourceMapURLService;
+
+  yield navigateTo(inspector, TEST_URL);
+
+  yield inspector.markup.expandAll();
+
+  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();
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/doc_markup_events-source_map.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <script type="application/javascript" src="events_bundle.js"></script>
+  </head>
+  <body onload="init();">
+    <div id="clicky">click here</div>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/events_bundle.js
@@ -0,0 +1,94 @@
+/******/ (function(modules) { // webpackBootstrap
+/******/ 	// The module cache
+/******/ 	var installedModules = {};
+/******/
+/******/ 	// The require function
+/******/ 	function __webpack_require__(moduleId) {
+/******/
+/******/ 		// Check if module is in cache
+/******/ 		if(installedModules[moduleId]) {
+/******/ 			return installedModules[moduleId].exports;
+/******/ 		}
+/******/ 		// Create a new module (and put it into the cache)
+/******/ 		var module = installedModules[moduleId] = {
+/******/ 			i: moduleId,
+/******/ 			l: false,
+/******/ 			exports: {}
+/******/ 		};
+/******/
+/******/ 		// Execute the module function
+/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
+/******/
+/******/ 		// Flag the module as loaded
+/******/ 		module.l = true;
+/******/
+/******/ 		// Return the exports of the module
+/******/ 		return module.exports;
+/******/ 	}
+/******/
+/******/
+/******/ 	// expose the modules object (__webpack_modules__)
+/******/ 	__webpack_require__.m = modules;
+/******/
+/******/ 	// expose the module cache
+/******/ 	__webpack_require__.c = installedModules;
+/******/
+/******/ 	// identity function for calling harmony imports with the correct context
+/******/ 	__webpack_require__.i = function(value) { return value; };
+/******/
+/******/ 	// define getter function for harmony exports
+/******/ 	__webpack_require__.d = function(exports, name, getter) {
+/******/ 		if(!__webpack_require__.o(exports, name)) {
+/******/ 			Object.defineProperty(exports, name, {
+/******/ 				configurable: false,
+/******/ 				enumerable: true,
+/******/ 				get: getter
+/******/ 			});
+/******/ 		}
+/******/ 	};
+/******/
+/******/ 	// getDefaultExport function for compatibility with non-harmony modules
+/******/ 	__webpack_require__.n = function(module) {
+/******/ 		var getter = module && module.__esModule ?
+/******/ 			function getDefault() { return module['default']; } :
+/******/ 			function getModuleExports() { return module; };
+/******/ 		__webpack_require__.d(getter, 'a', getter);
+/******/ 		return getter;
+/******/ 	};
+/******/
+/******/ 	// Object.prototype.hasOwnProperty.call
+/******/ 	__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };
+/******/
+/******/ 	// __webpack_public_path__
+/******/ 	__webpack_require__.p = "";
+/******/
+/******/ 	// Load entry module and return exports
+/******/ 	return __webpack_require__(__webpack_require__.s = 0);
+/******/ })
+/************************************************************************/
+/******/ ([
+/* 0 */
+/***/ (function(module, exports, __webpack_require__) {
+
+"use strict";
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+
+
+function clickme() {
+  console.log("clickme");
+}
+
+function init() {
+  let s = document.querySelector("#clicky");
+  s.addEventListener("click", clickme);
+}
+
+window.init = init;
+
+
+/***/ })
+/******/ ]);
+//# sourceMappingURL=events_bundle.js.map
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/events_bundle.js.map
@@ -0,0 +1,1 @@
+{"version":3,"sources":["webpack:///webpack/bootstrap 43c031d75b9220c44a01","webpack:///./events_original.js"],"names":[],"mappings":";AAAA;AACA;;AAEA;AACA;;AAEA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;;AAEA;AACA;;AAEA;AACA;;AAEA;AACA;AACA;;;AAGA;AACA;;AAEA;AACA;;AAEA;AACA,mDAA2C,cAAc;;AAEzD;AACA;AACA;AACA;AACA;AACA;AACA;AACA,aAAK;AACL;AACA;;AAEA;AACA;AACA;AACA,mCAA2B,0BAA0B,EAAE;AACvD,yCAAiC,eAAe;AAChD;AACA;AACA;;AAEA;AACA,8DAAsD,+DAA+D;;AAErH;AACA;;AAEA;AACA;;;;;;;;AChEA;AACA;AACA;;AAEA;;AAEA;AACA;AACA;;AAEA;AACA;AACA;AACA;;AAEA","file":"events_bundle.js","sourcesContent":[" \t// The module cache\n \tvar installedModules = {};\n\n \t// The require function\n \tfunction __webpack_require__(moduleId) {\n\n \t\t// Check if module is in cache\n \t\tif(installedModules[moduleId]) {\n \t\t\treturn installedModules[moduleId].exports;\n \t\t}\n \t\t// Create a new module (and put it into the cache)\n \t\tvar module = installedModules[moduleId] = {\n \t\t\ti: moduleId,\n \t\t\tl: false,\n \t\t\texports: {}\n \t\t};\n\n \t\t// Execute the module function\n \t\tmodules[moduleId].call(module.exports, module, module.exports, __webpack_require__);\n\n \t\t// Flag the module as loaded\n \t\tmodule.l = true;\n\n \t\t// Return the exports of the module\n \t\treturn module.exports;\n \t}\n\n\n \t// expose the modules object (__webpack_modules__)\n \t__webpack_require__.m = modules;\n\n \t// expose the module cache\n \t__webpack_require__.c = installedModules;\n\n \t// identity function for calling harmony imports with the correct context\n \t__webpack_require__.i = function(value) { return value; };\n\n \t// define getter function for harmony exports\n \t__webpack_require__.d = function(exports, name, getter) {\n \t\tif(!__webpack_require__.o(exports, name)) {\n \t\t\tObject.defineProperty(exports, name, {\n \t\t\t\tconfigurable: false,\n \t\t\t\tenumerable: true,\n \t\t\t\tget: getter\n \t\t\t});\n \t\t}\n \t};\n\n \t// getDefaultExport function for compatibility with non-harmony modules\n \t__webpack_require__.n = function(module) {\n \t\tvar getter = module && module.__esModule ?\n \t\t\tfunction getDefault() { return module['default']; } :\n \t\t\tfunction getModuleExports() { return module; };\n \t\t__webpack_require__.d(getter, 'a', getter);\n \t\treturn getter;\n \t};\n\n \t// Object.prototype.hasOwnProperty.call\n \t__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };\n\n \t// __webpack_public_path__\n \t__webpack_require__.p = \"\";\n\n \t// Load entry module and return exports\n \treturn __webpack_require__(__webpack_require__.s = 0);\n\n\n\n// WEBPACK FOOTER //\n// webpack/bootstrap 43c031d75b9220c44a01","/* vim: set ts=2 et sw=2 tw=80: */\n/* Any copyright is dedicated to the Public Domain.\n   http://creativecommons.org/publicdomain/zero/1.0/ */\n\n\"use strict\";\n\nfunction clickme() {\n  console.log(\"clickme\");\n}\n\nfunction init() {\n  let s = document.querySelector(\"#clicky\");\n  s.addEventListener(\"click\", clickme);\n}\n\nwindow.init = init;\n\n\n\n//////////////////\n// WEBPACK FOOTER\n// ./events_original.js\n// module id = 0\n// module chunks = 0"],"sourceRoot":""}
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/events_original.js
@@ -0,0 +1,16 @@
+/* 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";
+
+function clickme() {
+  console.log("clickme");
+}
+
+function init() {
+  let s = document.querySelector("#clicky");
+  s.addEventListener("click", clickme);
+}
+
+window.init = init;
--- a/devtools/client/inspector/markup/test/helper_events_test_runner.js
+++ b/devtools/client/inspector/markup/test/helper_events_test_runner.js
@@ -36,22 +36,24 @@ function* runEventPopupTests(url, tests)
  *        - 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
+ *        - isSourceMapped {Boolean} (optional) true if the location
+ *        is source-mapped, requiring some extra delay before the checks
  * @param {InspectorPanel} inspector The instance of InspectorPanel currently
  * opened
  * @param {TestActorFront} testActor
  */
 function* checkEventsForNode(test, inspector, testActor) {
-  let {selector, expected, beforeTest} = test;
+  let {selector, expected, beforeTest, isSourceMapped} = test;
   let container = yield getContainerForSelector(selector, inspector);
 
   if (typeof beforeTest === "function") {
     yield beforeTest(inspector, testActor);
   }
 
   let evHolder = container.elt.querySelector(".markupview-events");
 
@@ -60,23 +62,33 @@ function* checkEventsForNode(test, inspe
     is(evHolder.style.display, "none", "event bubble should be hidden");
     return;
   }
 
   let tooltip = inspector.markup.eventDetailsTooltip;
 
   yield selectNode(selector, inspector);
 
+  let sourceMapPromise = null;
+  if (isSourceMapped) {
+    sourceMapPromise = tooltip.once("event-tooltip-source-map-ready");
+  }
+
   // Click button to show tooltip
   info("Clicking evHolder");
   EventUtils.synthesizeMouseAtCenter(evHolder, {},
     inspector.markup.doc.defaultView);
   yield tooltip.once("shown");
   info("tooltip shown");
 
+  if (isSourceMapped) {
+    info("Waiting for source map to be applied");
+    yield sourceMapPromise;
+  }
+
   // Check values
   let headers = tooltip.panel.querySelectorAll(".event-header");
   let nodeFront = container.node;
   let cssSelector = nodeFront.nodeName + "#" + nodeFront.id;
 
   for (let i = 0; i < headers.length; i++) {
     info("Processing header[" + i + "] for " + cssSelector);
 
--- a/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js
+++ b/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js
@@ -7,18 +7,16 @@
 "use strict";
 
 const {LocalizationHelper} = require("devtools/shared/l10n");
 const L10N = new LocalizationHelper("devtools/client/locales/inspector.properties");
 
 const Editor = require("devtools/client/sourceeditor/editor");
 const beautify = require("devtools/shared/jsbeautify/beautify");
 
-loader.lazyRequireGetter(this, "viewSource", "devtools/client/shared/view-source");
-
 const XHTML_NS = "http://www.w3.org/1999/xhtml";
 const CONTAINER_WIDTH = 500;
 
 /**
  * Set the content of a provided HTMLTooltip instance to display a list of event
  * listeners, with their event type, capturing argument and a link to the code
  * of the event handler.
  *
@@ -59,20 +57,25 @@ EventTooltip.prototype = {
       extraKeys: {},
       theme: "mozilla markup-view"
     };
 
     let doc = this._tooltip.doc;
     this.container = doc.createElementNS(XHTML_NS, "div");
     this.container.className = "devtools-tooltip-events-container";
 
+    const sourceMapService = this._toolbox.sourceMapURLService;
+
     for (let listener of this._eventListenerInfos) {
       let phase = listener.capturing ? "Capturing" : "Bubbling";
       let level = listener.DOM0 ? "DOM0" : "DOM2";
 
+      // Create this early so we can refer to it from a closure, below.
+      let content = doc.createElementNS(XHTML_NS, "div");
+
       // Header
       let header = doc.createElementNS(XHTML_NS, "div");
       header.className = "event-header devtools-toolbar";
       this.container.appendChild(header);
 
       if (!listener.hide.debugger) {
         let debuggerIcon = doc.createElementNS(XHTML_NS, "img");
         debuggerIcon.className = "event-tooltip-debugger-icon";
@@ -98,16 +101,33 @@ EventTooltip.prototype = {
       let filename = doc.createElementNS(XHTML_NS, "span");
       filename.className = "event-tooltip-filename devtools-monospace";
 
       let text = listener.origin;
       let title = text;
       if (listener.hide.filename) {
         text = L10N.getStr("eventsTooltip.unknownLocation");
         title = L10N.getStr("eventsTooltip.unknownLocationExplanation");
+      } else if (sourceMapService) {
+        const location = this._parseLocation(text);
+        if (location) {
+          sourceMapService.originalPositionFor(location.url, location.line)
+            .then((originalLocation) => {
+              if (originalLocation) {
+                const { sourceUrl, line } = originalLocation;
+                let newURI = sourceUrl + ":" + line;
+                filename.textContent = newURI;
+                filename.setAttribute("title", newURI);
+                let eventEditor = this._eventEditors.get(content);
+                eventEditor.uri = newURI;
+              }
+              // This is emitted for testing.
+              this._tooltip.emit("event-tooltip-source-map-ready");
+            });
+        }
       }
 
       filename.textContent = text;
       filename.setAttribute("title", title);
       header.appendChild(filename);
 
       let attributesContainer = doc.createElementNS(XHTML_NS, "div");
       attributesContainer.className = "event-tooltip-attributes-container";
@@ -147,17 +167,16 @@ EventTooltip.prototype = {
         let dom0 = doc.createElementNS(XHTML_NS, "span");
         dom0.className = "event-tooltip-attributes";
         dom0.textContent = level;
         dom0.setAttribute("title", level);
         attributesBox.appendChild(dom0);
       }
 
       // Content
-      let content = doc.createElementNS(XHTML_NS, "div");
       let editor = new Editor(config);
       this._eventEditors.set(content, {
         editor: editor,
         handler: listener.handler,
         uri: listener.origin,
         dom0: listener.DOM0,
         native: listener.native,
         appended: false,
@@ -231,34 +250,45 @@ EventTooltip.prototype = {
   },
 
   _debugClicked: function (event) {
     let header = event.currentTarget;
     let content = header.nextElementSibling;
 
     let {uri} = this._eventEditors.get(content);
 
-    if (uri && uri !== "?") {
+    let location = this._parseLocation(uri);
+    if (location) {
       // Save a copy of toolbox as it will be set to null when we hide the tooltip.
       let toolbox = this._toolbox;
 
       this._tooltip.hide();
 
+      toolbox.viewSourceInDebugger(location.url, location.line);
+    }
+  },
+
+  /**
+   * Parse URI and return {url, line}; or return null if it can't be parsed.
+   */
+  _parseLocation: function (uri) {
+    if (uri && uri !== "?") {
       uri = uri.replace(/"/g, "");
 
       let matches = uri.match(/(.*):(\d+$)/);
-      let line = 1;
 
       if (matches) {
-        uri = matches[1];
-        line = matches[2];
+        return {
+          url: matches[1],
+          line: parseInt(matches[2], 10),
+        };
       }
-
-      viewSource.viewSourceInDebugger(toolbox, uri, line);
+      return {url: uri, line: 1};
     }
+    return null;
   },
 
   destroy: function () {
     if (this._tooltip) {
       this._tooltip.off("hidden", this.destroy);
 
       let boxes = this.container.querySelectorAll(".event-tooltip-content-box");