[preview] jump to definition should respect the current context

assets/panel/panel.js

@@ -105,7 +105,7 @@ DebuggerPanel.prototype = {
   },
 
   selectSource(url, line) {
-    this._actions.selectSourceURL(url, { location: { line } });
+    this._actions.selectSourceURL(url, { line });
   },
 
   getSource(sourceURL) {

src/actions/pause/paused.js

@@ -3,14 +3,10 @@
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
-
-import { isGeneratedId } from "devtools-source-map";
-
 import {
   getHiddenBreakpointLocation,
   isEvaluatingExpression,
   getSelectedFrame,
-  getVisibleSelectedFrame,
   getSources
 } from "../../selectors";
 
@@ -73,15 +69,10 @@ export function paused(pauseInfo: Pause) {
     }
 
     await dispatch(mapFrames());
-    const selectedFrame = getSelectedFrame(getState());
 
+    const selectedFrame = getSelectedFrame(getState());
     if (selectedFrame) {
-      const visibleFrame = getVisibleSelectedFrame(getState());
-      const location =
-        visibleFrame && isGeneratedId(visibleFrame.location.sourceId)
-          ? selectedFrame.generatedLocation
-          : selectedFrame.location;
-      await dispatch(selectLocation(location));
+      await dispatch(selectLocation(selectedFrame.location));
     }
 
     dispatch(togglePaneCollapse("end", false));

src/actions/pause/tests/pause.spec.js

@@ -230,7 +230,8 @@ describe("pause", () => {
         getOriginalSourceText: async () => ({
           source: "\n\nfunction fooOriginal() {\n  return -5;\n}",
           contentType: "text/javascript"
-        })
+        }),
+        getGeneratedLocation: async location => location
       };
 
       const store = createStore(mockThreadClient, {}, sourceMapsMock);

src/actions/sources/loadSourceText.js

@@ -71,7 +71,6 @@ export function loadSourceText(source: Source) {
     }
 
     const newSource = getSourceFromId(getState(), source.id);
-
     if (isOriginalId(newSource.id) && !newSource.isWasm) {
       const generatedSource = getGeneratedSource(getState(), source);
       await dispatch(loadSourceText(generatedSource));

src/actions/sources/prettyPrint.js

@@ -13,8 +13,8 @@ import { prettyPrint } from "../../workers/pretty-print";
 import { setSource } from "../../workers/parser";
 import { getPrettySourceURL, isLoaded } from "../../utils/source";
 import { loadSourceText } from "./loadSourceText";
-import { selectLocation } from "../sources";
 import { mapFrames } from "../pause";
+import { selectSpecificLocation } from "../sources";
 
 import {
   getSource,
@@ -106,7 +106,7 @@ export function togglePrettyPrint(sourceId: string) {
     if (prettySource) {
       const _sourceId = prettySource.id;
       return dispatch(
-        selectLocation({ ...options.location, sourceId: _sourceId })
+        selectSpecificLocation({ ...options.location, sourceId: _sourceId })
       );
     }
 
@@ -118,7 +118,10 @@ export function togglePrettyPrint(sourceId: string) {
     await dispatch(setSymbols(newPrettySource.id));
 
     dispatch(
-      selectLocation({ ...options.location, sourceId: newPrettySource.id })
+      selectSpecificLocation({
+        ...options.location,
+        sourceId: newPrettySource.id
+      })
     );
 
     return newPrettySource;

src/actions/sources/select.js

@@ -11,6 +11,7 @@
 
 import { isOriginalId } from "devtools-source-map";
 
+import { getSourceFromId } from "../../reducers/sources";
 import { setOutOfScopeLocations, setSymbols } from "../ast";
 import { closeActiveSearch, updateActiveFileSearch } from "../ui";
 
@@ -21,7 +22,7 @@ import { loadSourceText } from "./loadSourceText";
 import { prefs } from "../../utils/prefs";
 import { shouldPrettyPrint, isMinified } from "../../utils/source";
 import { createLocation } from "../../utils/location";
-import { getGeneratedLocation } from "../../utils/source-maps";
+import { getMappedLocation } from "../../utils/source-maps";
 
 import {
   getSource,
@@ -32,14 +33,9 @@ import {
   getSelectedSource
 } from "../../selectors";
 
-import type { Location, Source } from "../../types";
+import type { Location, Position, Source } from "../../types";
 import type { ThunkArgs } from "../types";
 
-declare type SelectSourceOptions = {
-  tabIndex?: number,
-  location?: { line: number, column?: ?number }
-};
-
 export const setSelectedLocation = (source: Source, location: Location) => ({
   type: "SET_SELECTED_LOCATION",
   source,
@@ -59,25 +55,25 @@ export const clearSelectedLocation = () => ({
 /**
  * Deterministically select a source that has a given URL. This will
  * work regardless of the connection status or if the source exists
- * yet. This exists mostly for external things to interact with the
+ * yet.
+ *
+ * This exists mostly for external things to interact with the
  * debugger.
  *
  * @memberof actions/sources
  * @static
  */
-export function selectSourceURL(
-  url: string,
-  options: SelectSourceOptions = {}
-) {
-  return async ({ dispatch, getState }: ThunkArgs) => {
+
+export function selectSourceURL(url: string, options: Position = { line: 1 }) {
+  return async ({ dispatch, getState, sourceMaps }: ThunkArgs) => {
     const source = getSourceByURL(getState(), url);
-    if (source) {
-      const sourceId = source.id;
-      const location = createLocation({ ...options.location, sourceId });
-      await dispatch(selectLocation(location));
-    } else {
-      dispatch(setPendingSelectedLocation(url, options));
+    if (!source) {
+      return dispatch(setPendingSelectedLocation(url, options));
     }
+
+    const sourceId = source.id;
+    const location = createLocation({ ...options, sourceId });
+    return dispatch(selectLocation(location));
   };
 }
 
@@ -98,9 +94,9 @@ export function selectSource(sourceId: string) {
  */
 export function selectLocation(
   location: Location,
-  { checkPrettyPrint = true }: Object = {}
+  { keepContext = true }: Object = {}
 ) {
-  return async ({ dispatch, getState, client }: ThunkArgs) => {
+  return async ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => {
     const currentSource = getSelectedSource(getState());
 
     if (!client) {
@@ -109,7 +105,7 @@ export function selectLocation(
       return;
     }
 
-    const source = getSource(getState(), location.sourceId);
+    let source = getSource(getState(), location.sourceId);
     if (!source) {
       // If there is no source we deselect the current selected source
       return dispatch(clearSelectedLocation());
@@ -120,6 +116,18 @@ export function selectLocation(
       dispatch(closeActiveSearch());
     }
 
+    // Preserve the current source map context (original / generated)
+    // when navigting to a new location.
+    const selectedSource = getSelectedSource(getState());
+    if (
+      keepContext &&
+      selectedSource &&
+      isOriginalId(selectedSource.id) != isOriginalId(location.sourceId)
+    ) {
+      location = await getMappedLocation(getState(), sourceMaps, location);
+      source = getSourceFromId(getState(), location.sourceId);
+    }
+
     dispatch(addTab(source.url, 0));
     dispatch(setSelectedLocation(source, location));
 
@@ -132,7 +140,7 @@ export function selectLocation(
     }
 
     if (
-      checkPrettyPrint &&
+      keepContext &&
       prefs.autoPrettyPrint &&
       !getPrettySource(getState(), loadedSource.id) &&
       shouldPrettyPrint(loadedSource) &&
@@ -158,7 +166,7 @@ export function selectLocation(
  * @static
  */
 export function selectSpecificLocation(location: Location) {
-  return selectLocation(location, { checkPrettyPrint: false });
+  return selectLocation(location, { keepContext: false });
 }
 
 /**
@@ -182,20 +190,13 @@ export function jumpToMappedLocation(location: Location) {
       return;
     }
 
-    const source = getSource(getState(), location.sourceId);
-    let pairedLocation;
-    if (isOriginalId(location.sourceId)) {
-      pairedLocation = await getGeneratedLocation(
-        getState(),
-        source,
-        location,
-        sourceMaps
-      );
-    } else {
-      pairedLocation = await sourceMaps.getOriginalLocation(location, source);
-    }
+    const pairedLocation = await getMappedLocation(
+      getState(),
+      sourceMaps,
+      location
+    );
 
-    return dispatch(selectLocation({ ...pairedLocation }));
+    return dispatch(selectSpecificLocation({ ...pairedLocation }));
   };
 }
 

src/actions/sources/tests/select.spec.js

@@ -2,13 +2,15 @@
  * 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 { getSymbols } from "../../../reducers/ast";
 import {
   actions,
   selectors,
   createStore,
   makeFrame,
   makeSource,
-  waitForState
+  waitForState,
+  makeOriginalSource
 } from "../../../utils/test-head";
 const {
   getSource,
@@ -58,16 +60,6 @@ describe("sources", () => {
     expect(locations).toHaveLength(1);
   });
 
-  it("should automatically select a pending source", async () => {
-    const { dispatch, getState } = createStore(sourceThreadClient);
-    const baseSource = makeSource("base.js");
-    await dispatch(actions.selectSourceURL(baseSource.url));
-
-    expect(getSelectedSource(getState())).toBe(undefined);
-    await dispatch(actions.newSource(baseSource));
-    expect(getSelectedSource(getState()).url).toBe(baseSource.url);
-  });
-
   it("should open a tab for the source", async () => {
     const { dispatch, getState } = createStore(sourceThreadClient);
     await dispatch(actions.newSource(makeSource("foo.js")));
@@ -150,4 +142,73 @@ describe("sources", () => {
     const clearResult = getState().sources.pendingSelectedLocation;
     expect(clearResult).toEqual({ url: "" });
   });
+
+  it("should keep the generated the viewing context", async () => {
+    const store = createStore(sourceThreadClient);
+    const { dispatch, getState } = store;
+    const baseSource = makeSource("base.js");
+    await dispatch(actions.newSource(baseSource));
+
+    await dispatch(
+      actions.selectLocation({ sourceId: baseSource.id, line: 1 })
+    );
+
+    expect(getSelectedSource(getState()).id).toBe(baseSource.id);
+    await waitForState(store, state => getSymbols(state, baseSource));
+  });
+
+  it("should keep the original the viewing context", async () => {
+    const { dispatch, getState } = createStore(
+      sourceThreadClient,
+      {},
+      {
+        getOriginalLocation: async location => ({ ...location, line: 12 }),
+        getGeneratedLocation: async location => ({ ...location, line: 12 }),
+        getOriginalSourceText: async () => ({ source: "" })
+      }
+    );
+
+    const baseSource = makeSource("base.js");
+    await dispatch(actions.newSource(baseSource));
+
+    const originalBaseSource = makeOriginalSource("base.js");
+    await dispatch(actions.newSource(originalBaseSource));
+
+    await dispatch(actions.selectSource(originalBaseSource.id));
+
+    const fooSource = makeSource("foo.js");
+    await dispatch(actions.newSource(fooSource));
+    await dispatch(actions.selectLocation({ sourceId: fooSource.id, line: 1 }));
+
+    expect(getSelectedLocation(getState()).line).toBe(12);
+  });
+
+  it("should change the original the viewing context", async () => {
+    const { dispatch, getState } = createStore(
+      sourceThreadClient,
+      {},
+      { getOriginalLocation: async location => ({ ...location, line: 12 }) }
+    );
+
+    const baseSource = makeOriginalSource("base.js");
+    await dispatch(actions.newSource(baseSource));
+    await dispatch(actions.selectSource(baseSource.id));
+
+    await dispatch(
+      actions.selectSpecificLocation({ sourceId: baseSource.id, line: 1 })
+    );
+    expect(getSelectedLocation(getState()).line).toBe(1);
+  });
+
+  describe("selectSourceURL", () => {
+    it("should automatically select a pending source", async () => {
+      const { dispatch, getState } = createStore(sourceThreadClient);
+      const baseSource = makeSource("base.js");
+      await dispatch(actions.selectSourceURL(baseSource.url));
+
+      expect(getSelectedSource(getState())).toBe(undefined);
+      await dispatch(actions.newSource(baseSource));
+      expect(getSelectedSource(getState()).url).toBe(baseSource.url);
+    });
+  });
 });

src/components/PrimaryPanes/index.js

@@ -33,7 +33,6 @@ type Props = {
   horizontal: boolean,
   sourceSearchOn: boolean,
   setPrimaryPaneTab: string => void,
-  selectLocation: Object => void,
   setActiveSearch: string => void,
   closeActiveSearch: () => void
 };
@@ -126,7 +125,6 @@ export default connect(
   mapStateToProps,
   {
     setPrimaryPaneTab: actions.setPrimaryPaneTab,
-    selectLocation: actions.selectLocation,
     setActiveSearch: actions.setActiveSearch,
     closeActiveSearch: actions.closeActiveSearch
   }

src/components/PrimaryPanes/tests/SourcesTree.spec.js

@@ -391,7 +391,6 @@ function generateDefaults(overrides) {
   };
   return {
     autoExpandAll: true,
-    selectLocation: jest.fn(),
     selectSource: jest.fn(),
     setExpandedState: jest.fn(),
     sources: defaultSources,