Bug 1527122 - [release 125] [Breakpoints] Improve logpoints ux (#7811). r=dwalsh
authorHubert Boma Manilla <b4bomsy@gmail.com>
Mon, 11 Feb 2019 17:02:28 -0800
changeset 458687 d73a4f61cf59
parent 458686 05720cbd2a93
child 458688 1728a42ced0c
push id35544
push userccoroiu@mozilla.com
push dateTue, 12 Feb 2019 16:29:08 +0000
treeherdermozilla-central@c849fb69e2e7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdwalsh
bugs1527122
milestone67.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 1527122 - [release 125] [Breakpoints] Improve logpoints ux (#7811). r=dwalsh
devtools/client/debugger/new/dist/debugger.css
devtools/client/debugger/new/src/components/Editor/Breakpoint.js
devtools/client/debugger/new/src/components/Editor/ColumnBreakpoint.js
devtools/client/debugger/new/src/components/Editor/ColumnBreakpoints.css
devtools/client/debugger/new/src/components/Editor/ConditionalPanel.js
devtools/client/debugger/new/src/components/Editor/Editor.css
devtools/client/debugger/new/src/components/Editor/menus/breakpoints.js
devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/Breakpoint.js
devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/Breakpoints.css
devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/BreakpointsContextMenu.js
devtools/client/debugger/new/src/types.js
devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
devtools/client/debugger/new/test/mochitest/helpers.js
devtools/client/locales/en-US/debugger.properties
--- a/devtools/client/debugger/new/dist/debugger.css
+++ b/devtools/client/debugger/new/dist/debugger.css
@@ -2994,19 +2994,19 @@ menuseparator {
   fill-opacity: 0.5;
 }
 
 .column-breakpoint.has-condition svg {
   fill: var(--theme-graphs-yellow);
   stroke: var(--theme-graphs-orange);
 }
 
-.column-breakpoint.has-condition.log svg {
-  fill: var(--theme-graphs-orange);
-  stroke: var(--theme-graphs-yellow);
+.column-breakpoint.has-log svg {
+  fill: var(--theme-graphs-purple);
+  stroke: var(--purple-60);
 }
 
 .theme-dark .column-breakpoint.active svg {
   fill: var(--blue-55);
   stroke: var(--blue-40);
 }
 
 .theme-dark .column-breakpoint.disabled svg {
@@ -3063,17 +3063,19 @@ menuseparator {
   margin-right: -7px;
   overflow: hidden;
 }
 
 .editor-wrapper {
   min-width: 0 !important;
 }
 
-.CodeMirror.cm-s-mozilla, .CodeMirror-scroll, .CodeMirror-sizer {
+.CodeMirror.cm-s-mozilla,
+.CodeMirror-scroll,
+.CodeMirror-sizer {
   overflow-anchor: none;
 }
 
 .theme-dark {
   --theme-conditional-breakpoint-color: #9fa4a9;
 }
 
 .theme-light {
@@ -3204,19 +3206,19 @@ html[dir="rtl"] .editor-mount {
   right: -16px;
 }
 
 .new-breakpoint.has-condition .CodeMirror-gutter-wrapper svg {
   fill: var(--theme-graphs-yellow);
   stroke: var(--theme-graphs-orange);
 }
 
-.new-breakpoint.has-condition.log .CodeMirror-gutter-wrapper svg {
-  fill: var(--theme-graphs-orange);
-  stroke: var(--theme-graphs-yellow);
+.new-breakpoint.has-log .CodeMirror-gutter-wrapper svg {
+  fill: var(--theme-graphs-purple);
+  stroke: var(--purple-60);
 }
 
 .editor.new-breakpoint.breakpoint-disabled svg {
   fill: var(--breakpoint-fill-disabled);
   stroke: var(--breakpoint-stroke-disabled);
   fill-opacity: 0.5;
 }
 
@@ -3475,30 +3477,22 @@ html[dir="rtl"] .breakpoints-exceptions 
 }
 
 html:not([dir="rtl"]) .breakpoints-list .breakpoint,
 html:not([dir="rtl"]) .breakpoints-list .breakpoint-heading,
 html:not([dir="rtl"]) .breakpoints-exceptions {
   border-left: 4px solid transparent;
 }
 
-html:not([dir="rtl"]) .breakpoints-list .breakpoint.is-conditional {
-  border-left-color: var(--theme-graphs-yellow);
-}
-
-html[dir="rtl"] .breakpoints-list .breakpoint.is-conditional {
-  border-right-color: var(--theme-graphs-yellow);
-}
-
-html:not([dir="rtl"]) .breakpoints-list .breakpoint.is-conditional.log {
-  border-left-color: var(--theme-graphs-orange);
-}
-
-html[dir="rtl"] .breakpoints-list .breakpoint.is-conditional.log {
-  border-right-color: var(--theme-graphs-orange);
+html .breakpoints-list .breakpoint.is-conditional {
+  border-inline-start-color: var(--theme-graphs-yellow);
+}
+
+html .breakpoints-list .breakpoint.is-log {
+  border-inline-start-color: var(--theme-graphs-purple);
 }
 
 html .breakpoints-list .breakpoint.paused {
   background-color: var(--theme-toolbar-background-alt);
   border-color: var(--breakpoint-active-color);
 }
 
 .breakpoints-list .breakpoint.disabled .breakpoint-label {
--- a/devtools/client/debugger/new/src/components/Editor/Breakpoint.js
+++ b/devtools/client/debugger/new/src/components/Editor/Breakpoint.js
@@ -52,16 +52,17 @@ class Breakpoint extends PureComponent<P
       return;
     }
 
     const line = toEditorLine(sourceId, this.selectedLocation.line);
 
     doc.setGutterMarker(line, "breakpoints", null);
     doc.removeLineClass(line, "line", "new-breakpoint");
     doc.removeLineClass(line, "line", "has-condition");
+    doc.removeLineClass(line, "line", "has-log");
   }
 
   get selectedLocation() {
     const { breakpoint, selectedSource } = this.props;
     return getSelectedLocation(breakpoint, selectedSource);
   }
 
   makeMarker() {
@@ -128,22 +129,23 @@ class Breakpoint extends PureComponent<P
 
     const sourceId = selectedSource.id;
     const line = toEditorLine(sourceId, this.selectedLocation.line);
     const doc = getDocument(sourceId);
 
     doc.setGutterMarker(line, "breakpoints", this.makeMarker());
 
     editor.codeMirror.addLineClass(line, "line", "new-breakpoint");
-    if (breakpoint.options.condition) {
+    editor.codeMirror.removeLineClass(line, "line", "has-condition");
+    editor.codeMirror.removeLineClass(line, "line", "has-log");
+
+    if (breakpoint.options.logValue) {
+      editor.codeMirror.addLineClass(line, "line", "has-log");
+    } else if (breakpoint.options.condition) {
       editor.codeMirror.addLineClass(line, "line", "has-condition");
-    } else if (breakpoint.options.logValue) {
-      editor.codeMirror.addLineClass(line, "line", "has-condition log");
-    } else {
-      editor.codeMirror.removeLineClass(line, "line", "has-condition");
     }
   };
 
   render() {
     return null;
   }
 }
 
--- a/devtools/client/debugger/new/src/components/Editor/ColumnBreakpoint.js
+++ b/devtools/client/debugger/new/src/components/Editor/ColumnBreakpoint.js
@@ -27,21 +27,25 @@ type Props = {
   breakpointActions: BreakpointItemActions
 };
 
 const breakpointImg = document.createElement("button");
 ReactDOM.render(<Svg name={"column-marker"} />, breakpointImg);
 
 function makeBookmark({ breakpoint }, { onClick, onContextMenu }) {
   const bp = breakpointImg.cloneNode(true);
-  const condition = breakpoint && breakpoint.options.condition;
+  if (!breakpoint) {
+    return;
+  }
+  const { condition, logValue } = breakpoint.options;
   const isActive = breakpoint && !breakpoint.disabled;
 
   bp.className = classnames("column-breakpoint", {
     "has-condition": condition,
+    "has-log": logValue,
     active: isActive,
     disabled: !isActive
   });
 
   if (condition) {
     bp.setAttribute("title", condition);
   }
   bp.onclick = onClick;
--- a/devtools/client/debugger/new/src/components/Editor/ColumnBreakpoints.css
+++ b/devtools/client/debugger/new/src/components/Editor/ColumnBreakpoints.css
@@ -31,19 +31,19 @@
   fill-opacity: 0.5;
 }
 
 .column-breakpoint.has-condition svg {
   fill: var(--theme-graphs-yellow);
   stroke: var(--theme-graphs-orange);
 }
 
-.column-breakpoint.has-condition.log svg {
-  fill: var(--theme-graphs-orange);
-  stroke: var(--theme-graphs-yellow);
+.column-breakpoint.has-log svg {
+  fill: var(--theme-graphs-purple);
+  stroke: var(--purple-60);
 }
 
 .theme-dark .column-breakpoint.active svg {
   fill: var(--blue-55);
   stroke: var(--blue-40);
 }
 
 .theme-dark .column-breakpoint.disabled svg {
--- a/devtools/client/debugger/new/src/components/Editor/ConditionalPanel.js
+++ b/devtools/client/debugger/new/src/components/Editor/ConditionalPanel.js
@@ -57,22 +57,24 @@ export class ConditionalPanel extends Pu
   onKey = (e: SyntheticKeyboardEvent<HTMLInputElement>) => {
     if (e.key === "Enter") {
       this.saveAndClose();
     } else if (e.key === "Escape") {
       this.props.closeConditionalPanel();
     }
   };
 
-  setBreakpoint(condition: string) {
-    const { location, log } = this.props;
-    return this.props.setBreakpointOptions(
-      location,
-      log ? { logValue: condition } : { condition }
-    );
+  setBreakpoint(value: string) {
+    const { location, log, breakpoint } = this.props;
+    const options = breakpoint ? breakpoint.options : {};
+    const type = log ? "logValue" : "condition";
+    return this.props.setBreakpointOptions(location, {
+      ...options,
+      [type]: value
+    });
   }
 
   clearConditionalPanel() {
     if (this.cbPanel) {
       this.cbPanel.clear();
       this.cbPanel = null;
     }
     if (this.scrollParent) {
--- a/devtools/client/debugger/new/src/components/Editor/Editor.css
+++ b/devtools/client/debugger/new/src/components/Editor/Editor.css
@@ -19,17 +19,19 @@
   margin-right: -7px;
   overflow: hidden;
 }
 
 .editor-wrapper {
   min-width: 0 !important;
 }
 
-.CodeMirror.cm-s-mozilla, .CodeMirror-scroll, .CodeMirror-sizer {
+.CodeMirror.cm-s-mozilla,
+.CodeMirror-scroll,
+.CodeMirror-sizer {
   overflow-anchor: none;
 }
 
 .theme-dark {
   --theme-conditional-breakpoint-color: #9fa4a9;
 }
 
 .theme-light {
@@ -160,19 +162,19 @@ html[dir="rtl"] .editor-mount {
   right: -16px;
 }
 
 .new-breakpoint.has-condition .CodeMirror-gutter-wrapper svg {
   fill: var(--theme-graphs-yellow);
   stroke: var(--theme-graphs-orange);
 }
 
-.new-breakpoint.has-condition.log .CodeMirror-gutter-wrapper svg {
-  fill: var(--theme-graphs-orange);
-  stroke: var(--theme-graphs-yellow);
+.new-breakpoint.has-log .CodeMirror-gutter-wrapper svg {
+  fill: var(--theme-graphs-purple);
+  stroke: var(--purple-60);
 }
 
 .editor.new-breakpoint.breakpoint-disabled svg {
   fill: var(--breakpoint-fill-disabled);
   stroke: var(--breakpoint-stroke-disabled);
   fill-opacity: 0.5;
 }
 
--- a/devtools/client/debugger/new/src/components/Editor/menus/breakpoints.js
+++ b/devtools/client/debugger/new/src/components/Editor/menus/breakpoints.js
@@ -28,40 +28,16 @@ export const removeBreakpointItem = (
   id: "node-menu-remove-breakpoint",
   label: L10N.getStr("editor.removeBreakpoint"),
   accesskey: L10N.getStr("shortcuts.toggleBreakpoint.accesskey"),
   disabled: false,
   click: () => breakpointActions.removeBreakpoint(breakpoint),
   accelerator: L10N.getStr("toggleBreakpoint.key")
 });
 
-export const createConditionalBreakpointItem = (
-  location: SourceLocation,
-  breakpointActions: BreakpointItemActions
-) => ({
-  id: "node-menu-add-conditional-breakpoint",
-  label: L10N.getStr("editor.addConditionalBreakpoint"),
-  accelerator: L10N.getStr("toggleCondPanel.key"),
-  accesskey: L10N.getStr("editor.addConditionBreakpoint.accesskey"),
-  disabled: false,
-  click: () => breakpointActions.openConditionalPanel(location)
-});
-
-export const createLogBreakpointItem = (
-  location: SourceLocation,
-  breakpointActions: BreakpointItemActions
-) => ({
-  id: "node-menu-add-log-breakpoint",
-  label: L10N.getStr("editor.addLogBreakpoint"),
-  accelerator: L10N.getStr("toggleCondPanel.key"),
-  accesskey: L10N.getStr("editor.addConditionBreakpoint.accesskey"),
-  disabled: false,
-  click: () => breakpointActions.openConditionalPanel(location)
-});
-
 export const addConditionalBreakpointItem = (
   location: SourceLocation,
   breakpointActions: BreakpointItemActions
 ) => ({
   id: "node-menu-add-conditional-breakpoint",
   label: L10N.getStr("editor.addConditionBreakpoint"),
   accelerator: L10N.getStr("toggleCondPanel.key"),
   accesskey: L10N.getStr("editor.addConditionBreakpoint.accesskey"),
@@ -118,20 +94,20 @@ export const editLogPointItem = (
   accelerator: L10N.getStr("toggleCondPanel.key")
 });
 
 export const logPointItem = (
   breakpoint: Breakpoint,
   breakpointActions: BreakpointItemActions
 ) => {
   const {
-    options: { condition },
+    options: { logValue },
     location
   } = breakpoint;
-  return condition
+  return logValue
     ? editLogPointItem(location, breakpointActions)
     : addLogPointItem(location, breakpointActions);
 };
 
 export const toggleDisabledBreakpointItem = (
   breakpoint: Breakpoint,
   breakpointActions: BreakpointItemActions
 ) => {
@@ -171,30 +147,31 @@ export function breakpointItems(
     );
   }
 
   items.push(conditionalBreakpointItem(breakpoint, breakpointActions));
 
   if (features.logPoints) {
     items.push(logPointItem(breakpoint, breakpointActions));
   }
+
   return items;
 }
 
 export function createBreakpointItems(
   location: SourceLocation,
   breakpointActions: BreakpointItemActions
 ) {
   const items = [
     addBreakpointItem(location, breakpointActions),
-    createConditionalBreakpointItem(location, breakpointActions)
+    addConditionalBreakpointItem(location, breakpointActions)
   ];
 
   if (features.logPoints) {
-    items.push(createLogBreakpointItem(location, breakpointActions));
+    items.push(addLogPointItem(location, breakpointActions));
   }
   return items;
 }
 
 // ToDo: Only enable if there are more than one breakpoints on a line?
 export const removeBreakpointsOnLineItem = (
   location: SourceLocation,
   breakpointActions: BreakpointItemActions
--- a/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/Breakpoint.js
+++ b/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/Breakpoint.js
@@ -122,20 +122,18 @@ class Breakpoint extends PureComponent<P
       ? `0x${line.toString(16).toUpperCase()}`
       : `${line}${columnVal}`;
 
     return bpLocation;
   }
 
   getBreakpointText() {
     const { breakpoint, selectedSource } = this.props;
-    return (
-      breakpoint.options.condition ||
-      getSelectedText(breakpoint, selectedSource)
-    );
+    const { condition, logValue } = breakpoint.options;
+    return logValue || condition || getSelectedText(breakpoint, selectedSource);
   }
 
   highlightText = memoize(
     (text = "", editor) => {
       if (!editor.CodeMirror) {
         return { __html: text };
       }
 
@@ -146,25 +144,24 @@ class Breakpoint extends PureComponent<P
     (text, editor) => `${text} - ${editor.CodeMirror ? "editor" : ""}`
   );
 
   /* eslint-disable react/no-danger */
   render() {
     const { breakpoint } = this.props;
     const text = this.getBreakpointText();
     const editor = getEditor();
-
     return (
       <div
         className={classnames({
           breakpoint,
           paused: this.isCurrentlyPausedAtBreakpoint(),
           disabled: breakpoint.disabled,
           "is-conditional": !!breakpoint.options.condition,
-          log: !!breakpoint.options.logValue
+          "is-log": !!breakpoint.options.logValue
         })}
         onClick={this.selectBreakpoint}
         onDoubleClick={this.onDoubleClick}
         onContextMenu={this.onContextMenu}
       >
         <input
           id={breakpoint.id}
           type="checkbox"
--- a/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/Breakpoints.css
+++ b/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/Breakpoints.css
@@ -105,30 +105,22 @@ html[dir="rtl"] .breakpoints-exceptions 
 }
 
 html:not([dir="rtl"]) .breakpoints-list .breakpoint,
 html:not([dir="rtl"]) .breakpoints-list .breakpoint-heading,
 html:not([dir="rtl"]) .breakpoints-exceptions {
   border-left: 4px solid transparent;
 }
 
-html:not([dir="rtl"]) .breakpoints-list .breakpoint.is-conditional {
-  border-left-color: var(--theme-graphs-yellow);
+html .breakpoints-list .breakpoint.is-conditional {
+  border-inline-start-color: var(--theme-graphs-yellow);
 }
 
-html[dir="rtl"] .breakpoints-list .breakpoint.is-conditional {
-  border-right-color: var(--theme-graphs-yellow);
-}
-
-html:not([dir="rtl"]) .breakpoints-list .breakpoint.is-conditional.log {
-  border-left-color: var(--theme-graphs-orange);
-}
-
-html[dir="rtl"] .breakpoints-list .breakpoint.is-conditional.log {
-  border-right-color: var(--theme-graphs-orange);
+html .breakpoints-list .breakpoint.is-log {
+  border-inline-start-color: var(--theme-graphs-purple);
 }
 
 html .breakpoints-list .breakpoint.paused {
   background-color: var(--theme-toolbar-background-alt);
   border-color: var(--breakpoint-active-color);
 }
 
 .breakpoints-list .breakpoint.disabled .breakpoint-label {
--- a/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/BreakpointsContextMenu.js
+++ b/devtools/client/debugger/new/src/components/SecondaryPanes/Breakpoints/BreakpointsContextMenu.js
@@ -2,16 +2,18 @@
  * 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 { buildMenu, showMenu } from "devtools-contextmenu";
 import { getSelectedLocation } from "../../../utils/source-maps";
 import actions from "../../../actions";
+import { features } from "../../../utils/prefs";
+
 import type { Breakpoint, Source } from "../../../types";
 
 type Props = {
   breakpoint: Breakpoint,
   breakpoints: Breakpoint[],
   selectedSource: Source,
   removeBreakpoint: typeof actions.removeBreakpoint,
   removeBreakpoints: typeof actions.removeBreakpoints,
@@ -184,17 +186,21 @@ export default function showContextMenu(
     click: () => toggleBreakpoints(true, otherEnabledBreakpoints)
   };
 
   const removeConditionItem = {
     id: "node-menu-remove-condition",
     label: removeConditionLabel,
     accesskey: removeConditionKey,
     disabled: false,
-    click: () => setBreakpointOptions(selectedLocation, {})
+    click: () =>
+      setBreakpointOptions(selectedLocation, {
+        ...breakpoint.options,
+        condition: null
+      })
   };
 
   const addConditionItem = {
     id: "node-menu-add-condition",
     label: addConditionLabel,
     accesskey: addConditionKey,
     click: () => {
       selectSpecificLocation(selectedLocation);
@@ -207,16 +213,50 @@ export default function showContextMenu(
     label: editConditionLabel,
     accesskey: editConditionKey,
     click: () => {
       selectSpecificLocation(selectedLocation);
       openConditionalPanel(selectedLocation);
     }
   };
 
+  const addLogPointItem = {
+    id: "node-menu-add-log-point",
+    label: L10N.getStr("editor.addLogPoint"),
+    accesskey: L10N.getStr("editor.addLogPoint.accesskey"),
+    disabled: false,
+    click: () => openConditionalPanel(selectedLocation, true),
+    accelerator: L10N.getStr("toggleCondPanel.key")
+  };
+
+  const editLogPointItem = {
+    id: "node-menu-edit-log-point",
+    label: L10N.getStr("editor.editLogPoint"),
+    accesskey: L10N.getStr("editor.addLogPoint.accesskey"),
+    disabled: false,
+    click: () => openConditionalPanel(selectedLocation, true),
+    accelerator: L10N.getStr("toggleCondPanel.key")
+  };
+
+  const removeLogPointItem = {
+    id: "node-menu-remove-log",
+    label: L10N.getStr("editor.removeLogPoint.label"),
+    accesskey: L10N.getStr("editor.removeLogPoint.accesskey"),
+    disabled: false,
+    click: () =>
+      setBreakpointOptions(selectedLocation, {
+        ...breakpoint.options,
+        logValue: null
+      })
+  };
+
+  const logPointItem = breakpoint.options.logValue
+    ? editLogPointItem
+    : addLogPointItem;
+
   const hideEnableSelfItem = !breakpoint.disabled;
   const hideEnableAllItem = disabledBreakpoints.length === 0;
   const hideEnableOthersItem = otherDisabledBreakpoints.length === 0;
   const hideDisableAllItem = enabledBreakpoints.length === 0;
   const hideDisableOthersItem = otherEnabledBreakpoints.length === 0;
   const hideDisableSelfItem = breakpoint.disabled;
 
   const items = [
@@ -249,14 +289,22 @@ export default function showContextMenu(
     },
     {
       item: editConditionItem,
       hidden: () => !breakpoint.options.condition
     },
     {
       item: removeConditionItem,
       hidden: () => !breakpoint.options.condition
+    },
+    {
+      item: logPointItem,
+      hidden: () => !features.logPoints
+    },
+    {
+      item: removeLogPointItem,
+      hidden: () => !features.logPoints || !breakpoint.options.logValue
     }
   ];
 
   showMenu(contextMenuEvent, buildMenu(items));
   return null;
 }
--- a/devtools/client/debugger/new/src/types.js
+++ b/devtools/client/debugger/new/src/types.js
@@ -125,18 +125,18 @@ export type Breakpoint = {|
   +options: BreakpointOptions
 |};
 
 /**
  * Options for a breakpoint that can be modified by the user.
  */
 export type BreakpointOptions = {
   hidden?: boolean,
-  condition?: string,
-  logValue?: string
+  condition?: string | null,
+  logValue?: string | null
 };
 
 export type BreakpointActor = {|
   +actor: ActorId,
   +source: SourceActor
 |};
 
 /**
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
@@ -12,30 +12,72 @@ function findBreakpoint(dbg, url, line, 
   return getBreakpoint(getState(), location);
 }
 
 function getLineEl(dbg, line) {
   const lines = dbg.win.document.querySelectorAll(".CodeMirror-code > div");
   return lines[line - 1];
 }
 
-function assertEditorBreakpoint(dbg, line, shouldExist) {
-  const exists = getLineEl(dbg, line).classList.contains("has-condition");
+function assertEditorBreakpoint(
+  dbg,
+  line,
+  { hasCondition = false, hasLog = false } = {}
+) {
+  const hasConditionClass = getLineEl(dbg, line).classList.contains(
+    "has-condition"
+  );
 
   ok(
-    exists === shouldExist,
-    `Breakpoint ${shouldExist ? "exists" : "does not exist"} on line ${line}`
+    hasConditionClass === hasCondition,
+    `Breakpoint condition ${
+      hasCondition ? "exists" : "does not exist"
+    } on line ${line}`
+  );
+
+  const hasLogClass = getLineEl(dbg, line).classList.contains("has-log");
+
+  ok(
+    hasLogClass === hasLog,
+    `Breakpoint log ${hasLog ? "exists" : "does not exist"} on line ${line}`
   );
 }
 
 function waitForElementFocus(dbg, el) {
   const doc = dbg.win.document;
   return waitFor(() => doc.activeElement == el && doc.hasFocus());
 }
 
+function waitForBreakpoint(dbg, url, line) {
+  return waitForState(dbg, () => findBreakpoint(dbg, url, line));
+}
+
+function waitForBreakpointWithCondition(dbg, url, line, cond) {
+  return waitForState(dbg, () => {
+    const bp = findBreakpoint(dbg, url, line);
+    return (
+      bp && bp.options.condition && (!cond || bp.options.condition == cond)
+    );
+  });
+}
+
+function waitForBreakpointWithLog(dbg, url, line) {
+  return waitForState(dbg, () => {
+    const bp = findBreakpoint(dbg, url, line);
+    return bp && bp.options.logValue;
+  });
+}
+
+function waitForBreakpointWithoutCondition(dbg, url, line) {
+  return waitForState(dbg, () => {
+    const bp = findBreakpoint(dbg, url, line);
+    return bp && !bp.options.condition;
+  });
+}
+
 async function assertConditionalBreakpointIsFocused(dbg) {
   const input = findElement(dbg, "conditionalPanelInput");
   await waitForElementFocus(dbg, input);
 }
 
 async function setConditionalBreakpoint(dbg, index, condition) {
   const {
     addConditionalBreakpoint,
@@ -50,57 +92,79 @@ async function setConditionalBreakpoint(
   await assertConditionalBreakpointIsFocused(dbg);
 
   // Position cursor reliably at the end of the text.
   pressKey(dbg, "End");
   type(dbg, condition);
   pressKey(dbg, "Enter");
 }
 
+async function setLogPoint(dbg, index, value) {
+  const { addLogPoint, editLogPoint } = selectors.gutterContextMenu;
+
+  // Make this work with either add or edit menu items
+  const selector = `${addLogPoint},${editLogPoint}`;
+
+  rightClickElement(dbg, "gutter", 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, value);
+  pressKey(dbg, "Enter");
+}
+
 add_task(async function() {
   const dbg = await initDebugger("doc-scripts.html", "simple2");
   await pushPref("devtools.debugger.features.column-breakpoints", false);
+  await pushPref("devtools.debugger.features.log-points", true);
 
   await selectSource(dbg, "simple2");
   await waitForSelectedSource(dbg, "simple2");
 
   await setConditionalBreakpoint(dbg, 5, "1");
   await waitForDispatch(dbg, "ADD_BREAKPOINT");
+  await waitForBreakpointWithCondition(dbg, "simple2", 5);
 
   let bp = findBreakpoint(dbg, "simple2", 5);
   is(bp.options.condition, "1", "breakpoint is created with the condition");
-  assertEditorBreakpoint(dbg, 5, true);
+  assertEditorBreakpoint(dbg, 5, { hasCondition: true });
 
-  // Edit the conditional breakpoint set above
-  const bpCondition1 = waitForDispatch(dbg, "SET_BREAKPOINT_OPTIONS");
+  info("Edit the conditional breakpoint set above");
   await setConditionalBreakpoint(dbg, 5, "2");
-  await bpCondition1;
+  await waitForBreakpointWithCondition(dbg, "simple2", 5, "12");
 
   bp = findBreakpoint(dbg, "simple2", 5);
   is(bp.options.condition, "12", "breakpoint is created with the condition");
-  assertEditorBreakpoint(dbg, 5, true);
+  assertEditorBreakpoint(dbg, 5, { hasCondition: true });
 
   clickElement(dbg, "gutter", 5);
   await waitForDispatch(dbg, "REMOVE_BREAKPOINT");
   bp = findBreakpoint(dbg, "simple2", 5);
   is(bp, null, "breakpoint was removed");
-  assertEditorBreakpoint(dbg, 5, false);
+  assertEditorBreakpoint(dbg, 5);
 
-  // Adding a condition to a breakpoint
+  info("Adding a condition to a breakpoint");
   clickElement(dbg, "gutter", 5);
   await waitForDispatch(dbg, "ADD_BREAKPOINT");
-  const bpCondition2 = waitForDispatch(dbg, "SET_BREAKPOINT_OPTIONS");
   await setConditionalBreakpoint(dbg, 5, "1");
-  await bpCondition2;
+  await waitForBreakpointWithCondition(dbg, "simple2", 5);
 
   bp = findBreakpoint(dbg, "simple2", 5);
   is(bp.options.condition, "1", "breakpoint is created with the condition");
-  assertEditorBreakpoint(dbg, 5, true);
+  assertEditorBreakpoint(dbg, 5, { hasCondition: true });
 
-  const bpCondition3 = waitForDispatch(dbg, "SET_BREAKPOINT_OPTIONS");
-  // right click breakpoint in breakpoints list
   rightClickElement(dbg, "breakpointItem", 3);
-  // select "remove condition";
+  info('select "remove condition"');
   selectContextMenuItem(dbg, selectors.breakpointContextMenu.removeCondition);
-  await bpCondition3;
+  await waitForBreakpointWithoutCondition(dbg, "simple2", 5);
   bp = findBreakpoint(dbg, "simple2", 5);
   is(bp.options.condition, undefined, "breakpoint condition removed");
+
+  info('Add "log point"');
+  await setLogPoint(dbg, 5, "44");
+  assertEditorBreakpoint(dbg, 5, { hasLog: true });
+  await waitForBreakpointWithLog(dbg, "simple2", 5);
+  bp = findBreakpoint(dbg, "simple2", 5);
+  is(bp.options.logValue, "44", "breakpoint condition removed");
 });
--- a/devtools/client/debugger/new/test/mochitest/helpers.js
+++ b/devtools/client/debugger/new/test/mochitest/helpers.js
@@ -1108,17 +1108,19 @@ const selectors = {
   frame: i => `.frames [role="list"] [role="listitem"]:nth-child(${i})`,
   frames: '.frames [role="list"] [role="listitem"]',
   gutter: i => `.CodeMirror-code *:nth-child(${i}) .CodeMirror-linenumber`,
   // These work for bobth the breakpoint listing and gutter marker
   gutterContextMenu: {
     addConditionalBreakpoint:
       "#node-menu-add-condition, #node-menu-add-conditional-breakpoint",
     editConditionalBreakpoint:
-      "#node-menu-edit-condition, #node-menu-edit-conditional-breakpoint"
+      "#node-menu-edit-condition, #node-menu-edit-conditional-breakpoint",
+    addLogPoint: "#node-menu-add-log-point",
+    editLogPoint: "#node-menu-edit-log-point"
   },
   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",
--- a/devtools/client/locales/en-US/debugger.properties
+++ b/devtools/client/locales/en-US/debugger.properties
@@ -529,16 +529,22 @@ editor.addLogBreakpoint=Add log point
 # LOCALIZATION NOTE (editor.addLogPoint): Editor gutter context
 # menu item for adding a log point on a line.
 editor.addLogPoint=Add log
 editor.addLogPoint.accesskey=l
 
 # LOCALIZATION NOTE (editor.editLogPoint): Editor gutter context menu item
 # for editing a log point already set on a line.
 editor.editLogPoint=Edit log
+editor.editLogPoint.accesskey=E
+
+# LOCALIZATION NOTE (editor.removeLogPoint): Context menu item for removing
+# a log point on a line.
+editor.removeLogPoint.label=Remove log
+editor.removeLogPoint.accesskey=V
 
 # LOCALIZATION NOTE (editor.conditionalPanel.placeholder): Placeholder text for
 # input element inside ConditionalPanel component
 editor.conditionalPanel.placeholder=This breakpoint will pause when the expression is true
 
 # LOCALIZATION NOTE (editor.conditionalPanel.logPoint.placeholder): Placeholder text for
 # input element inside ConditionalPanel component when a log point is set
 editor.conditionalPanel.logPoint.placeholder=This breakpoint will log the result of the expression