From ea788b5b502b4ff63d04f7460860c266b884e507 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 12 May 2017 14:34:26 -0600 Subject: [PATCH] Bug 1333352 - use client-side source map service in markup view event bubble; r=jdescottes MozReview-Commit-ID: D8bF5kkHp2p --HG-- extra : rebase_source : 88ff42bbedbd844dee3360c132f40ec8188aa692 --- .eslintignore | 1 + .../client/inspector/markup/test/browser.ini | 5 + .../test/browser_markup_events_source_map.js | 58 ++++++++++++ .../test/doc_markup_events-source_map.html | 10 ++ .../inspector/markup/test/events_bundle.js | 94 +++++++++++++++++++ .../markup/test/events_bundle.js.map | 1 + .../inspector/markup/test/events_original.js | 16 ++++ .../markup/test/helper_events_test_runner.js | 14 ++- .../widgets/tooltip/EventTooltipHelper.js | 48 ++++++++-- 9 files changed, 237 insertions(+), 10 deletions(-) create mode 100644 devtools/client/inspector/markup/test/browser_markup_events_source_map.js create mode 100644 devtools/client/inspector/markup/test/doc_markup_events-source_map.html create mode 100644 devtools/client/inspector/markup/test/events_bundle.js create mode 100644 devtools/client/inspector/markup/test/events_bundle.js.map create mode 100644 devtools/client/inspector/markup/test/events_original.js diff --git a/.eslintignore b/.eslintignore index 1297dcd81b3f..1ec0b73278d8 100644 --- a/.eslintignore +++ b/.eslintignore @@ -172,6 +172,7 @@ 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 diff --git a/devtools/client/inspector/markup/test/browser.ini b/devtools/client/inspector/markup/test/browser.ini index b8bf84413e13..dc4aa3d18003 100644 --- a/devtools/client/inspector/markup/test/browser.ini +++ b/devtools/client/inspector/markup/test/browser.ini @@ -19,6 +19,7 @@ support-files = 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 @@ -37,6 +38,9 @@ support-files = 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 @@ -114,6 +118,7 @@ skip-if = true # Bug 1177550 [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] diff --git a/devtools/client/inspector/markup/test/browser_markup_events_source_map.js b/devtools/client/inspector/markup/test/browser_markup_events_source_map.js new file mode 100644 index 000000000000..5c59f0edccf2 --- /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(); +}); diff --git a/devtools/client/inspector/markup/test/doc_markup_events-source_map.html b/devtools/client/inspector/markup/test/doc_markup_events-source_map.html new file mode 100644 index 000000000000..ece3abd7b837 --- /dev/null +++ b/devtools/client/inspector/markup/test/doc_markup_events-source_map.html @@ -0,0 +1,10 @@ + + + + + + + +
click here
+ + diff --git a/devtools/client/inspector/markup/test/events_bundle.js b/devtools/client/inspector/markup/test/events_bundle.js new file mode 100644 index 000000000000..4759b2722661 --- /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 diff --git a/devtools/client/inspector/markup/test/events_bundle.js.map b/devtools/client/inspector/markup/test/events_bundle.js.map new file mode 100644 index 000000000000..ab8e56cd2f53 --- /dev/null +++ b/devtools/client/inspector/markup/test/events_bundle.js.map @@ -0,0 +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 diff --git a/devtools/client/inspector/markup/test/events_original.js b/devtools/client/inspector/markup/test/events_original.js new file mode 100644 index 000000000000..5a446424b59e --- /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; diff --git a/devtools/client/inspector/markup/test/helper_events_test_runner.js b/devtools/client/inspector/markup/test/helper_events_test_runner.js index d8208730daf3..d6f95cad2667 100644 --- a/devtools/client/inspector/markup/test/helper_events_test_runner.js +++ b/devtools/client/inspector/markup/test/helper_events_test_runner.js @@ -41,12 +41,14 @@ function* runEventPopupTests(url, tests) { * - 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") { @@ -65,6 +67,11 @@ function* checkEventsForNode(test, inspector, testActor) { 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, {}, @@ -72,6 +79,11 @@ function* checkEventsForNode(test, inspector, testActor) { 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; diff --git a/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js b/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js index e9862c98bd9d..253ade6a9e61 100644 --- a/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js +++ b/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js @@ -12,8 +12,6 @@ const L10N = new LocalizationHelper("devtools/client/locales/inspector.propertie 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; @@ -64,10 +62,15 @@ EventTooltip.prototype = { 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"; @@ -103,6 +106,23 @@ EventTooltip.prototype = { 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; @@ -152,7 +172,6 @@ EventTooltip.prototype = { } // Content - let content = doc.createElementNS(XHTML_NS, "div"); let editor = new Editor(config); this._eventEditors.set(content, { editor: editor, @@ -236,24 +255,35 @@ EventTooltip.prototype = { 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 () {