Bug 1546462 - Don't run search upon location change r=jlast
authorDavid Walsh <dwalsh@mozilla.com>
Fri, 26 Apr 2019 14:30:04 +0000
changeset 530304 51f7ce6207bbcb1ddca1f62acd82d55a69f7be41
parent 530303 e02e1b1425934e95738946a7b7602d341844c011
child 530305 fc4e5936eeae0683cb1c8db8d369100e07e1b37e
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1546462
milestone68.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 1546462 - Don't run search upon location change r=jlast Differential Revision: https://phabricator.services.mozilla.com/D28740
devtools/client/debugger/src/actions/file-search.js
devtools/client/debugger/src/actions/ui.js
devtools/client/debugger/src/utils/editor/source-search.js
devtools/client/debugger/test/mochitest/browser.ini
devtools/client/debugger/test/mochitest/browser_dbg-search-file-paused.js
--- a/devtools/client/debugger/src/actions/file-search.js
+++ b/devtools/client/debugger/src/actions/file-search.js
@@ -92,17 +92,22 @@ export function updateSearchResults(
       matches,
       matchIndex,
       count: matches.length,
       index: characterIndex
     }
   };
 }
 
-export function searchContents(cx: Context, query: string, editor: Object) {
+export function searchContents(
+  cx: Context,
+  query: string,
+  editor: Object,
+  focusFirstResult?: boolean = true
+) {
   return async ({ getState, dispatch }: ThunkArgs) => {
     const modifiers = getFileSearchModifiers(getState());
     const selectedSourceWithContent = getSelectedSourceWithContent(getState());
 
     if (
       !editor ||
       !selectedSourceWithContent ||
       !selectedSourceWithContent.content ||
@@ -126,17 +131,17 @@ export function searchContents(cx: Conte
     if (selectedContent.type === "wasm") {
       text = renderWasmText(selectedSource.id, selectedContent).join("\n");
     } else {
       text = selectedContent.value;
     }
 
     const matches = await getMatches(query, text, _modifiers);
 
-    const res = find(ctx, query, true, _modifiers);
+    const res = find(ctx, query, true, _modifiers, focusFirstResult);
     if (!res) {
       return;
     }
 
     const { ch, line } = res;
 
     dispatch(updateSearchResults(cx, ch, line, matches));
   };
--- a/devtools/client/debugger/src/actions/ui.js
+++ b/devtools/client/debugger/src/actions/ui.js
@@ -54,17 +54,17 @@ export function setActiveSearch(activeSe
 }
 
 export function updateActiveFileSearch(cx: Context) {
   return ({ dispatch, getState }: ThunkArgs) => {
     const isFileSearchOpen = getActiveSearch(getState()) === "file";
     const fileSearchQuery = getFileSearchQuery(getState());
     if (isFileSearchOpen && fileSearchQuery) {
       const editor = getEditor();
-      dispatch(searchContents(cx, fileSearchQuery, editor));
+      dispatch(searchContents(cx, fileSearchQuery, editor, false));
     }
   };
 }
 
 export function toggleFrameworkGrouping(toggleValue: boolean) {
   return ({ dispatch, getState }: ThunkArgs) => {
     dispatch({
       type: "TOGGLE_FRAMEWORK_GROUPING",
--- a/devtools/client/debugger/src/utils/editor/source-search.js
+++ b/devtools/client/debugger/src/utils/editor/source-search.js
@@ -127,18 +127,25 @@ export function getMatchIndex(
 /**
  * If there's a saved search, selects the next results.
  * Otherwise, creates a new search and selects the first
  * result.
  *
  * @memberof utils/source-search
  * @static
  */
-function doSearch(ctx, rev, query, keepSelection, modifiers: SearchModifiers) {
-  const { cm } = ctx;
+function doSearch(
+  ctx,
+  rev,
+  query,
+  keepSelection,
+  modifiers: SearchModifiers,
+  focusFirstResult?: boolean = true
+) {
+  const { cm, ed } = ctx;
   if (!cm) {
     return;
   }
   const defaultIndex = { line: -1, ch: -1 };
 
   return cm.operation(function() {
     if (!query || isWhitespace(query)) {
       clearSearch(cm, query);
@@ -148,16 +155,23 @@ function doSearch(ctx, rev, query, keepS
     const state = getSearchState(cm, query);
     const isNewQuery = state.query !== query;
     state.query = query;
 
     updateOverlay(cm, state, query, modifiers);
     updateCursor(cm, state, keepSelection);
     const searchLocation = searchNext(ctx, rev, query, isNewQuery, modifiers);
 
+    // We don't want to jump the editor
+    // when we're selecting text
+    if (!cm.state.selectingText && searchLocation && focusFirstResult) {
+      ed.alignLine(searchLocation.from.line, "center");
+      cm.setSelection(searchLocation.from, searchLocation.to);
+    }
+
     return searchLocation ? searchLocation.from : defaultIndex;
   });
 }
 
 export function searchSourceForHighlight(
   ctx: Object,
   rev: boolean,
   query: string,
@@ -192,17 +206,17 @@ function getCursorPos(newQuery, rev, sta
 
 /**
  * Selects the next result of a saved search.
  *
  * @memberof utils/source-search
  * @static
  */
 function searchNext(ctx, rev, query, newQuery, modifiers) {
-  const { cm, ed } = ctx;
+  const { cm } = ctx;
   let nextMatch;
   cm.operation(function() {
     const state = getSearchState(cm, query);
     const pos = getCursorPos(newQuery, rev, state);
 
     if (!state.query) {
       return;
     }
@@ -215,23 +229,16 @@ function searchNext(ctx, rev, query, new
 
     if (!cursor.find(rev) && state.query) {
       cursor = getSearchCursor(cm, state.query, location, modifiers);
       if (!cursor.find(rev)) {
         return;
       }
     }
 
-    // We don't want to jump the editor
-    // when we're selecting text
-    if (!cm.state.selectingText) {
-      ed.alignLine(cursor.from().line, "center");
-      cm.setSelection(cursor.from(), cursor.to());
-    }
-
     nextMatch = { from: cursor.from(), to: cursor.to() };
   });
 
   return nextMatch;
 }
 
 function findNextOnLine(ctx, rev, query, newQuery, modifiers, line, ch) {
   const { cm, ed } = ctx;
@@ -291,20 +298,28 @@ export function clearSearch(cm: any, que
  *
  * @memberof utils/source-search
  * @static
  */
 export function find(
   ctx: any,
   query: string,
   keepSelection: boolean,
-  modifiers: SearchModifiers
+  modifiers: SearchModifiers,
+  focusFirstResult?: boolean
 ) {
   clearSearch(ctx.cm, query);
-  return doSearch(ctx, false, query, keepSelection, modifiers);
+  return doSearch(
+    ctx,
+    false,
+    query,
+    keepSelection,
+    modifiers,
+    focusFirstResult
+  );
 }
 
 /**
  * Finds the next item based on the currently saved search.
  *
  * @memberof utils/source-search
  * @static
  */
--- a/devtools/client/debugger/test/mochitest/browser.ini
+++ b/devtools/client/debugger/test/mochitest/browser.ini
@@ -750,16 +750,18 @@ skip-if = os == "win"
 [browser_dbg-returnvalues.js]
 [browser_dbg-reload.js]
 [browser_dbg-reloading.js]
 skip-if = true
 [browser_dbg-pause-points.js]
 [browser_dbg-scopes-mutations.js]
 [browser_dbg-search-file.js]
 skip-if = os == "win" # Bug 1393121
+[browser_dbg-search-file-paused.js]
+skip-if = os == "win" # Bug 1393121
 [browser_dbg-quick-open.js]
 skip-if = os == "win"
 [browser_dbg-search-project.js]
 [browser_dbg-blackbox-original.js]
 [browser_dbg-sourcemaps.js]
 [browser_dbg-sourcemaps-breakpoints.js]
 [browser_dbg-sourcemaps-disabled.js]
 [browser_dbg-sourcemaps-reload.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-search-file-paused.js
@@ -0,0 +1,71 @@
+/* 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/>. */
+
+// Tests the search bar correctly responds to queries, enter, shift enter
+
+function waitForSearchState(dbg) {
+  return waitForState(dbg, () => getCM(dbg).state.search);
+}
+
+function getFocusedEl(dbg) {
+  const doc = dbg.win.document;
+  return doc.activeElement;
+}
+
+function pressMouseDown(dbg, node) {
+  EventUtils.sendMouseEvent({ type: "mousedown" }, node, dbg.win);
+}
+
+add_task(async function() {
+  const dbg = await initDebugger("doc-scripts.html", "simple1.js", "simple2.js");
+  const {
+    selectors: { getBreakpoints, getBreakpoint, getActiveSearch },
+    getState
+  } = dbg;
+
+  info("Add a breakpoint, wait for pause");
+  const source = findSource(dbg, "simple2.js");
+  await selectSource(dbg, source.url);
+  await addBreakpoint(dbg, source, 5);
+  invokeInTab("main");
+  await waitForPaused(dbg);
+
+  info("Starting a search for 'bar'");
+  const cm = getCM(dbg);
+  pressKey(dbg, "fileSearch");
+  is(dbg.selectors.getActiveSearch(), "file");
+  const el = getFocusedEl(dbg);
+  type(dbg, "bar");
+  await waitForSearchState(dbg);
+
+  info("Ensuring 'bar' matches are highlighted");
+  pressKey(dbg, "Enter");
+  is(cm.state.search.posFrom.line, 1);
+  pressKey(dbg, "Enter");
+  is(cm.state.search.posFrom.line, 4);
+
+  info("Switching files via frame click");
+  const frames = findAllElements(dbg, "frames");
+  pressMouseDown(dbg, frames[1])
+
+  // Ensure that the debug line is in view, and not the first "bar" instance,
+  // which the user would have to scroll down for
+  const { top } = cm.getScrollInfo();
+  is(top, 0, "First search term is not in view");
+
+  // Change the search term and go back to the first source in stack
+  info("Switching to paused file via frame click");
+  pressKey(dbg, "fileSearch");
+  el.value = "";
+  type(dbg, "func");
+  await waitForSearchState(dbg);
+  pressMouseDown(dbg, frames[0]);
+  await waitFor(() => cm.state.search.query === "func");
+
+  // Ensure there is a match for the new term
+  pressKey(dbg, "Enter");
+  is(cm.state.search.posFrom.line, 0);
+  pressKey(dbg, "Enter");
+  is(cm.state.search.posFrom.line, 1);
+});