Bug 1822307 - [devtools] Ensure checking highlightedLineRange's related source. r=devtools-reviewers,nchevobbe

We were not reading highlightedLineRange's sourceId
and so we might have been highlighting the wrong lines.

Move some logic up to Editor/index which knows about the displayed source.
This also help completely disable the HighlightedLines component until
an highlights start.

Differential Revision: https://phabricator.services.mozilla.com/D175878

devtools/client/debugger/src/actions/tests/ui.spec.js

@@ -2,13 +2,20 @@
  * 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/>. */
 
-import { createStore, selectors, actions } from "../../utils/test-head";
+import {
+  createStore,
+  selectors,
+  actions,
+  makeSource,
+} from "../../utils/test-head";
+import { createLocation } from "../../utils/location";
+import { mockCommandClient } from "./helpers/mockCommandClient";
 
 const {
   getActiveSearch,
   getFrameworkGroupingState,
   getPaneCollapse,
-  getHighlightedLineRange,
+  getHighlightedLineRangeForSelectedSource,
 } = selectors;
 
 describe("ui", () => {
@@ -41,18 +48,43 @@ describe("ui", () => {
     expect(getFrameworkGroupingState(getState())).toBe(!currentState);
   });
 
-  it("should highlight lines", () => {
-    const { dispatch, getState } = createStore();
-    const range = { start: 3, end: 5, sourceId: "2" };
+  it("should highlight lines", async () => {
+    const { dispatch, getState } = createStore(mockCommandClient);
+    const base = await dispatch(
+      actions.newGeneratedSource(makeSource("base.js"))
+    );
+    const sourceActor = selectors.getFirstSourceActorForGeneratedSource(
+      getState(),
+      base.id
+    );
+    const cx = selectors.getThreadContext(getState());
+    //await dispatch(actions.selectSource(cx, base, sourceActor));
+    const location = createLocation({
+      source: base,
+      line: 1,
+      sourceActor,
+    });
+    await dispatch(actions.selectLocation(cx, location));
+
+    const range = { start: 3, end: 5, sourceId: base.id };
     dispatch(actions.highlightLineRange(range));
-    expect(getHighlightedLineRange(getState())).toEqual(range);
+    expect(getHighlightedLineRangeForSelectedSource(getState())).toEqual(range);
   });
 
-  it("should clear highlight lines", () => {
-    const { dispatch, getState } = createStore();
+  it("should clear highlight lines", async () => {
+    const { dispatch, getState } = createStore(mockCommandClient);
+    const base = await dispatch(
+      actions.newGeneratedSource(makeSource("base.js"))
+    );
+    const sourceActor = selectors.getFirstSourceActorForGeneratedSource(
+      getState(),
+      base.id
+    );
+    const cx = selectors.getThreadContext(getState());
+    await dispatch(actions.selectSource(cx, base, sourceActor));
     const range = { start: 3, end: 5, sourceId: "2" };
     dispatch(actions.highlightLineRange(range));
     dispatch(actions.clearHighlightLineRange());
-    expect(getHighlightedLineRange(getState())).toEqual(null);
+    expect(getHighlightedLineRangeForSelectedSource(getState())).toEqual(null);
   });
 });

devtools/client/debugger/src/components/Editor/HighlightLines.js

@@ -4,14 +4,12 @@
 
 import { Component } from "react";
 import PropTypes from "prop-types";
-import { connect } from "../../utils/connect";
-import { getHighlightedLineRange } from "../../selectors";
 
 class HighlightLines extends Component {
   static get propTypes() {
     return {
       editor: PropTypes.object.isRequired,
-      highlightedLineRange: PropTypes.object,
+      range: PropTypes.object.isRequired,
     };
   }
 
@@ -33,15 +31,15 @@ class HighlightLines extends Component {
   }
 
   clearHighlightRange() {
-    const { highlightedLineRange, editor } = this.props;
+    const { range, editor } = this.props;
 
     const { codeMirror } = editor;
 
-    if (!highlightedLineRange || !codeMirror) {
+    if (!range || !codeMirror) {
       return;
     }
 
-    const { start, end } = highlightedLineRange;
+    const { start, end } = range;
     codeMirror.operation(() => {
       for (let line = start - 1; line < end; line++) {
         codeMirror.removeLineClass(line, "wrap", "highlight-lines");
@@ -50,15 +48,15 @@ class HighlightLines extends Component {
   }
 
   highlightLineRange = () => {
-    const { highlightedLineRange, editor } = this.props;
+    const { range, editor } = this.props;
 
     const { codeMirror } = editor;
 
-    if (!highlightedLineRange || !codeMirror) {
+    if (!range || !codeMirror) {
       return;
     }
 
-    const { start, end } = highlightedLineRange;
+    const { start, end } = range;
 
     codeMirror.operation(() => {
       editor.alignLine(start);
@@ -73,6 +71,4 @@ class HighlightLines extends Component {
   }
 }
 
-export default connect(state => ({
-  highlightedLineRange: getHighlightedLineRange(state),
-}))(HighlightLines);
+export default HighlightLines;

devtools/client/debugger/src/components/Editor/index.js

@@ -42,6 +42,7 @@ import {
   getHighlightedCalls,
   getBlackBoxRanges,
   isSourceBlackBoxed,
+  getHighlightedLineRangeForSelectedSource,
 } from "../../selectors";
 
 // Redux actions
@@ -139,6 +140,7 @@ class Editor extends PureComponent {
       skipPausing: PropTypes.bool.isRequired,
       blackboxedRanges: PropTypes.object.isRequired,
       breakableLines: PropTypes.object.isRequired,
+      highlightedLineRange: PropTypes.object,
     };
   }
 
@@ -147,7 +149,6 @@ class Editor extends PureComponent {
     super(props);
 
     this.state = {
-      highlightedLineRange: null,
       editor: null,
       contextMenu: null,
     };
@@ -664,6 +665,7 @@ class Editor extends PureComponent {
       isPaused,
       inlinePreviewEnabled,
       editorWrappingEnabled,
+      highlightedLineRange,
     } = this.props;
     const { editor, contextMenu } = this.state;
 
@@ -679,7 +681,9 @@ class Editor extends PureComponent {
         <EmptyLines editor={editor} />
         <Breakpoints editor={editor} cx={cx} />
         <Preview editor={editor} editorRef={this.$editorWrapper} />
-        <HighlightLines editor={editor} />
+        {highlightedLineRange ? (
+          <HighlightLines editor={editor} range={highlightedLineRange} />
+        ) : null}
         {features.blackboxLines ? <BlackboxLines editor={editor} /> : null}
         <Exceptions />
         {
@@ -759,6 +763,7 @@ const mapStateToProps = state => {
     highlightedCalls: getHighlightedCalls(state, getCurrentThread(state)),
     blackboxedRanges: getBlackBoxRanges(state),
     breakableLines: getSelectedBreakableLines(state),
+    highlightedLineRange: getHighlightedLineRangeForSelectedSource(state),
   };
 };
 

devtools/client/debugger/src/selectors/ui.js

@@ -2,6 +2,8 @@
  * 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/>. */
 
+import { getSelectedSource } from "./sources";
+
 export function getSelectedPrimaryPaneTab(state) {
   return state.ui.selectedPrimaryPaneTab;
 }
@@ -22,8 +24,20 @@ export function getPaneCollapse(state, position) {
   return state.ui.endPanelCollapsed;
 }
 
-export function getHighlightedLineRange(state) {
-  return state.ui.highlightedLineRange;
+export function getHighlightedLineRangeForSelectedSource(state) {
+  const selectedSource = getSelectedSource(state);
+  if (!selectedSource) {
+    return null;
+  }
+  // Only return the highlighted line range if it matches the selected source
+  const highlightedLineRange = state.ui.highlightedLineRange;
+  if (
+    highlightedLineRange &&
+    selectedSource.id == highlightedLineRange.sourceId
+  ) {
+    return highlightedLineRange;
+  }
+  return null;
 }
 
 export function getConditionalPanelLocation(state) {