Bug 672733 - Web console autocomplete search should be case insensitive.r=ochameau,nchevobbe draft
authorsole <sole@mozilla.com>
Thu, 06 Apr 2017 13:23:13 +0100
changeset 556978 6490374a002781501d9c72a3ef1099751d32a91b
parent 556946 3c68d659c2b715f811708f043a1e7169d77be2ba
child 622983 5ceecae971a19b98b01224f79280e14eda5872fe
push id52646
push userbmo:sole@mozilla.com
push dateThu, 06 Apr 2017 12:23:38 +0000
reviewersochameau, nchevobbe
bugs672733
milestone55.0a1
Bug 672733 - Web console autocomplete search should be case insensitive.r=ochameau,nchevobbe MozReview-Commit-ID: 4UY7KGCa6IU
devtools/client/shared/autocomplete-popup.js
devtools/client/webconsole/jsterm.js
devtools/client/webconsole/test/browser_webconsole_bug_585991_autocomplete_keys.js
devtools/server/actors/webconsole.js
--- a/devtools/client/shared/autocomplete-popup.js
+++ b/devtools/client/shared/autocomplete-popup.js
@@ -14,17 +14,17 @@ const {PrefObserver} = require("devtools
 let itemIdCounter = 0;
 /**
  * Autocomplete popup UI implementation.
  *
  * @constructor
  * @param {Document} toolboxDoc
  *        The toolbox document to attach the autocomplete popup panel.
  * @param {Object} options
- *        An object consiting any of the following options:
+ *        An object consisting of any of the following options:
  *        - listId {String} The id for the list <LI> element.
  *        - position {String} The position for the tooltip ("top" or "bottom").
  *        - theme {String} String related to the theme of the popup
  *        - autoSelect {Boolean} Boolean to allow the first entry of the popup
  *          panel to be automatically selected when the popup shows.
  *        - onSelect {String} Callback called when the selected index is updated.
  *        - onClick {String} Callback called when the autocomplete popup receives a click
  *          event. The selectedIndex will already be updated if need be.
--- a/devtools/client/webconsole/jsterm.js
+++ b/devtools/client/webconsole/jsterm.js
@@ -1496,18 +1496,20 @@ JSTerm.prototype = {
       // Find the last non-alphanumeric other than _ or $ if it exists.
       let lastNonAlpha = input.match(/[^a-zA-Z0-9_$][a-zA-Z0-9_$]*$/);
       // If input contains non-alphanumerics, use the part after the last one
       // to filter the cache
       if (lastNonAlpha) {
         filterBy = input.substring(input.lastIndexOf(lastNonAlpha) + 1);
       }
 
+      // Convert filter to lower case only once
+      let filterByLc = filterBy.toLowerCase();
       let newList = cache.sort().filter(function (l) {
-        return l.startsWith(filterBy);
+        return l.toLowerCase().startsWith(filterByLc);
       });
 
       this.lastCompletion = {
         requestId: null,
         completionType: type,
         value: null,
       };
 
@@ -1566,17 +1568,21 @@ JSTerm.prototype = {
     if (!matches.length) {
       this.clearCompletion();
       callback && callback(this);
       this.emit("autocomplete-updated");
       return;
     }
 
     let items = matches.reverse().map(function (match) {
-      return { preLabel: lastPart, label: match };
+      /* NOTE that we're not adding a `preLabel` because if you add a
+       * preLabel the pop up will use it as label content instead of
+       * the label itself
+       */
+      return { label: match };
     });
 
     let popup = this.autocompletePopup;
     popup.setItems(items);
 
     let completionType = this.lastCompletion.completionType;
     this.lastCompletion = {
       value: inputValue,
@@ -1655,23 +1661,26 @@ JSTerm.prototype = {
    *         True if there was a selected completion item and the input value
    *         was updated, false otherwise.
    */
   acceptProposedCompletion: function () {
     let updated = false;
 
     let currentItem = this.autocompletePopup.selectedItem;
     if (currentItem && this.lastCompletion.value) {
-      let suffix =
-        currentItem.label.substring(this.lastCompletion.matchProp.length);
+      let value = this.getInputValue();
       let cursor = this.inputNode.selectionStart;
-      let value = this.getInputValue();
-      this.setInputValue(value.substr(0, cursor) +
-        suffix + value.substr(cursor));
-      let newCursor = cursor + suffix.length;
+      let matchLength = this.lastCompletion.matchProp.length;
+      let completion = currentItem.label;
+      let dotPosition = cursor - matchLength;
+      let newValue = value.substring(0, dotPosition) + completion
+        + value.substring(cursor, value.length);
+      let newCursor = dotPosition + completion.length;
+
+      this.setInputValue(newValue);
       this.inputNode.selectionStart = this.inputNode.selectionEnd = newCursor;
       updated = true;
     }
 
     this.clearCompletion();
 
     return updated;
   },
--- a/devtools/client/webconsole/test/browser_webconsole_bug_585991_autocomplete_keys.js
+++ b/devtools/client/webconsole/test/browser_webconsole_bug_585991_autocomplete_keys.js
@@ -20,16 +20,18 @@ add_task(function* () {
   yield consoleOpened(hud);
   yield popupHideAfterTab();
   yield testReturnKey();
   yield dontShowArrayNumbers();
   yield testReturnWithNoSelection();
   yield popupHideAfterReturnWithNoSelection();
   yield testCompletionInText();
   yield popupHideAfterCompletionInText();
+  yield testCaseInsensitiveCompletionAllCaps();
+  yield testCaseInsensitiveCompletionMixedCase();
 
   HUD = popup = jsterm = inputNode = completeNode = null;
   Services.prefs.setBoolPref(PREF_AUTO_MULTILINE, true);
 });
 
 var consoleOpened = Task.async(function* (hud) {
   let deferred = promise.defer();
   HUD = hud;
@@ -360,8 +362,67 @@ function popupHideAfterCompletionInText(
      "completion was successful after VK_TAB");
   is(inputNode.selectionStart, 26, "cursor location is correct");
   is(inputNode.selectionStart, inputNode.selectionEnd,
      "cursor location (confirmed)");
   ok(!completeNode.value, "completeNode is empty");
 
   return promise.resolve();
 }
+
+function testCaseInsensitiveCompletionAllCaps() {
+  info("test that case insensitive completion works with all caps (see bug 672733)");
+
+  let deferred = promise.defer();
+
+  popup.once("popup-opened", function onShown() {
+    EventUtils.synthesizeKey("F", {});
+    EventUtils.synthesizeKey("O", {});
+    EventUtils.synthesizeKey("O", {});
+
+    is(popup.itemCount, 1, "popup.itemCount is correct");
+
+    let items = popup.getItems();
+    let containsItem = items[0].label === "foobarBug585991";
+    ok(containsItem, "getItems returns the expected item");
+
+    deferred.resolve();
+  });
+
+  jsterm.setInputValue("window");
+  EventUtils.synthesizeKey(".", {});
+
+  return deferred.promise;
+}
+
+function testCaseInsensitiveCompletionMixedCase() {
+  info("test completion works with mixed case input (see bug 672733)");
+
+  let deferred = promise.defer();
+
+  popup.once("popup-opened", function onShown() {
+    EventUtils.synthesizeKey("T", {});
+    EventUtils.synthesizeKey("e", {});
+    EventUtils.synthesizeKey("S", {});
+    EventUtils.synthesizeKey("t", {});
+
+    is(popup.itemCount, 2, "popup.itemCount is correct");
+
+    let items = popup.getItems().reverse();
+    let expectedItems = ["testBug873250a", "testBug873250b"];
+    let sameItems = items.every((prop, index) => {
+      return expectedItems[index] === prop.label;
+    });
+    ok(sameItems, "getItems returns the items we expect");
+
+    popup.once("popup-closed", function popupHidden() {
+      deferred.resolve();
+    });
+
+    EventUtils.synthesizeKey("VK_TAB", {});
+    EventUtils.synthesizeKey("VK_RETURN", {});
+  });
+
+  jsterm.setInputValue("window");
+  EventUtils.synthesizeKey(".", {});
+
+  return deferred.promise;
+}
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -1039,30 +1039,36 @@ WebConsoleActor.prototype =
 
     if (!hadDebuggee && dbgObject) {
       this.dbg.removeDebuggee(this.evalWindow);
     }
 
     let matches = result.matches || [];
     let reqText = aRequest.text.substr(0, aRequest.cursor);
 
-    // We consider '$' as alphanumerc because it is used in the names of some
+    // We consider '$' as alphanumeric because it is used in the names of some
     // helper functions.
     let lastNonAlphaIsDot = /[.][a-zA-Z0-9$]*$/.test(reqText);
     if (!lastNonAlphaIsDot) {
       if (!this._webConsoleCommandsCache) {
         let helpers = {
           sandbox: Object.create(null)
         };
         addWebConsoleCommands(helpers);
         this._webConsoleCommandsCache =
           Object.getOwnPropertyNames(helpers.sandbox);
       }
-      matches = matches.concat(this._webConsoleCommandsCache
-          .filter(n => n.startsWith(result.matchProp)));
+      if(result.matchProp) {
+        // We want to match case-insensitively, but we don't need to convert on
+        // every match, so we'll keep the result in a variable
+        let matchPropLc = result.matchProp.toLowerCase();
+        matches = matches.concat(this._webConsoleCommandsCache.filter(n => {
+          return n.toLowerCase().startsWith(matchPropLc);
+        }));
+      }
     }
 
     return {
       from: this.actorID,
       matches: matches.sort(),
       matchProp: result.matchProp,
     };
   },