Bug 1514339 - Update Debugger [Frontend v109] Make gutter marker or breakpoint represent first breakpoint on line (#7410). r=davidwalsh
authorJason Laster <jlaster@mozilla.com>
Fri, 14 Dec 2018 14:45:59 -0500
changeset 451231 1ece4c0a980921719f9cbff62f0cd1d743a3e3bd
parent 451230 95d786a9ab1c9533a12fe42fe375088962c7f2ab
child 451232 9f3c3d04e8b00144c8c5e5d5c7be8a74f1985e78
push id35230
push userbtara@mozilla.com
push dateWed, 19 Dec 2018 04:55:30 +0000
treeherdermozilla-central@204ab379fb82 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidwalsh
bugs1514339
milestone66.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 1514339 - Update Debugger [Frontend v109] Make gutter marker or breakpoint represent first breakpoint on line (#7410). r=davidwalsh
devtools/client/debugger/new/src/components/Editor/Breakpoints.js
devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/index.js
devtools/client/debugger/new/src/selectors/index.js
devtools/client/debugger/new/src/selectors/visibleBreakpoints.js
devtools/client/debugger/new/src/utils/breakpoint/index.js
devtools/client/debugger/new/test/mochitest/browser.ini
devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-columns.js
devtools/client/debugger/new/test/mochitest/helpers.js
--- a/devtools/client/debugger/new/src/components/Editor/Breakpoints.js
+++ b/devtools/client/debugger/new/src/components/Editor/Breakpoints.js
@@ -1,19 +1,18 @@
 /* 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/>. */
 
 // @flow
 import { connect } from "react-redux";
 import React, { Component } from "react";
-
 import Breakpoint from "./Breakpoint";
 
-import { getSelectedSource, getVisibleBreakpoints } from "../../selectors";
+import { getSelectedSource, getFirstVisibleBreakpoints } from "../../selectors";
 import { makeLocationId } from "../../utils/breakpoint";
 import { isLoaded } from "../../utils/source";
 
 import type { Breakpoint as BreakpointType, Source } from "../../types";
 
 type Props = {
   selectedSource: Source,
   breakpoints: BreakpointType[],
@@ -49,11 +48,13 @@ class Breakpoints extends Component<Prop
           );
         })}
       </div>
     );
   }
 }
 
 export default connect(state => ({
-  breakpoints: getVisibleBreakpoints(state),
+  // Retrieves only the first breakpoint per line so that the
+  // breakpoint marker represents only the first breakpoint
+  breakpoints: getFirstVisibleBreakpoints(state),
   selectedSource: getSelectedSource(state)
 }))(Breakpoints);
--- a/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/index.js
+++ b/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/index.js
@@ -10,17 +10,20 @@ import { connect } from "react-redux";
 
 import ExceptionOption from "./ExceptionOption";
 
 import Breakpoint from "./Breakpoint";
 import BreakpointHeading from "./BreakpointHeading";
 
 import actions from "../../../actions";
 import { getDisplayPath } from "../../../utils/source";
-import { makeLocationId, sortBreakpoints } from "../../../utils/breakpoint";
+import {
+  makeLocationId,
+  sortFormattedBreakpoints
+} from "../../../utils/breakpoint";
 
 import { getSelectedSource, getBreakpointSources } from "../../../selectors";
 
 import type { Source } from "../../../types";
 import type { BreakpointSources } from "../../../selectors/breakpointSources";
 
 import "./Breakpoints.css";
 
@@ -74,17 +77,17 @@ class Breakpoints extends Component<Prop
     const { breakpointSources } = this.props;
     const sources = [
       ...breakpointSources.map(({ source, breakpoints }) => source)
     ];
 
     return [
       ...breakpointSources.map(({ source, breakpoints, i }) => {
         const path = getDisplayPath(source, sources);
-        const sortedBreakpoints = sortBreakpoints(breakpoints);
+        const sortedBreakpoints = sortFormattedBreakpoints(breakpoints);
 
         return [
           <BreakpointHeading
             source={source}
             sources={sources}
             path={path}
             key={source.url}
           />,
--- a/devtools/client/debugger/new/src/selectors/index.js
+++ b/devtools/client/debugger/new/src/selectors/index.js
@@ -22,17 +22,20 @@ export {
   getQuickOpenQuery,
   getQuickOpenType
 } from "../reducers/quick-open";
 
 export {
   getBreakpointAtLocation,
   getBreakpointsAtLine
 } from "./breakpointAtLocation";
-export { getVisibleBreakpoints } from "./visibleBreakpoints";
+export {
+  getVisibleBreakpoints,
+  getFirstVisibleBreakpoints
+} from "./visibleBreakpoints";
 export { inComponent } from "./inComponent";
 export { isSelectedFrameVisible } from "./isSelectedFrameVisible";
 export { getCallStackFrames } from "./getCallStackFrames";
 export { getVisibleSelectedFrame } from "./visibleSelectedFrame";
 export { getBreakpointSources } from "./breakpointSources";
 export { getXHRBreakpoints, shouldPauseOnAnyXHR } from "./breakpoints";
 export { visibleColumnBreakpoints } from "./visibleColumnBreakpoints";
 export { getVisiblePausePoints } from "./visiblePausePoints";
--- a/devtools/client/debugger/new/src/selectors/visibleBreakpoints.js
+++ b/devtools/client/debugger/new/src/selectors/visibleBreakpoints.js
@@ -1,19 +1,23 @@
 /* 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/>. */
 
 // @flow
 
+import { isGeneratedId } from "devtools-source-map";
+import { createSelector } from "reselect";
+import { uniqBy } from "lodash";
+
 import { getBreakpointsList } from "../reducers/breakpoints";
 import { getSelectedSource } from "../reducers/sources";
-import { isGeneratedId } from "devtools-source-map";
-import { createSelector } from "reselect";
+
 import memoize from "../utils/memoize";
+import { sortBreakpoints } from "../utils/breakpoint";
 
 import type { Breakpoint, Source } from "../types";
 
 function getLocation(breakpoint, isGeneratedSource) {
   return isGeneratedSource
     ? breakpoint.generatedLocation || breakpoint.location
     : breakpoint.location;
 }
@@ -51,8 +55,22 @@ export const getVisibleBreakpoints = cre
       return null;
     }
 
     return breakpoints
       .filter(bp => isVisible(bp, selectedSource))
       .map(bp => formatBreakpoint(bp, selectedSource));
   }
 );
+
+/*
+ * Finds the first breakpoint per line, which appear in the selected source.
+  */
+export const getFirstVisibleBreakpoints = createSelector(
+  getVisibleBreakpoints,
+  breakpoints => {
+    if (!breakpoints) {
+      return null;
+    }
+
+    return uniqBy(sortBreakpoints(breakpoints), bp => bp.location.line);
+  }
+);
--- a/devtools/client/debugger/new/src/utils/breakpoint/index.js
+++ b/devtools/client/debugger/new/src/utils/breakpoint/index.js
@@ -181,15 +181,25 @@ export function createPendingBreakpoint(
     condition: bp.condition,
     disabled: bp.disabled,
     location: pendingLocation,
     astLocation: bp.astLocation,
     generatedLocation: pendingGeneratedLocation
   };
 }
 
-export function sortBreakpoints(breakpoints: FormattedBreakpoint[]) {
+export function sortFormattedBreakpoints(breakpoints: FormattedBreakpoint[]) {
+  return _sortBreakpoints(breakpoints, "selectedLocation");
+}
+
+export function sortBreakpoints(breakpoints: Breakpoint[]) {
+  return _sortBreakpoints(breakpoints, "location");
+}
+
+function _sortBreakpoints(breakpoints: Array<Object>, property: string) {
   return sortBy(breakpoints, [
-    "selectedLocation.line",
-    ({ selectedLocation }) =>
-      selectedLocation.column === undefined || selectedLocation.column
+    // Priority: line number, undefined column, column number
+    `${property}.line`,
+    bp => {
+      return bp[property].column === undefined || bp[property].column;
+    }
   ]);
 }
--- a/devtools/client/debugger/new/test/mochitest/browser.ini
+++ b/devtools/client/debugger/new/test/mochitest/browser.ini
@@ -666,16 +666,17 @@ skip-if = ccov || (verify && debug && (o
 [browser_dbg-sourcemapped-stepping.js]
 skip-if = (os == 'win' && os_version == '10.0' && ccov) # Bug 1480680
 [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-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/new/test/mochitest/browser_dbg-breakpoints-columns.js
@@ -0,0 +1,106 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function getColumnBreakpointElements(dbg) {
+  return findAllElementsWithSelector(dbg, ".column-breakpoint");
+}
+
+async function assertConditionalBreakpointIsFocused(dbg) {
+  const input = findElement(dbg, "conditionalPanelInput");
+  await waitForElementFocus(dbg, input);
+}
+
+function waitForElementFocus(dbg, el) {
+  const doc = dbg.win.document;
+  return waitFor(() => doc.activeElement == el && doc.hasFocus());
+}
+
+function hasCondition(marker) {
+  return marker.classList.contains("has-condition");
+}
+
+async function setConditionalBreakpoint(dbg, index, condition) {
+  const {
+      addConditionalBreakpoint,
+      editBreakpoint
+  } = selectors.gutterContextMenu;
+  // Make this work with either add or edit menu items
+  const selector = `${addConditionalBreakpoint},${editBreakpoint}`;
+
+  rightClickElement(dbg, "breakpointItem", index);
+  selectContextMenuItem(dbg, selector);
+  await waitForElement(dbg, "conditionalPanelInput");
+  await assertConditionalBreakpointIsFocused(dbg);
+
+  // Position cursor reliably at the end of the text.
+  pressKey(dbg, "End");
+  type(dbg, condition);
+  pressKey(dbg, "Enter");
+}
+
+function removeBreakpointViaContext(dbg, index) {
+  rightClickElement(dbg, "breakpointItem", index);
+  selectContextMenuItem(dbg, "#node-menu-delete-self");
+}
+
+// Test enabling and disabling a breakpoint using the check boxes
+add_task(async function() {
+  const dbg = await initDebugger("doc-scripts.html", "simple1");
+  await pushPref("devtools.debugger.features.column-breakpoints", true);
+
+  await selectSource(dbg, "simple1");
+
+  // Scroll down to desired line so that column breakpoints render
+  getCM(dbg).setCursor({ line: 15, ch: 0 });
+
+  // Create a breakpoint at 15:undefined
+  await addBreakpoint(dbg, "simple1", 15);
+  
+  // Wait for column breakpoint markers
+  await waitForElementWithSelector(dbg, ".column-breakpoint");
+
+  let columnBreakpointMarkers = getColumnBreakpointElements(dbg);
+  ok(
+    columnBreakpointMarkers.length === 2, 
+      "2 column breakpoint markers display"
+  );
+
+  // Create a breakpoint at 15:8
+  columnBreakpointMarkers[0].click();
+
+  // Create a breakpoint at 15:28
+  columnBreakpointMarkers[1].click();
+
+  // Wait for breakpoints in right panel to render
+  await waitForState(dbg, state => {
+    return dbg.win.document.querySelectorAll(".breakpoints-list .breakpoint").length === 3;
+  })
+
+  // Scroll down in secondary pane so element we want to right-click is showing
+  dbg.win.document.querySelector(".secondary-panes").scrollTop = 100;
+
+  // Set a condition at 15:8
+  await setConditionalBreakpoint(dbg, 4, "Eight");
+
+  // Ensure column breakpoint is yellow
+  await waitForElementWithSelector(dbg, ".column-breakpoint.has-condition");
+  
+  // Remove the breakpoint from 15:undefined via the secondary pane context menu
+  removeBreakpointViaContext(dbg, 3);
+
+  // Ensure that there's still a marker on line 15
+  await waitForState(dbg, state => dbg.selectors.getBreakpointCount(state) == 2);
+  await waitForElementWithSelector(dbg, ".column-breakpoint.has-condition");
+  columnBreakpointMarkers = getColumnBreakpointElements(dbg);
+  ok(hasCondition(columnBreakpointMarkers[0]), "First column breakpoint has conditional style");
+
+  // Remove the breakpoint from 15:8
+  removeBreakpointViaContext(dbg, 3);
+
+  // Ensure there's still a marker and it has no condition
+  await waitForState(dbg, state => dbg.selectors.getBreakpointCount(state) == 1);
+  await waitForElementWithSelector(dbg, ".column-breakpoint");
+
+  // Ensure the first column breakpoint has no conditional style
+  await waitFor(() => !hasCondition(getColumnBreakpointElements(dbg)[0]));
+});
--- a/devtools/client/debugger/new/test/mochitest/helpers.js
+++ b/devtools/client/debugger/new/test/mochitest/helpers.js
@@ -1047,19 +1047,20 @@ const selectors = {
   },
   scopes: ".scopes-list",
   scopeNode: i => `.scopes-list .tree-node:nth-child(${i}) .object-label`,
   scopeValue: i =>
     `.scopes-list .tree-node:nth-child(${i}) .object-delimiter + *`,
   frame: i => `.frames ul li:nth-child(${i})`,
   frames: ".frames ul li",
   gutter: i => `.CodeMirror-code *:nth-child(${i}) .CodeMirror-linenumber`,
+  // These work for bobth the breakpoint listing and gutter marker
   gutterContextMenu: {
-    addConditionalBreakpoint: "#node-menu-add-conditional-breakpoint",
-    editBreakpoint: "#node-menu-edit-conditional-breakpoint"
+    addConditionalBreakpoint: "#node-menu-add-condition, #node-menu-add-conditional-breakpoint",
+    editBreakpoint: "#node-menu-edit-condition, #node-menu-edit-conditional-breakpoint"
   },
   menuitem: i => `menupopup menuitem:nth-child(${i})`,
   pauseOnExceptions: ".pause-exceptions",
   breakpoint: ".CodeMirror-code > .new-breakpoint",
   highlightLine: ".CodeMirror-code > .highlight-line",
   debugLine: ".new-debug-line",
   debugErrorLine: ".new-debug-line-error",
   codeMirror: ".CodeMirror",
@@ -1111,16 +1112,20 @@ function findElement(dbg, elementName, .
 }
 
 function findElementWithSelector(dbg, selector) {
   return dbg.win.document.querySelector(selector);
 }
 
 function findAllElements(dbg, elementName, ...args) {
   const selector = getSelector(elementName, ...args);
+  return findAllElementsWithSelector(dbg, selector)
+}
+
+function findAllElementsWithSelector(dbg, selector) {
   return dbg.win.document.querySelectorAll(selector);
 }
 
 /**
  * Simulates a mouse click in the debugger DOM.
  *
  * @memberof mochitest/helpers
  * @param {Object} dbg
@@ -1154,16 +1159,17 @@ function dblClickElement(dbg, elementNam
     { clickCount: 2 },
     dbg.win
   );
 }
 
 function rightClickElement(dbg, elementName, ...args) {
   const selector = getSelector(elementName, ...args);
   const doc = dbg.win.document;
+
   return EventUtils.synthesizeMouseAtCenter(
     doc.querySelector(selector),
     { type: "contextmenu" },
     dbg.win
   );
 }
 
 function selectContextMenuItem(dbg, selector) {