Bug 1557138 Clicking logpoint location in console opens the logpoint editing panel in debugger r=nchevobbe,jlast
authorchujun <chujunlu@hotmail.com>
Wed, 14 Aug 2019 16:10:50 +0000
changeset 488006 6e86ec2e3a21791238aa7f0108c0aabecc6a3e15
parent 488005 405b6f21b6405b3e5bc0b85a2a5de51d619f1a35
child 488007 d14199c9c1cc887fb8e67298e970429848a0d591
push id36434
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:44:30 +0000
treeherdermozilla-central@144fbfb409b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe, jlast
bugs1557138
milestone70.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 1557138 Clicking logpoint location in console opens the logpoint editing panel in debugger r=nchevobbe,jlast I pass `frame.origin` in `viewSourceInDebugger`. When `reason` is logpoint, run `openConditionalPanel`. Differential Revision: https://phabricator.services.mozilla.com/D35624
devtools/client/debugger/panel.js
devtools/client/debugger/src/components/Editor/ConditionalPanel.js
devtools/client/debugger/src/reducers/breakpoints.js
devtools/client/shared/components/Frame.js
devtools/client/shared/components/test/mochitest/test_frame_01.html
devtools/client/shared/view-source.js
devtools/client/webconsole/test/browser/browser.ini
devtools/client/webconsole/test/browser/browser_webconsole_location_logpoint_debugger_link.js
devtools/client/webconsole/test/browser/head.js
devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint-1.js
devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint-2.js
devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint.html
devtools/client/webconsole/utils/messages.js
--- a/devtools/client/debugger/panel.js
+++ b/devtools/client/debugger/panel.js
@@ -155,19 +155,24 @@ DebuggerPanel.prototype = {
     return this._selectors.getIsPaused(this._getState(), thread);
   },
 
   selectSourceURL(url, line, column) {
     const cx = this._selectors.getContext(this._getState());
     return this._actions.selectSourceURL(cx, url, { line, column });
   },
 
-  selectSource(sourceId, line, column) {
+  async selectSource(sourceId, line, column) {
     const cx = this._selectors.getContext(this._getState());
-    return this._actions.selectSource(cx, sourceId, { line, column });
+    const location = { sourceId, line, column };
+
+    await this._actions.selectSource(cx, sourceId, location);
+    if (this._selectors.hasLogpoint(this._getState(), location)) {
+      this._actions.openConditionalPanel(location, true);
+    }
   },
 
   canLoadSource(sourceId) {
     return this._selectors.canLoadSource(this._getState(), sourceId);
   },
 
   getSourceByActorId(sourceId) {
     return this._selectors.getSourceByActorId(this._getState(), sourceId);
--- a/devtools/client/debugger/src/components/Editor/ConditionalPanel.js
+++ b/devtools/client/debugger/src/components/Editor/ConditionalPanel.js
@@ -7,17 +7,17 @@ import React, { PureComponent } from "re
 import ReactDOM from "react-dom";
 import { connect } from "../../utils/connect";
 import classNames from "classnames";
 import "./ConditionalPanel.css";
 import { toEditorLine } from "../../utils/editor";
 import actions from "../../actions";
 
 import {
-  getBreakpointForLocation,
+  getBreakpoint,
   getConditionalPanelLocation,
   getLogPointStatus,
   getContext,
 } from "../../selectors";
 
 import type { SourceLocation, Context } from "../../types";
 
 function addNewLine(doc: Object) {
@@ -226,17 +226,17 @@ export class ConditionalPanel extends Pu
     return null;
   }
 }
 
 const mapStateToProps = state => {
   const location = getConditionalPanelLocation(state);
   return {
     cx: getContext(state),
-    breakpoint: getBreakpointForLocation(state, location),
+    breakpoint: getBreakpoint(state, location),
     location,
     log: getLogPointStatus(state),
   };
 };
 
 const {
   setBreakpointOptions,
   openConditionalPanel,
--- a/devtools/client/debugger/src/reducers/breakpoints.js
+++ b/devtools/client/debugger/src/reducers/breakpoints.js
@@ -173,18 +173,22 @@ export function getBreakpointsList(state
 }
 
 export function getBreakpointCount(state: OuterState): number {
   return getBreakpointsList(state).length;
 }
 
 export function getBreakpoint(
   state: OuterState,
-  location: SourceLocation
+  location: ?SourceLocation
 ): ?Breakpoint {
+  if (!location) {
+    return undefined;
+  }
+
   const breakpoints = getBreakpointsMap(state);
   return breakpoints[makeBreakpointId(location)];
 }
 
 export function getBreakpointsDisabled(state: OuterState): boolean {
   const breakpoints = getBreakpointsList(state);
   return breakpoints.every(breakpoint => breakpoint.disabled);
 }
@@ -203,27 +207,35 @@ export function getBreakpointsForSource(
   return breakpoints.filter(bp => {
     const location = isGeneratedSource ? bp.generatedLocation : bp.location;
     return location.sourceId === sourceId && (!line || line == location.line);
   });
 }
 
 export function getBreakpointForLocation(
   state: OuterState,
-  location: SourceLocation | null
+  location: ?SourceLocation
 ): ?Breakpoint {
-  if (!location || !location.sourceId) {
+  if (!location) {
     return undefined;
   }
 
   const isGeneratedSource = isGeneratedId(location.sourceId);
   return getBreakpointsList(state).find(bp => {
     const loc = isGeneratedSource ? bp.generatedLocation : bp.location;
     return isMatchingLocation(loc, location);
   });
 }
 
 export function getHiddenBreakpoint(state: OuterState): ?Breakpoint {
   const breakpoints = getBreakpointsList(state);
   return breakpoints.find(bp => bp.options.hidden);
 }
 
+export function hasLogpoint(
+  state: OuterState,
+  location: ?SourceLocation
+): ?string {
+  const breakpoint = getBreakpoint(state, location);
+  return breakpoint && breakpoint.options.logValue;
+}
+
 export default update;
--- a/devtools/client/shared/components/Frame.js
+++ b/devtools/client/shared/components/Frame.js
@@ -199,21 +199,21 @@ class Frame extends Component {
             },
             functionDisplayName
           ),
           " "
         );
       }
     }
 
-    // If the message comes from a logPoint or conditional breakpoint,
+    // If the message comes from a logPoint,
     // prefix the source location accordingly
-    if (frame.origin) {
+    if (frame.options) {
       let locationPrefix;
-      if (frame.origin === "logPoint") {
+      if (frame.options.logPoint) {
         locationPrefix = "Logpoint @ ";
       }
 
       if (locationPrefix) {
         sourceElements.push(
           dom.span(
             {
               key: "locationPrefix",
--- a/devtools/client/shared/components/test/mochitest/test_frame_01.html
+++ b/devtools/client/shared/components/test/mochitest/test_frame_01.html
@@ -328,20 +328,20 @@ window.onload = async function () {
       tooltip: "View source in Debugger → https://bugzilla.mozilla.org/original.js:23",
       source: "https://bugzilla.mozilla.org/original.js",
     });
 
     // Check when a message comes from a logPoint,
     // a prefix should render before source
     await checkFrameComponent({
       frame: {
-        origin: "logPoint",
         source: "http://myfile.com/mahscripts.js",
         line: 55,
         column: 10,
+        options: { logPoint: true },
       }
     }, {
       locationPrefix: "Logpoint @ ",
       file: "mahscripts.js",
       line: 55,
       column: 10,
       shouldLink: true,
       tooltip: "View source in Debugger → http://myfile.com/mahscripts.js:55:10",
--- a/devtools/client/shared/view-source.js
+++ b/devtools/client/shared/view-source.js
@@ -47,17 +47,17 @@ exports.viewSourceInStyleEditor = async 
  * the source was able to be displayed in the Debugger, as the built-in Firefox
  * View Source is the fallback.
  *
  * @param {Toolbox} toolbox
  * @param {string} sourceURL
  * @param {number} sourceLine
  * @param {number} sourceColumn
  * @param {string} sourceID
- * @param {string} [reason=unknown]
+ * @param {(string|object)} [reason=unknown]
  *
  * @return {Promise<boolean>}
  */
 exports.viewSourceInDebugger = async function(
   toolbox,
   sourceURL,
   sourceLine,
   sourceColumn,
--- a/devtools/client/webconsole/test/browser/browser.ini
+++ b/devtools/client/webconsole/test/browser/browser.ini
@@ -86,16 +86,19 @@ support-files =
   test-insecure-passwords-web-console-warning.html
   test-inspect-cross-domain-objects-frame.html
   test-inspect-cross-domain-objects-top.html
   test_jsterm_screenshot_command.html
   test-local-session-storage.html
   test-location-debugger-link-console-log.js
   test-location-debugger-link-errors.js
   test-location-debugger-link.html
+  test-location-debugger-link-logpoint-1.js
+  test-location-debugger-link-logpoint-2.js
+  test-location-debugger-link-logpoint.html
   test-location-styleeditor-link-1.css
   test-location-styleeditor-link-2.css
   test-location-styleeditor-link-minified.css
   test-location-styleeditor-link.html
   test-message-categories-canvas-css.html
   test-message-categories-canvas-css.js
   test-message-categories-css-loader.css
   test-message-categories-css-loader.css^headers^
@@ -354,16 +357,17 @@ skip-if = fission
 [browser_webconsole_input_field_focus_on_panel_select.js]
 [browser_webconsole_input_focus.js]
 [browser_webconsole_insecure_passwords_about_blank_web_console_warning.js]
 [browser_webconsole_insecure_passwords_web_console_warning.js]
 [browser_webconsole_inspect_cross_domain_object.js]
 skip-if = fission
 [browser_webconsole_keyboard_accessibility.js]
 [browser_webconsole_location_debugger_link.js]
+[browser_webconsole_location_logpoint_debugger_link.js]
 [browser_webconsole_location_scratchpad_link.js]
 [browser_webconsole_location_styleeditor_link.js]
 [browser_webconsole_logErrorInPage.js]
 [browser_webconsole_loglimit.js]
 [browser_webconsole_logWarningInPage.js]
 [browser_webconsole_longstring_getter.js]
 [browser_webconsole_longstring.js]
 [browser_webconsole_message_categories.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/browser/browser_webconsole_location_logpoint_debugger_link.js
@@ -0,0 +1,187 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test clicking locations of logpoint logs and errors will open corresponding
+// conditional panels in the debugger.
+
+"use strict";
+
+const TEST_URI =
+  "http://example.com/browser/devtools/client/webconsole/" +
+  "test/browser/test-location-debugger-link-logpoint.html";
+
+add_task(async function() {
+  // On e10s, the exception thrown in test-location-debugger-link-errors.js
+  // is triggered in child process and is ignored by test harness
+  if (!Services.appinfo.browserTabsRemoteAutostart) {
+    expectUncaughtException();
+  }
+
+  // Eliminate interference from "saved" breakpoints
+  // when running the test multiple times
+  await clearDebuggerPreferences();
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  info("Open the Debugger panel");
+  await openDebugger();
+
+  const toolbox = gDevTools.getToolbox(hud.target);
+  const dbg = createDebuggerContext(toolbox);
+  await selectSource(dbg, "test-location-debugger-link-logpoint-1.js");
+
+  info("Add a logpoint with an invalid expression");
+  await setLogPoint(dbg, 9, "undefinedVariable");
+
+  info("Add a logpoint with a valid expression");
+  await setLogPoint(dbg, 10, "`a is ${a}`");
+
+  await assertEditorLogpoint(dbg, 9, { hasLog: true });
+  await assertEditorLogpoint(dbg, 10, { hasLog: true });
+
+  info("Close the file in the debugger");
+  await closeTab(dbg, "test-location-debugger-link-logpoint-1.js");
+
+  info("Selecting the console");
+  await toolbox.selectTool("webconsole");
+
+  info("Call the function");
+  await invokeInTab("add");
+
+  info("Wait for two messages");
+  await waitFor(() => findMessages(hud, "").length === 2);
+
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "[Logpoint threw]: undefinedVariable is not defined",
+    true,
+    false,
+    false,
+    "undefinedVariable"
+  );
+
+  info("Selecting the console again");
+  await toolbox.selectTool("webconsole");
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "a is 1",
+    true,
+    false,
+    false,
+    "`a is ${a}`"
+  );
+
+  // Test clicking location of a removed logpoint, or a newly added breakpoint
+  // at an old logpoint's location will only highlight its line
+  info("Remove the logpoints");
+  const source = await findSource(
+    dbg,
+    "test-location-debugger-link-logpoint-1.js"
+  );
+  await removeBreakpoint(dbg, source.id, 9);
+  await removeBreakpoint(dbg, source.id, 10);
+  await addBreakpoint(dbg, "test-location-debugger-link-logpoint-1.js", 10);
+
+  info("Selecting the console");
+  await toolbox.selectTool("webconsole");
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "[Logpoint threw]: undefinedVariable is not defined",
+    true,
+    9,
+    12
+  );
+
+  info("Selecting the console again");
+  await toolbox.selectTool("webconsole");
+  await testOpenInDebugger(hud, toolbox, "a is 1", true, 10, 12);
+});
+
+// Test clicking locations of logpoints from different files
+add_task(async function() {
+  if (!Services.appinfo.browserTabsRemoteAutostart) {
+    expectUncaughtException();
+  }
+
+  await clearDebuggerPreferences();
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  info("Open the Debugger panel");
+  await openDebugger();
+
+  const toolbox = gDevTools.getToolbox(hud.target);
+  const dbg = createDebuggerContext(toolbox);
+
+  info("Add a logpoint to the first file");
+  await selectSource(dbg, "test-location-debugger-link-logpoint-1.js");
+  await setLogPoint(dbg, 10, "`a is ${a}`");
+
+  info("Add a logpoint to the second file");
+  await selectSource(dbg, "test-location-debugger-link-logpoint-2.js");
+  await setLogPoint(dbg, 10, "`c is ${c}`");
+
+  info("Selecting the console");
+  await toolbox.selectTool("webconsole");
+
+  info("Call the function from the first file");
+  await invokeInTab("add");
+
+  info("Wait for the first message");
+  await waitFor(() => findMessages(hud, "").length === 1);
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "a is 1",
+    true,
+    false,
+    false,
+    "`a is ${a}`"
+  );
+
+  info("Selecting the console again");
+  await toolbox.selectTool("webconsole");
+
+  info("Call the function from the second file");
+  await invokeInTab("subtract");
+
+  info("Wait for the second message");
+  await waitFor(() => findMessages(hud, "").length === 2);
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "c is 1",
+    true,
+    false,
+    false,
+    "`c is ${c}`"
+  );
+});
+
+async function setLogPoint(dbg, index, expression) {
+  rightClickElement(dbg, "gutter", index);
+  selectContextMenuItem(
+    dbg,
+    `${selectors.addLogItem},${selectors.editLogItem}`
+  );
+  const onBreakpointSet = waitForDispatch(dbg, "SET_BREAKPOINT");
+  await typeInPanel(dbg, expression);
+  await onBreakpointSet;
+}
+
+function getLineEl(dbg, line) {
+  const lines = dbg.win.document.querySelectorAll(".CodeMirror-code > div");
+  return lines[line - 1];
+}
+
+function assertEditorLogpoint(dbg, line, { hasLog = false } = {}) {
+  const hasLogClass = getLineEl(dbg, line).classList.contains("has-log");
+
+  ok(
+    hasLogClass === hasLog,
+    `Breakpoint log ${hasLog ? "exists" : "does not exist"} on line ${line}`
+  );
+}
--- a/devtools/client/webconsole/test/browser/head.js
+++ b/devtools/client/webconsole/test/browser/head.js
@@ -376,69 +376,75 @@ function waitForNodeMutation(node, obser
       resolve(mutations);
       observer.disconnect();
     });
     observer.observe(node, observeConfig);
   });
 }
 
 /**
- * Search for a given message.  When found, simulate a click on the
+ * Search for a given message. When found, simulate a click on the
  * message's location, checking to make sure that the debugger opens
- * the corresponding URL.
+ * the corresponding URL. If the message was generated by a logpoint,
+ * check if the corresponding logpoint editing panel is opened.
  *
  * @param {Object} hud
  *        The webconsole
  * @param {Object} toolbox
  *        The toolbox
  * @param {String} text
- *        The text to search for.  This should be contained in the
- *        message.  The searching is done with @see findMessage.
+ *        The text to search for. This should be contained in the
+ *        message. The searching is done with @see findMessage.
  * @param {boolean} expectUrl
  *        Whether the URL in the opened source should match the link, or whether
  *        it is expected to be null.
  * @param {boolean} expectLine
  *        It indicates if there is the need to check the line.
  * @param {boolean} expectColumn
  *        It indicates if there is the need to check the column.
+ * @param {String} logPointExpr
+ *        The logpoint expression
  */
 async function testOpenInDebugger(
   hud,
   toolbox,
   text,
   expectUrl = true,
   expectLine = true,
-  expectColumn = true
+  expectColumn = true,
+  logPointExpr = undefined
 ) {
   info(`Finding message for open-in-debugger test; text is "${text}"`);
   const messageNode = await waitFor(() => findMessage(hud, text));
   const frameLinkNode = messageNode.querySelector(
     ".message-location .frame-link"
   );
   ok(frameLinkNode, "The message does have a location link");
   await checkClickOnNode(
     hud,
     toolbox,
     frameLinkNode,
     expectUrl,
     expectLine,
-    expectColumn
+    expectColumn,
+    logPointExpr
   );
 }
 
 /**
  * Helper function for testOpenInDebugger.
  */
 async function checkClickOnNode(
   hud,
   toolbox,
   frameLinkNode,
   expectUrl,
   expectLine,
-  expectColumn
+  expectColumn,
+  logPointExpr
 ) {
   info("checking click on node location");
 
   const onSourceInDebuggerOpened = once(hud.ui, "source-in-debugger-opened");
 
   EventUtils.sendMouseEvent(
     { type: "click" },
     frameLinkNode.querySelector(".frame-link-filename")
@@ -480,16 +486,32 @@ async function checkClickOnNode(
     ok(column, `source column found ("${column}")`);
 
     is(
       dbg._selectors.getSelectedLocation(dbg._getState()).column,
       column,
       "expected source column"
     );
   }
+
+  if (logPointExpr !== undefined && logPointExpr !== "") {
+    const inputEl = dbg.panelWin.document.activeElement;
+    is(
+      inputEl.tagName,
+      "TEXTAREA",
+      "The textarea of logpoint panel is focused"
+    );
+
+    const inputValue = inputEl.parentElement.parentElement.innerText.trim();
+    is(
+      inputValue,
+      logPointExpr,
+      "The input in the open logpoint panel matches the logpoint expression"
+    );
+  }
 }
 
 /**
  * Returns true if the give node is currently focused.
  */
 function hasFocus(node) {
   return (
     node.ownerDocument.activeElement == node && node.ownerDocument.hasFocus()
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint-1.js
@@ -0,0 +1,15 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript 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 add() {
+  const a = 1;
+  const b = 2;
+
+  return a + b;
+}
+
+add();
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint-2.js
@@ -0,0 +1,15 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript 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 subtract() {
+  const c = 1;
+  const d = 2;
+
+  return c - d;
+}
+
+subtract();
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8" />
+    <title>
+      Web Console test for opening logpoint message links in Debugger
+    </title>
+    <!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+    <script
+      type="text/javascript"
+      src="test-location-debugger-link-logpoint-1.js"
+    ></script>
+    <script
+      type="text/javascript"
+      src="test-location-debugger-link-logpoint-2.js"
+    ></script>
+  </head>
+  <body>
+    <p>Web Console test for opening logpoint message links in Debugger.</p>
+    <button onclick="add()">Add</button>
+    <button onclick="subtract()">Subtract</button>
+  </body>
+</html>
--- a/devtools/client/webconsole/utils/messages.js
+++ b/devtools/client/webconsole/utils/messages.js
@@ -242,17 +242,17 @@ function transformConsoleAPICallPacket(p
         source: message.filename,
         sourceId: message.sourceId,
         line: message.lineNumber,
         column: message.columnNumber,
       }
     : null;
 
   if (type === "logPointError" || type === "logPoint") {
-    frame.origin = "logPoint";
+    frame.options = { logPoint: true };
   }
 
   return new ConsoleMessage({
     source: MESSAGE_SOURCE.CONSOLE_API,
     type,
     level,
     parameters,
     messageText,