Bug 1488662 Change go to line shortcut to Ctrl+G r=davidwalsh
authorchujun <chujunlu@hotmail.com>
Tue, 04 Jun 2019 01:25:36 +0000
changeset 476995 812f70d7ccae4fcde784ac2183f23bfddd8943ea
parent 476994 3693f44248096b69909ce96879e25e0d5300ebde
child 476996 2dacf7539a2f6238bc9731fddc5eac8460fb9afc
push id87172
push userdwalsh@mozilla.com
push dateWed, 05 Jun 2019 14:54:22 +0000
treeherderautoland@812f70d7ccae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidwalsh
bugs1488662
milestone69.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 1488662 Change go to line shortcut to Ctrl+G r=davidwalsh The current “go to line” keyboard shortcut `CmdOrCtrl+;` conflicts with the “step in” shortcut. The "go to line" shortcut is replaced with the shortcut for Chrome debugger and Sublime Text: `Ctrl+G`. Upper or lower cases don’t matter. 1. Current behavior: 1a) With no file open in editor; or with a file open in editor, but not pausing at a breakpoint: `Cmd+;` or `Cmd+Shift+;` opens the “go to line” box. 1b) Pause at a breakpoint: `Cmd+;` runs `step in`, and also opens the “go to line” box; `Cmd+Shift+;` runs `step over`, and also opens the “go to line” box. 2) Code changes: 2a) Create gotoLineModal.key3. 2b) Update localization note. 2b) Update snapshot test. 3) After the code changes, on macOS: 3a) With no file open in editor: `Ctrl+G` opens the “go to line” box. 3b) With a file open, but not pausing at breakpoints: `Ctrl+G`opens the “go to line” box; hitting `Ctrl+G` again or `escape` closes the box. 3c) Pause at a breakpoint: `Cmd+;` or `Cmd+Shift+;` doesn’t open the “go to line” box. `Ctrl+G` opens the “go to line” box. Differential Revision: https://phabricator.services.mozilla.com/D26256
devtools/client/debugger/src/components/App.js
devtools/client/debugger/src/components/Editor/SearchBar.js
devtools/client/debugger/src/components/ShortcutsModal.js
devtools/client/debugger/src/components/test/__snapshots__/ShortcutsModal.spec.js.snap
devtools/client/debugger/test/mochitest/browser.ini
devtools/client/debugger/test/mochitest/browser_dbg-go-to-line.js
devtools/client/debugger/test/mochitest/browser_dbg-search-file.js
devtools/client/debugger/test/mochitest/helpers.js
devtools/client/locales/en-US/debugger.properties
--- a/devtools/client/debugger/src/components/App.js
+++ b/devtools/client/debugger/src/components/App.js
@@ -114,17 +114,17 @@ class App extends Component<Props, State
     );
 
     const searchKeys = [
       L10N.getStr("sources.search.key2"),
       L10N.getStr("sources.search.alt.key"),
     ];
     searchKeys.forEach(key => shortcuts.on(key, this.toggleQuickOpenModal));
 
-    shortcuts.on(L10N.getStr("gotoLineModal.key2"), (_, e) =>
+    shortcuts.on(L10N.getStr("gotoLineModal.key3"), (_, e) =>
       this.toggleQuickOpenModal(_, e, ":")
     );
 
     shortcuts.on("Escape", this.onEscape);
     shortcuts.on("Cmd+/", this.onCommandSlash);
   }
 
   componentWillUnmount() {
@@ -136,17 +136,17 @@ class App extends Component<Props, State
     );
 
     const searchKeys = [
       L10N.getStr("sources.search.key2"),
       L10N.getStr("sources.search.alt.key"),
     ];
     searchKeys.forEach(key => shortcuts.off(key, this.toggleQuickOpenModal));
 
-    shortcuts.off(L10N.getStr("gotoLineModal.key2"), this.toggleQuickOpenModal);
+    shortcuts.off(L10N.getStr("gotoLineModal.key3"), this.toggleQuickOpenModal);
 
     shortcuts.off("Escape", this.onEscape);
   }
 
   onEscape = (_, e) => {
     const {
       activeSearch,
       closeActiveSearch,
--- a/devtools/client/debugger/src/components/Editor/SearchBar.js
+++ b/devtools/client/debugger/src/components/Editor/SearchBar.js
@@ -32,18 +32,18 @@ import type { Modifiers, SearchResults }
 
 import SearchInput from "../shared/SearchInput";
 import { debounce } from "lodash";
 import "./SearchBar.css";
 
 import type SourceEditor from "../../utils/editor/source-editor";
 
 function getShortcuts() {
-  const searchAgainKey = L10N.getStr("sourceSearch.search.again.key2");
-  const searchAgainPrevKey = L10N.getStr("sourceSearch.search.againPrev.key2");
+  const searchAgainKey = L10N.getStr("sourceSearch.search.again.key3");
+  const searchAgainPrevKey = L10N.getStr("sourceSearch.search.againPrev.key3");
   const searchKey = L10N.getStr("sourceSearch.search.key2");
 
   return {
     shiftSearchAgainShortcut: searchAgainPrevKey,
     searchAgainShortcut: searchAgainKey,
     searchShortcut: searchKey,
   };
 }
--- a/devtools/client/debugger/src/components/ShortcutsModal.js
+++ b/devtools/client/debugger/src/components/ShortcutsModal.js
@@ -75,29 +75,29 @@ export class ShortcutsModal extends Comp
     return (
       <ul className="shortcuts-list">
         {this.renderShorcutItem(
           L10N.getStr("shortcuts.fileSearch2"),
           formatKeyShortcut(L10N.getStr("sources.search.key2"))
         )}
         {this.renderShorcutItem(
           L10N.getStr("shortcuts.searchAgain2"),
-          formatKeyShortcut(L10N.getStr("sourceSearch.search.again.key2"))
+          formatKeyShortcut(L10N.getStr("sourceSearch.search.again.key3"))
         )}
         {this.renderShorcutItem(
           L10N.getStr("shortcuts.projectSearch2"),
           formatKeyShortcut(L10N.getStr("projectTextSearch.key"))
         )}
         {this.renderShorcutItem(
           L10N.getStr("shortcuts.functionSearch2"),
           formatKeyShortcut(L10N.getStr("functionSearch.key"))
         )}
         {this.renderShorcutItem(
           L10N.getStr("shortcuts.gotoLine"),
-          formatKeyShortcut(L10N.getStr("gotoLineModal.key2"))
+          formatKeyShortcut(L10N.getStr("gotoLineModal.key3"))
         )}
       </ul>
     );
   }
 
   renderShortcutsContent() {
     return (
       <div
--- a/devtools/client/debugger/src/components/test/__snapshots__/ShortcutsModal.spec.js.snap
+++ b/devtools/client/debugger/src/components/test/__snapshots__/ShortcutsModal.spec.js.snap
@@ -146,19 +146,19 @@ exports[`ShortcutsModal renders when ena
         </li>
         <li>
           <span>
             Find next
           </span>
           <span>
             <span
               className="keystroke"
-              key="Ctrl+G"
+              key="Cmd+G"
             >
-              Ctrl+G
+              Cmd+G
             </span>
           </span>
         </li>
         <li>
           <span>
             Find in files
           </span>
           <span>
@@ -185,19 +185,19 @@ exports[`ShortcutsModal renders when ena
         </li>
         <li>
           <span>
             Go to line
           </span>
           <span>
             <span
               className="keystroke"
-              key="Ctrl+;"
+              key="Ctrl+G"
             >
-              Ctrl+;
+              Ctrl+G
             </span>
           </span>
         </li>
       </ul>
     </div>
   </div>
 </Slide>
 `;
--- a/devtools/client/debugger/test/mochitest/browser.ini
+++ b/devtools/client/debugger/test/mochitest/browser.ini
@@ -711,16 +711,17 @@ skip-if = (os == "win" && ccov) # Bug 14
 [browser_dbg-debugger-buttons.js]
 [browser_dbg-editor-gutter.js]
 [browser_dbg-editor-select.js]
 [browser_dbg-editor-highlight.js]
 [browser_dbg-ember-quickstart.js]
 [browser_dbg-expressions.js]
 [browser_dbg-expressions-error.js]
 [browser_dbg-expressions-focus.js]
+[browser_dbg-go-to-line.js]
 [browser_dbg-html-breakpoints.js]
 [browser_dbg-iframes.js]
 [browser_dbg-inline-cache.js]
 skip-if = ccov && os == 'win' # Bug 1443132
 [browser_dbg-inspector-integration.js]
 [browser_dbg-keyboard-navigation.js]
 [browser_dbg-keyboard-shortcuts.js]
 skip-if = os == "linux" # bug 1351952
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-go-to-line.js
@@ -0,0 +1,60 @@
+/* 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/>. */
+
+// Test the "go to line" feature correctly responses to keyboard shortcuts.
+
+function assertEnabled(dbg) {
+  is(dbg.selectors.getQuickOpenEnabled(), true, "quickOpen enabled");
+}
+
+function assertDisabled(dbg) {
+  is(dbg.selectors.getQuickOpenEnabled(), false, "quickOpen disabled");
+}
+
+async function waitForGoToLineBoxFocus(dbg) {
+  await waitFor(() => dbg.win.document.activeElement.tagName === "INPUT");
+}
+
+function assertLine(dbg, lineNumber) {
+  is(
+    dbg.selectors.getSelectedLocation().line,
+    lineNumber,
+    `goto line is ${lineNumber}`
+  );
+}
+
+add_task(async function() {
+  const dbg = await initDebugger("doc-scripts.html", "long.js");
+  await selectSource(dbg, "long");
+  await waitForSelectedSource(dbg, "long");
+
+  info('Test opening');
+  pressKey(dbg, "goToLine");
+  assertEnabled(dbg);
+  is(
+    dbg.win.document.activeElement.tagName,
+    "INPUT",
+    "The input area of 'go to line' box is focused"
+  );
+
+  info('Test closing by the same keyboard shortcut');
+  pressKey(dbg, "goToLine");
+  assertDisabled(dbg);
+  is(findElement(dbg, "searchField"), null, "The 'go to line' box is closed");
+
+  info('Test closing by escape');
+  pressKey(dbg, "goToLine");
+  assertEnabled(dbg);
+
+  pressKey(dbg, "Escape");
+  assertDisabled(dbg);
+  is(findElement(dbg, "searchField"), null, "The 'go to line' box is closed");
+
+  info('Test going to the correct line');
+  pressKey(dbg, "goToLine");
+  await waitForGoToLineBoxFocus(dbg);
+  type(dbg, "66");
+  pressKey(dbg, "Enter");
+  assertLine(dbg, 66);
+});
--- a/devtools/client/debugger/test/mochitest/browser_dbg-search-file.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-search-file.js
@@ -1,14 +1,16 @@
 /* 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
 
+const isMacOS = AppConstants.platform === "macosx";
+
 function waitForSearchState(dbg) {
   return waitForState(dbg, () => getCM(dbg).state.search);
 }
 
 function getFocusedEl(dbg) {
   const doc = dbg.win.document;
   return doc.activeElement;
 }
@@ -44,22 +46,25 @@ add_task(async function() {
   is(state.posFrom.line, 3);
 
   pressKey(dbg, "Enter");
   is(state.posFrom.line, 4);
 
   pressKey(dbg, "ShiftEnter");
   is(state.posFrom.line, 3);
 
-  pressKey(dbg, "fileSearchNext");
-  is(state.posFrom.line, 4);
+  if (isMacOS) {
+    info('cmd+G and cmdShift+G shortcut for traversing results only work for macOS');
+    pressKey(dbg, "fileSearchNext");
+    is(state.posFrom.line, 4);
 
-  pressKey(dbg, "fileSearchPrev");
-  is(state.posFrom.line, 3);
-
+    pressKey(dbg, "fileSearchPrev");
+    is(state.posFrom.line, 3);
+  }
+  
   pressKey(dbg, "fileSearch");
   type(dbg, "fun");
 
   pressKey(dbg, "Enter");
   is(state.posFrom.line, 4);
 
   // selecting another source keeps search open
   await selectSource(dbg, "simple2");
--- a/devtools/client/debugger/test/mochitest/helpers.js
+++ b/devtools/client/debugger/test/mochitest/helpers.js
@@ -1029,18 +1029,19 @@ const startKey = isMac
 const keyMappings = {
   close: { code: "w", modifiers: cmdOrCtrl },
   debugger: { code: "s", modifiers: shiftOrAlt },
   inspector: { code: "c", modifiers: shiftOrAlt },
   quickOpen: { code: "p", modifiers: cmdOrCtrl },
   quickOpenFunc: { code: "o", modifiers: cmdShift },
   quickOpenLine: { code: ":", modifiers: cmdOrCtrl },
   fileSearch: { code: "f", modifiers: cmdOrCtrl },
-  fileSearchNext: { code: "g", modifiers: cmdOrCtrl },
+  fileSearchNext: { code: "g", modifiers: { metaKey: true } },
   fileSearchPrev: { code: "g", modifiers: cmdShift },
+  goToLine: { code: "g", modifiers: { ctrlKey: true } },
   Enter: { code: "VK_RETURN" },
   ShiftEnter: { code: "VK_RETURN", modifiers: shiftOrAlt },
   AltEnter: {
     code: "VK_RETURN",
     modifiers: { altKey: true }
   },
   Up: { code: "VK_UP" },
   Down: { code: "VK_DOWN" },
--- a/devtools/client/locales/en-US/debugger.properties
+++ b/devtools/client/locales/en-US/debugger.properties
@@ -300,27 +300,27 @@ sourceSearch.search.key2=CmdOrCtrl+F
 # LOCALIZATION NOTE (sourceSearch.search.placeholder): placeholder text in
 # the source search input bar
 sourceSearch.search.placeholder=Search in file…
 
 # LOCALIZATION NOTE (sourceSearch.search.placeholder2): placeholder text in
 # the source search input bar
 sourceSearch.search.placeholder2=Find in file…
 
-# LOCALIZATION NOTE (sourceSearch.search.again.key2): Key shortcut to highlight
+# LOCALIZATION NOTE (sourceSearch.search.again.key3): Key shortcut to highlight
 # the next occurrence of the last search triggered from a source search
-# Do not localize "CmdOrCtrl+G", or change the format of the string. These are
+# Do not localize "Cmd+G", or change the format of the string. These are
 # key identifiers, not messages displayed to the user.
-sourceSearch.search.again.key2=CmdOrCtrl+G
+sourceSearch.search.again.key3=Cmd+G
 
-# LOCALIZATION NOTE (sourceSearch.search.againPrev.key2): Key shortcut to highlight
+# LOCALIZATION NOTE (sourceSearch.search.againPrev.key3): Key shortcut to highlight
 # the previous occurrence of the last search triggered from a source search
-# Do not localize "CmdOrCtrl+Shift+G", or change the format of the string. These are
+# Do not localize "Cmd+Shift+G", or change the format of the string. These are
 # key identifiers, not messages displayed to the user.
-sourceSearch.search.againPrev.key2=CmdOrCtrl+Shift+G
+sourceSearch.search.againPrev.key3=Cmd+Shift+G
 
 # LOCALIZATION NOTE (sourceSearch.resultsSummary1): Shows a summary of
 # the number of matches for autocomplete
 sourceSearch.resultsSummary1=%d results
 
 # LOCALIZATION NOTE (noMatchingStringsText): The text to display in the
 # global search results when there are no matching strings after filtering.
 noMatchingStringsText=No matches found
@@ -914,21 +914,21 @@ functionSearchSeparatorLabel=←
 # LOCALIZATION NOTE(gotoLineModal.placeholder): The placeholder
 # text displayed when the user searches for specific lines in a file
 gotoLineModal.placeholder=Go to line…
 
 # LOCALIZATION NOTE(gotoLineModal.title): The message shown to users
 # to open the go to line modal
 gotoLineModal.title=Go to a line number in a file
 
-# LOCALIZATION NOTE(gotoLineModal.key2): The shortcut for opening the
+# LOCALIZATION NOTE(gotoLineModal.key3): The shortcut for opening the
 # go to line modal
-# Do not localize "CmdOrCtrl+;", or change the format of the string. These are
+# Do not localize "Ctrl+G", or change the format of the string. These are
 # key identifiers, not messages displayed to the user.
-gotoLineModal.key2=CmdOrCtrl+;
+gotoLineModal.key3=Ctrl+G
 
 # LOCALIZATION NOTE(symbolSearch.search.functionsPlaceholder): The placeholder
 # text displayed when the user searches for functions in a file
 symbolSearch.search.functionsPlaceholder=Search functions…
 symbolSearch.search.functionsPlaceholder.title=Search for a function in a file
 
 # LOCALIZATION NOTE(symbolSearch.search.variablesPlaceholder): The placeholder
 # text displayed when the user searches for variables in a file