Bug 1552609 - Adding a logpoint fails and breaks locations. r=davidwalsh a=jcristau
authorJason Laster <jlaster@mozilla.com>
Tue, 21 May 2019 20:46:04 +0000
changeset 536450 a5352b191e5a39803a6ed74a48279529277aa6e6
parent 536449 baf9be06329bcfa0777295b41b55b23debafb195
child 536451 daa553c05384776bc8c79f5ac4e5f3570d91a320
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidwalsh, jcristau
bugs1552609
milestone68.0
Bug 1552609 - Adding a logpoint fails and breaks locations. r=davidwalsh a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D31870
devtools/client/debugger/src/actions/breakpoints/index.js
devtools/client/debugger/src/components/Editor/Breakpoint.js
devtools/client/debugger/src/components/Editor/ColumnBreakpoint.js
devtools/client/debugger/src/components/Editor/ConditionalPanel.js
devtools/client/debugger/src/components/Editor/menus/breakpoints.js
devtools/client/debugger/test/mochitest/.eslintrc
devtools/client/debugger/test/mochitest/browser.ini
devtools/client/debugger/test/mochitest/browser_dbg-breakpoints-cond-source-maps.js
devtools/client/debugger/test/mochitest/browser_dbg-breakpoints-cond.js
devtools/client/debugger/test/mochitest/helpers.js
--- a/devtools/client/debugger/src/actions/breakpoints/index.js
+++ b/devtools/client/debugger/src/actions/breakpoints/index.js
@@ -10,29 +10,27 @@
  */
 
 import { PROMISE } from "../utils/middleware/promise";
 import {
   getBreakpointsList,
   getXHRBreakpoints,
   getSelectedSource,
   getBreakpointAtLocation,
-  getConditionalPanelLocation,
   getBreakpointsForSource,
   getBreakpointsAtLine,
 } from "../../selectors";
 import { createXHRBreakpoint } from "../../utils/breakpoint";
 import {
   addBreakpoint,
   removeBreakpoint,
   enableBreakpoint,
   disableBreakpoint,
 } from "./modify";
 import remapLocations from "./remapLocations";
-import { closeConditionalPanel } from "../ui";
 
 // this will need to be changed so that addCLientBreakpoint is removed
 
 import type { ThunkArgs } from "../types";
 import type {
   Breakpoint,
   Source,
   SourceLocation,
@@ -114,20 +112,21 @@ export function toggleAllBreakpoints(
  * @static
  */
 export function toggleBreakpoints(
   cx: Context,
   shouldDisableBreakpoints: boolean,
   breakpoints: Breakpoint[]
 ) {
   return async ({ dispatch }: ThunkArgs) => {
-    const promises = breakpoints.map(breakpoint =>
-      shouldDisableBreakpoints
-        ? dispatch(disableBreakpoint(cx, breakpoint))
-        : dispatch(enableBreakpoint(cx, breakpoint))
+    const promises = breakpoints.map(
+      breakpoint =>
+        shouldDisableBreakpoints
+          ? dispatch(disableBreakpoint(cx, breakpoint))
+          : dispatch(enableBreakpoint(cx, breakpoint))
     );
 
     await Promise.all(promises);
   };
 }
 
 export function toggleBreakpointsAtLine(
   cx: Context,
@@ -213,20 +212,16 @@ export function toggleBreakpointAtLine(c
   return ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
     const state = getState();
     const selectedSource = getSelectedSource(state);
 
     if (!selectedSource) {
       return;
     }
 
-    if (getConditionalPanelLocation(getState())) {
-      dispatch(closeConditionalPanel());
-    }
-
     const bp = getBreakpointAtLocation(state, { line, column: undefined });
     if (bp) {
       return dispatch(removeBreakpoint(cx, bp));
     }
     return dispatch(
       addBreakpoint(cx, {
         sourceId: selectedSource.id,
         sourceUrl: selectedSource.url,
--- a/devtools/client/debugger/src/components/Editor/Breakpoint.js
+++ b/devtools/client/debugger/src/components/Editor/Breakpoint.js
@@ -101,20 +101,25 @@ class Breakpoint extends PureComponent<P
     return breakpointActions.removeBreakpointsAtLine(
       cx,
       selectedLocation.sourceId,
       selectedLocation.line
     );
   };
 
   onContextMenu = (event: MouseEvent) => {
-    const { cx, breakpoint, breakpointActions } = this.props;
+    const { cx, breakpoint, selectedSource, breakpointActions } = this.props;
     event.stopPropagation();
     event.preventDefault();
-    showMenu(event, breakpointItems(cx, breakpoint, breakpointActions));
+    const selectedLocation = getSelectedLocation(breakpoint, selectedSource);
+
+    showMenu(
+      event,
+      breakpointItems(cx, breakpoint, selectedLocation, breakpointActions)
+    );
   };
 
   addBreakpoint(props: Props) {
     const { breakpoint, editor, selectedSource } = props;
     const selectedLocation = getSelectedLocation(breakpoint, selectedSource);
 
     // Hidden Breakpoints are never rendered on the client
     if (breakpoint.options.hidden) {
--- a/devtools/client/debugger/src/components/Editor/ColumnBreakpoint.js
+++ b/devtools/client/debugger/src/components/Editor/ColumnBreakpoint.js
@@ -4,16 +4,17 @@
 
 // @flow
 import { PureComponent } from "react";
 import classnames from "classnames";
 import { showMenu } from "devtools-contextmenu";
 
 import { getDocument } from "../../utils/editor";
 import { breakpointItems, createBreakpointItems } from "./menus/breakpoints";
+import { getSelectedLocation } from "../../utils/selected-location";
 
 // eslint-disable-next-line max-len
 import type { ColumnBreakpoint as ColumnBreakpointType } from "../../selectors/visibleColumnBreakpoints";
 import type { BreakpointItemActions } from "./menus/breakpoints";
 import type { Source, Context } from "../../types";
 
 type Bookmark = {
   clear: Function,
@@ -96,22 +97,32 @@ export default class ColumnBreakpoint ex
   };
 
   onContextMenu = (event: MouseEvent) => {
     event.stopPropagation();
     event.preventDefault();
     const {
       cx,
       columnBreakpoint: { breakpoint, location },
+      source,
       breakpointActions,
     } = this.props;
 
-    const items = breakpoint
-      ? breakpointItems(cx, breakpoint, breakpointActions)
-      : createBreakpointItems(cx, location, breakpointActions);
+    let items = createBreakpointItems(cx, location, breakpointActions);
+
+    if (breakpoint) {
+      const selectedLocation = getSelectedLocation(breakpoint, source);
+
+      items = breakpointItems(
+        cx,
+        breakpoint,
+        selectedLocation,
+        breakpointActions
+      );
+    }
 
     showMenu(event, items);
   };
 
   componentDidMount() {
     this.addColumnBreakpoint();
   }
 
--- a/devtools/client/debugger/src/components/Editor/ConditionalPanel.js
+++ b/devtools/client/debugger/src/components/Editor/ConditionalPanel.js
@@ -132,16 +132,17 @@ export class ConditionalPanel extends Pu
     this.cbPanel = editor.codeMirror.addLineWidget(
       editorLine,
       this.renderConditionalPanel(props),
       {
         coverGutter: true,
         noHScroll: true,
       }
     );
+
     if (this.input) {
       let parent: ?Node = this.input.parentNode;
       while (parent) {
         if (
           parent instanceof HTMLElement &&
           parent.classList.contains("CodeMirror-scroll")
         ) {
           this.scrollParent = parent;
@@ -153,17 +154,17 @@ export class ConditionalPanel extends Pu
       if (this.scrollParent) {
         this.scrollParent.addEventListener("scroll", this.repositionOnScroll);
         this.repositionOnScroll();
       }
     }
   }
 
   createEditor = (input: ?HTMLTextAreaElement) => {
-    const { log, editor } = this.props;
+    const { log, editor, closeConditionalPanel } = this.props;
 
     const codeMirror = editor.CodeMirror.fromTextArea(input, {
       mode: "javascript",
       theme: "mozilla",
       placeholder: L10N.getStr(
         log
           ? "editor.conditionalPanel.logPoint.placeholder2"
           : "editor.conditionalPanel.placeholder2"
@@ -171,16 +172,18 @@ export class ConditionalPanel extends Pu
     });
 
     codeMirror.on("keydown", (cm, e) => {
       if (e.key === "Enter") {
         e.codemirrorIgnore = true;
       }
     });
 
+    codeMirror.on("blur", (cm, e) => closeConditionalPanel());
+
     const codeMirrorWrapper = codeMirror.getWrapperElement();
 
     codeMirrorWrapper.addEventListener("keydown", e => {
       codeMirror.save();
       this.onKey(e);
     });
 
     this.input = input;
@@ -201,17 +204,16 @@ export class ConditionalPanel extends Pu
 
     const panel = document.createElement("div");
     ReactDOM.render(
       <div
         className={classNames("conditional-breakpoint-panel", {
           "log-point": log,
         })}
         onClick={() => this.keepFocusOnInput()}
-        onBlur={this.props.closeConditionalPanel}
         ref={node => (this.panelNode = node)}
       >
         <div className="prompt">ยป</div>
         <textarea
           defaultValue={defaultValue}
           ref={input => this.createEditor(input)}
         />
       </div>,
--- a/devtools/client/debugger/src/components/Editor/menus/breakpoints.js
+++ b/devtools/client/debugger/src/components/Editor/menus/breakpoints.js
@@ -56,21 +56,21 @@ export const editConditionalBreakpointIt
   accelerator: L10N.getStr("toggleCondPanel.breakpoint.key"),
   accesskey: L10N.getStr("editor.addConditionBreakpoint.accesskey"),
   disabled: false,
   click: () => breakpointActions.openConditionalPanel(location),
 });
 
 export const conditionalBreakpointItem = (
   breakpoint: Breakpoint,
+  location: SourceLocation,
   breakpointActions: BreakpointItemActions
 ) => {
   const {
     options: { condition },
-    location,
   } = breakpoint;
   return condition
     ? editConditionalBreakpointItem(location, breakpointActions)
     : addConditionalBreakpointItem(location, breakpointActions);
 };
 
 export const addLogPointItem = (
   location: SourceLocation,
@@ -93,21 +93,21 @@ export const editLogPointItem = (
   accesskey: L10N.getStr("editor.editLogPoint.accesskey"),
   disabled: false,
   click: () => breakpointActions.openConditionalPanel(location, true),
   accelerator: L10N.getStr("toggleCondPanel.logPoint.key"),
 });
 
 export const logPointItem = (
   breakpoint: Breakpoint,
+  location: SourceLocation,
   breakpointActions: BreakpointItemActions
 ) => {
   const {
     options: { logValue },
-    location,
   } = breakpoint;
   return logValue
     ? editLogPointItem(location, breakpointActions)
     : addLogPointItem(location, breakpointActions);
 };
 
 export const toggleDisabledBreakpointItem = (
   cx: Context,
@@ -128,47 +128,37 @@ export const toggleDisabledBreakpointIte
           label: L10N.getStr("editor.disableBreakpoint"),
         }),
   };
 };
 
 export function breakpointItems(
   cx: Context,
   breakpoint: Breakpoint,
+  selectedLocation: SourceLocation,
   breakpointActions: BreakpointItemActions
 ) {
   const items = [
     removeBreakpointItem(cx, breakpoint, breakpointActions),
     toggleDisabledBreakpointItem(cx, breakpoint, breakpointActions),
   ];
 
-  if (features.columnBreakpoints) {
-    items.push(
-      { type: "separator" },
-      removeBreakpointsOnLineItem(cx, breakpoint.location, breakpointActions),
-      breakpoint.disabled
-        ? enableBreakpointsOnLineItem(
-            cx,
-            breakpoint.location,
-            breakpointActions
-          )
-        : disableBreakpointsOnLineItem(
-            cx,
-            breakpoint.location,
-            breakpointActions
-          ),
-      { type: "separator" }
-    );
-  }
+  items.push(
+    { type: "separator" },
+    removeBreakpointsOnLineItem(cx, selectedLocation, breakpointActions),
+    breakpoint.disabled
+      ? enableBreakpointsOnLineItem(cx, selectedLocation, breakpointActions)
+      : disableBreakpointsOnLineItem(cx, selectedLocation, breakpointActions),
+    { type: "separator" }
+  );
 
-  items.push(conditionalBreakpointItem(breakpoint, breakpointActions));
-
-  if (features.logPoints) {
-    items.push(logPointItem(breakpoint, breakpointActions));
-  }
+  items.push(
+    conditionalBreakpointItem(breakpoint, selectedLocation, breakpointActions)
+  );
+  items.push(logPointItem(breakpoint, selectedLocation, breakpointActions));
 
   return items;
 }
 
 export function createBreakpointItems(
   cx: Context,
   location: SourceLocation,
   breakpointActions: BreakpointItemActions
--- a/devtools/client/debugger/test/mochitest/.eslintrc
+++ b/devtools/client/debugger/test/mochitest/.eslintrc
@@ -37,16 +37,17 @@
     "selectSource": false,
     "prettyPrint": false,
     "closeTab": false,
     "pushPref": false,
     "waitFor": false,
 
     // Globals introduced in debugger-specific head.js
     "getContext": false,
+    "getEditorLineGutter": false,
     "getCM": false,
     "getDebuggerSplitConsole": false,
     "hasConsoleMessage": false,
     "findConsoleMessage": false,
     "promise": false,
     "BrowserToolboxProcess": false,
     "OS": false,
     "selectors": false,
@@ -106,16 +107,17 @@
     "toggleCallStack": false,
     "toggleScopes": false,
     "isVisibleWithin": false,
     "isPaused": false,
     "hoverAtPos": false,
     "clickElement": false,
     "clickElementWithSelector": false,
     "clickDOMElement": false,
+    "dblClickElement": false,
     "altClickElement": false,
     "rightClickElement": false,
     "rightClickEl": false,
     "clickGutter": false,
     "typeInPanel": false,
     "selectMenuItem": false,
     "selectContextMenuItem": false,
     "togglePauseOnExceptions": false,
--- a/devtools/client/debugger/test/mochitest/browser.ini
+++ b/devtools/client/debugger/test/mochitest/browser.ini
@@ -684,16 +684,17 @@ skip-if = true # original stepping is cu
 [browser_dbg-sourcemapped-preview.js]
 skip-if = os == "win" # Bug 1448523, Bug 1448450
 [browser_dbg-breaking.js]
 [browser_dbg-breaking-from-console.js]
 [browser_dbg-breakpoints.js]
 [browser_dbg-breakpoints-actions.js]
 [browser_dbg-breakpoints-columns.js]
 [browser_dbg-breakpoints-cond.js]
+[browser_dbg-breakpoints-cond-source-maps.js]
 [browser_dbg-breakpoints-duplicate-functions.js]
 [browser_dbg-browser-content-toolbox.js]
 skip-if = !e10s || verify # This test is only valid in e10s
 [browser_dbg-breakpoints-reloading.js]
 [browser_dbg-breakpoint-skipping.js]
 [browser_dbg-call-stack.js]
 [browser_dbg-scopes.js]
 [browser_dbg-chrome-create.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-breakpoints-cond-source-maps.js
@@ -0,0 +1,41 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/>. */
+
+async function setLogPoint(dbg, index) {
+  const gutterEl = await getEditorLineGutter(dbg, index);
+  rightClickEl(dbg, gutterEl);
+  selectContextMenuItem(
+    dbg,
+    `${selectors.addLogItem},${selectors.editLogItem}`
+  );
+}
+
+async function waitForConditionalPanelFocus(dbg) {
+  await waitFor(() => dbg.win.document.activeElement.tagName === "TEXTAREA");
+}
+
+function getConditionalPanel(dbg, line) {
+  return getCM(dbg).doc.getLineHandle(line - 1).widgets[0];
+}
+
+// Confirms that a conditional panel is opened at the
+// correct location in generated files.
+add_task(async function() {
+  const dbg = await initDebugger("doc-sourcemaps.html", "entry.js");
+
+  await selectSource(dbg, "bundle.js");
+  getCM(dbg).scrollIntoView({ line: 55, ch: 0 });
+
+  setLogPoint(dbg, 55);
+  await waitForConditionalPanelFocus(dbg);
+  ok(
+    !!getConditionalPanel(dbg, 55),
+    "conditional panel panel is open on line 55"
+  );
+  is(
+    dbg.selectors.getConditionalPanelLocation().line,
+    55,
+    "conditional panel location is line 55"
+  );
+});
--- a/devtools/client/debugger/test/mochitest/browser_dbg-breakpoints-cond.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-breakpoints-cond.js
@@ -58,17 +58,16 @@ async function setLogPoint(dbg, index, v
 
 async function waitForConditionalPanelFocus(dbg) {
   await waitFor(() => dbg.win.document.activeElement.tagName === "TEXTAREA");
 }
 
 add_task(async function() {
   const dbg = await initDebugger("doc-scripts.html", "simple2");
   await pushPref("devtools.debugger.features.column-breakpoints", true);
-  await pushPref("devtools.debugger.features.log-points", true);
 
   await selectSource(dbg, "simple2");
   await waitForSelectedSource(dbg, "simple2");
 
   info("Set condition `1`");
   await setConditionalBreakpoint(dbg, 5, "1");
   await waitForCondition(dbg, 1);
 
@@ -125,20 +124,20 @@ add_task(async function() {
 
   info("Double click the conditional breakpoint in secondary pane");
   dblClickElement(dbg, "conditionalBreakpointInSecPane");
   is(
     dbg.win.document.activeElement.tagName,
     "TEXTAREA",
     "The textarea of conditional breakpoint panel is focused"
   );
-  
+
   info("Click the conditional breakpoint in secondary pane");
   await clickElement(dbg, "conditionalBreakpointInSecPane");
-  let conditonalPanel = findElement(dbg, "conditionalPanel");
+  const conditonalPanel = findElement(dbg, "conditionalPanel");
   is(conditonalPanel, null, "The conditional breakpoint panel is closed");
 
   rightClickElement(dbg, "breakpointItem", 2);
   info('select "remove condition"');
   selectContextMenuItem(dbg, selectors.breakpointContextMenu.removeCondition);
   await waitForBreakpointWithoutCondition(dbg, "simple2", 5);
   bp = findBreakpoint(dbg, "simple2", 5);
   is(bp.options.condition, undefined, "breakpoint condition removed");
@@ -157,14 +156,14 @@ add_task(async function() {
 
   info("Double click the logpoint in secondary pane");
   dblClickElement(dbg, "logPointInSecPane");
   is(
     dbg.win.document.activeElement.tagName,
     "TEXTAREA",
     "The textarea of logpoint panel is focused"
   );
-  
+
   info("Click the logpoint in secondary pane");
   await clickElement(dbg, "logPointInSecPane");
-  let logPointPanel = findElement(dbg, "logPointPanel");
+  const logPointPanel = findElement(dbg, "logPointPanel");
   is(logPointPanel, null, "The logpoint panel is closed");
 });
--- a/devtools/client/debugger/test/mochitest/helpers.js
+++ b/devtools/client/debugger/test/mochitest/helpers.js
@@ -1149,16 +1149,21 @@ function isVisible(outerEl, innerEl) {
     innerRect.left >= outerRect.left ||
     innerRect.right <= outerRect.right ||
     (innerRect.left < outerRect.left && innerRect.right > outerRect.right);
 
   const visible = verticallyVisible && horizontallyVisible;
   return visible;
 }
 
+async function getEditorLineGutter(dbg, line) {
+  const lineEl = await getEditorLineEl(dbg, line);
+  return lineEl.firstChild;
+}
+
 async function getEditorLineEl(dbg, line) {
   let el = await codeMirrorGutterElement(dbg, line);
   while (el && !el.matches(".CodeMirror-code > div")) {
     el = el.parentElement;
   }
 
   return el;
 }