Bug 1483880 - Always hide the autocompletion popup in acceptProposedCompletion; r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 22 Aug 2018 11:40:09 +0000
changeset 432789 28d5979962b7aacb307fbbaf1ce438d5560044ce
parent 432788 c2616aee4d444980703769f85056d48cd4f1483d
child 432790 61396b1eaa14477e196564ea0c4c0ba79439a64d
push id34487
push usernerli@mozilla.com
push dateWed, 22 Aug 2018 16:28:03 +0000
treeherdermozilla-central@61396b1eaa14 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1483880
milestone63.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 1483880 - Always hide the autocompletion popup in acceptProposedCompletion; r=Honza. When hitting enter, if there was no autocompletionText shown (because the input was matching a proposed value), then the autocompletion popup wasn't closed. We now close it every time, and only check the completionText to insert a possible string after the cursor. A test case is added to make sure we don't regress this. Differential Revision: https://phabricator.services.mozilla.com/D3763
devtools/client/webconsole/components/JSTerm.js
devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_return_key.js
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -1221,23 +1221,21 @@ class JSTerm extends Component {
       !completionText
       && this.autocompletePopup.isOpen
       && this.autocompletePopup.selectedItem
     ) {
       const {selectedItem} = this.autocompletePopup;
       completionText = selectedItem.label.substring(selectedItem.preLabel.length);
     }
 
-    if (!completionText) {
-      return false;
-    }
+    this.clearCompletion();
 
-    this.insertStringAtCursor(completionText);
-    this.clearCompletion();
-    return true;
+    if (completionText) {
+      this.insertStringAtCursor(completionText);
+    }
   }
 
   getInputValueBeforeCursor() {
     if (this.editor) {
       return this.editor.getDoc().getRange({line: 0, ch: 0}, this.editor.getCursor());
     }
 
     if (this.inputNode) {
--- a/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_return_key.js
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_return_key.js
@@ -1,29 +1,30 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-// See Bug 585991.
+// Test that the Enter keys works as expected. See Bug 585991 and 1483880.
 
 const TEST_URI = `data:text/html;charset=utf-8,
 <head>
   <script>
     /* Create a prototype-less object so popup does not contain native
      * Object prototype properties.
      */
     window.foobar = Object.create(null);
     Object.assign(window.foobar, {
       item0: "value0",
       item1: "value1",
       item2: "value2",
       item3: "value3",
+      item33: "value33",
     });
   </script>
 </head>
 <body>bug 585991 - test pressing return with open popup</body>`;
 
 add_task(async function() {
   // Run test with legacy JsTerm
   await pushPref("devtools.webconsole.jsterm.codeMirror", false);
@@ -32,46 +33,66 @@ add_task(async function() {
   await pushPref("devtools.webconsole.jsterm.codeMirror", true);
   await performTests();
 });
 
 async function performTests() {
   const { jsterm } = await openNewTabAndConsole(TEST_URI);
   const { autocompletePopup: popup } = jsterm;
 
-  const onPopUpOpen = popup.once("popup-opened");
+  let onPopUpOpen = popup.once("popup-opened");
 
   info("wait for completion suggestions: window.foobar.");
 
   jsterm.setInputValue("window.fooba");
   EventUtils.sendString("r.");
 
   await onPopUpOpen;
 
   ok(popup.isOpen, "popup is open");
 
   const expectedPopupItems = [
     "item0",
     "item1",
     "item2",
     "item3",
+    "item33",
   ];
   is(popup.itemCount, expectedPopupItems.length, "popup.itemCount is correct");
   is(popup.selectedIndex, 0, "First index from top is selected");
 
   EventUtils.synthesizeKey("KEY_ArrowUp");
 
-  is(popup.selectedIndex, 3, "index 3 is selected");
-  is(popup.selectedItem.label, "item3", "item3 is selected");
+  is(popup.selectedIndex, expectedPopupItems.length - 1, "last index is selected");
+  is(popup.selectedItem.label, "item33", "item33 is selected");
   const prefix = jsterm.getInputValue().replace(/[\S]/g, " ");
-  checkJsTermCompletionValue(jsterm, prefix + "item3", "completeNode.value holds item3");
+  checkJsTermCompletionValue(jsterm, prefix + "item33",
+    "completeNode.value holds item33");
 
   info("press Return to accept suggestion. wait for popup to hide");
-  const onPopupClose = popup.once("popup-closed");
+  let onPopupClose = popup.once("popup-closed");
   EventUtils.synthesizeKey("KEY_Enter");
 
   await onPopupClose;
 
   ok(!popup.isOpen, "popup is not open after KEY_Enter");
+  is(jsterm.getInputValue(), "window.foobar.item33",
+    "completion was successful after KEY_Enter");
+  ok(!getJsTermCompletionValue(jsterm), "completeNode is empty");
+
+  info("Test that hitting enter when the completeNode is empty closes the popup");
+  onPopUpOpen = popup.once("popup-opened");
+  info("wait for completion suggestions: window.foobar.item3");
+  jsterm.setInputValue("window.foobar.item");
+  EventUtils.sendString("3");
+  await onPopUpOpen;
+
+  is(popup.selectedItem.label, "item3", "item3 is selected");
+  ok(!getJsTermCompletionValue(jsterm), "completeNode is empty");
+
+  onPopupClose = popup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Enter");
+  await onPopupClose;
+
+  ok(!popup.isOpen, "popup is not open after KEY_Enter");
   is(jsterm.getInputValue(), "window.foobar.item3",
     "completion was successful after KEY_Enter");
-  ok(!getJsTermCompletionValue(jsterm), "completeNode is empty");
 }