Bug 1514766 - Change the order of the autocomplete popup to display lowercase items and perfect matches first. r=nchevobbe.
authorArt-Vanderlay <causality1@outlook.com>
Tue, 16 Apr 2019 12:31:51 +0000
changeset 469651 0fab2cc0f44d
parent 469650 8186ed6d36ff
child 469652 56b4bb2871d1
push id35878
push userapavel@mozilla.com
push dateTue, 16 Apr 2019 15:43:40 +0000
treeherdermozilla-central@258af4e91151 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1514766
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 1514766 - Change the order of the autocomplete popup to display lowercase items and perfect matches first. r=nchevobbe. The autocomplete popup in the web console now favours matches with the user input in alphabetical order with lowercase letters and perfect matches brought to the top of the list. e.g. Typing 'document.styleSheets' brings up the result: -styleSheets -styleSheetSets Differential Revision: https://phabricator.services.mozilla.com/D19330
devtools/client/webconsole/reducers/autocomplete.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_jsterm_completion_bracket.js
devtools/client/webconsole/test/mochitest/browser_jsterm_completion_case_sensitivity.js
devtools/client/webconsole/test/mochitest/browser_jsterm_completion_perfect_match.js
devtools/server/actors/webconsole.js
devtools/shared/webconsole/test/test_jsterm_autocomplete.html
--- a/devtools/client/webconsole/reducers/autocomplete.js
+++ b/devtools/client/webconsole/reducers/autocomplete.js
@@ -137,16 +137,34 @@ function autoCompleteRetrieveFromCache(s
 
     if (looseMatching) {
       return l.toLocaleLowerCase().startsWith(filterByLc);
     }
 
     return l.startsWith(filterBy);
   });
 
+  newList.sort((a, b) => {
+    const startingQuoteRegex = /^('|"|`)/;
+    const aFirstMeaningfulChar = startingQuoteRegex.test(a) ? a[1] : a[0];
+    const bFirstMeaningfulChar = startingQuoteRegex.test(b) ? b[1] : b[0];
+    const lA = aFirstMeaningfulChar.toLocaleLowerCase() === aFirstMeaningfulChar;
+    const lB = bFirstMeaningfulChar.toLocaleLowerCase() === bFirstMeaningfulChar;
+    if (lA === lB) {
+      if (a === filterBy) {
+        return -1;
+      }
+      if (b === filterBy) {
+        return 1;
+      }
+      return a.localeCompare(b);
+    }
+    return lA ? -1 : 1;
+  });
+
   return {
     ...state,
     isUnsafeGetter: false,
     getterPath: null,
     matches: newList,
     matchProp: filterBy,
     isElementAccess: cache.isElementAccess,
   };
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -186,16 +186,17 @@ skip-if = verify
 [browser_jsterm_await_dynamic_import.js]
 [browser_jsterm_await_error.js]
 [browser_jsterm_await_helper_dollar_underscore.js]
 [browser_jsterm_await_paused.js]
 [browser_jsterm_await.js]
 [browser_jsterm_completion_bracket_cached_results.js]
 [browser_jsterm_completion_bracket.js]
 [browser_jsterm_completion_case_sensitivity.js]
+[browser_jsterm_completion_perfect_match.js]
 [browser_jsterm_completion_dollar_underscore.js]
 [browser_jsterm_completion_dollar_zero.js]
 [browser_jsterm_completion.js]
 [browser_jsterm_content_defined_helpers.js]
 [browser_jsterm_context_menu_labels.js]
 skip-if = (os == "win" && processor == "aarch64") # disabled on aarch64 due to 1531571
 [browser_jsterm_copy_command.js]
 [browser_jsterm_ctrl_a_select_all.js]
--- a/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_bracket.js
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_bracket.js
@@ -44,77 +44,77 @@ async function testInputs(hud) {
     input: "window.testObject[",
     expectedItems: [
       `"bar"`,
       `"da'ta'test"`,
       `"da\\"ta\\"test"`,
       `"da\`ta\`test"`,
       `"data-test"`,
       `"dataTest"`,
+      `"DAT_\\\\a\\"'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST"`,
       `"DATA-TEST"`,
-      `"DAT_\\\\a\\"'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST"`,
     ],
     expectedCompletionText: `"bar"]`,
     expectedInputAfterCompletion: `window.testObject["bar"]`,
   }, {
     description: "Test that the list can be filtered even without quote",
     input: "window.testObject[d",
     expectedItems: [
       `"da'ta'test"`,
       `"da\\"ta\\"test"`,
       `"da\`ta\`test"`,
       `"data-test"`,
       `"dataTest"`,
+      `"DAT_\\\\a\\"'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST"`,
       `"DATA-TEST"`,
-      `"DAT_\\\\a\\"'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST"`,
     ],
     expectedCompletionText: `a'ta'test"]`,
     expectedInputAfterCompletion: `window.testObject["da'ta'test"]`,
   }, {
     description: "Test filtering with quote and string",
     input: `window.testObject["d`,
     expectedItems: [
       `"da'ta'test"`,
       `"da\\"ta\\"test"`,
       `"da\`ta\`test"`,
       `"data-test"`,
       `"dataTest"`,
+      `"DAT_\\\\a\\"'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST"`,
       `"DATA-TEST"`,
-      `"DAT_\\\\a\\"'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST"`,
     ],
     expectedCompletionText: `a'ta'test"]`,
     expectedInputAfterCompletion: `window.testObject["da'ta'test"]`,
   }, {
     description: "Test filtering with simple quote and string",
     input: `window.testObject['d`,
     expectedItems: [
       `'da"ta"test'`,
       `'da\\'ta\\'test'`,
       `'da\`ta\`test'`,
       `'data-test'`,
       `'dataTest'`,
+      `'DAT_\\\\a"\\'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST'`,
       `'DATA-TEST'`,
-      `'DAT_\\\\a"\\'\`\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST'`,
     ],
     expectedCompletionText: `a"ta"test']`,
     expectedInputAfterCompletion: `window.testObject['da"ta"test']`,
   }, {
     description: "Test filtering with template literal and string",
     input: "window.testObject[`d",
     expectedItems: [
+      "`da'ta'test`",
       "`da\"ta\"test`",
-      "`da'ta'test`",
       "`da\\`ta\\`test`",
       "`data-test`",
       "`dataTest`",
+      "`DAT_\\\\a\"'\\`\\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST`",
       "`DATA-TEST`",
-      "`DAT_\\\\a\"'\\`\\${0}\\u0000\\b\\t\\n\\f\\r\\ude80\\ud83d🚀_TEST`",
     ],
-    expectedCompletionText: 'a"ta"test`]',
-    expectedInputAfterCompletion: 'window.testObject[`da"ta"test`]',
+    expectedCompletionText: "a\'ta\'test`]",
+    expectedInputAfterCompletion: "window.testObject[`da\'ta\'test`]",
   }, {
     description: "Test that filtering is case insensitive",
     input: "window.testObject[data-t",
     expectedItems: [
       `"data-test"`,
       `"DATA-TEST"`,
     ],
     expectedCompletionText: `est"]`,
--- a/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_case_sensitivity.js
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_case_sensitivity.js
@@ -100,20 +100,20 @@ async function performTests() {
 
   setInputValue(hud, "");
 
   info("Check that filtering the cache works like on the server");
   onPopUpOpen = autocompletePopup.once("popup-opened");
   EventUtils.sendString("fooBar.");
   await onPopUpOpen;
   is(getAutocompletePopupLabels(autocompletePopup).join(" - "),
-    "test - Foo - TEST - Test", "popup has expected items");
+    "test - Foo - Test - TEST", "popup has expected items");
 
   onAutoCompleteUpdated = jsterm.once("autocomplete-updated");
   EventUtils.sendString("T");
   await onAutoCompleteUpdated;
-  is(getAutocompletePopupLabels(autocompletePopup).join(" - "), "TEST - Test",
+  is(getAutocompletePopupLabels(autocompletePopup).join(" - "), "Test - TEST",
     "popup was filtered case-sensitively, as expected");
 }
 
 function getAutocompletePopupLabels(autocompletePopup) {
   return autocompletePopup.items.map(i => i.label);
 }
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_perfect_match.js
@@ -0,0 +1,50 @@
+/* -*- 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/ */
+
+// Tests that code completion works properly in regards to case sensitivity.
+
+"use strict";
+
+const TEST_URI = `data:text/html;charset=utf8,<p>test completion perfect match.
+  <script>
+    x = Object.create(null, Object.getOwnPropertyDescriptors({
+      foo: 1,
+      foO: 2,
+      fOo: 3,
+      fOO: 4,
+    }));
+  </script>`;
+
+add_task(async function() {
+  // Run test with legacy JsTerm
+  await pushPref("devtools.webconsole.jsterm.codeMirror", false);
+  await performTests();
+  // And then run it with the CodeMirror-powered one.
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
+  await performTests();
+});
+
+async function performTests() {
+  const hud = await openNewTabAndConsole(TEST_URI);
+  const {jsterm} = hud;
+  const {autocompletePopup} = jsterm;
+
+  info("Check that filtering the cache works like on the server");
+  const onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.");
+  await onPopUpOpen;
+  is(getAutocompletePopupLabels(autocompletePopup).join(" - "), "foo - foO - fOo - fOO",
+    "popup has expected item, in expected order");
+
+  const onAutoCompleteUpdated = jsterm.once("autocomplete-updated");
+  EventUtils.sendString("foO");
+  await onAutoCompleteUpdated;
+  is(getAutocompletePopupLabels(autocompletePopup).join(" - "), "foO - foo - fOo - fOO",
+    "popup has expected item, in expected order");
+}
+
+function getAutocompletePopupLabels(autocompletePopup) {
+  return autocompletePopup.items.map(i => i.label);
+}
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -1245,17 +1245,23 @@ WebConsoleActor.prototype =
       // first letter was lowercase).
       const firstMeaningfulCharIndex = isElementAccess ? 1 : 0;
       matches = Array.from(matches).sort((a, b) => {
         const aFirstMeaningfulChar = a[firstMeaningfulCharIndex];
         const bFirstMeaningfulChar = b[firstMeaningfulCharIndex];
         const lA = aFirstMeaningfulChar.toLocaleLowerCase() === aFirstMeaningfulChar;
         const lB = bFirstMeaningfulChar.toLocaleLowerCase() === bFirstMeaningfulChar;
         if (lA === lB) {
-          return a < b ? -1 : 1;
+          if (a === matchProp) {
+            return -1;
+          }
+          if (b === matchProp) {
+            return 1;
+          }
+          return a.localeCompare(b);
         }
         return lA ? -1 : 1;
       });
     }
 
     return {
       from: this.actorID,
       matches,
--- a/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
+++ b/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
@@ -341,32 +341,32 @@
     const {matches} = await client.autocomplete("true || foobar");
     is(matches.length, 1, "autocomplete returns expected results");
     is(matches.join("-"), "foobarObject");
   }
 
   async function doInsensitiveAutocomplete(client) {
     info("test autocomplete for 'window.insensitiveTestCase.'");
     let {matches} = await client.autocomplete("window.insensitiveTestCase.");
-    is(matches.join("-"), "prop-pröp-PROP-PRÖP-Prop",
+    is(matches.join("-"), "prop-pröp-Prop-PROP-PRÖP",
       "autocomplete returns the expected items, in the expected order");
 
     info("test autocomplete for 'window.insensitiveTestCase.p'");
     matches = (await client.autocomplete("window.insensitiveTestCase.p")).matches;
-    is(matches.join("-"), "prop-pröp-PROP-PRÖP-Prop",
+    is(matches.join("-"), "prop-pröp-Prop-PROP-PRÖP",
       "autocomplete is case-insensitive when first letter is lowercased");
 
     info("test autocomplete for 'window.insensitiveTestCase.pRoP'");
     matches = (await client.autocomplete("window.insensitiveTestCase.pRoP")).matches;
-    is(matches.join("-"), "prop-PROP-Prop",
+    is(matches.join("-"), "prop-Prop-PROP",
       "autocomplete is case-insensitive when first letter is lowercased");
 
     info("test autocomplete for 'window.insensitiveTestCase.P'");
     matches = (await client.autocomplete("window.insensitiveTestCase.P")).matches;
-    is(matches.join("-"), "PROP-PRÖP-Prop",
+    is(matches.join("-"), "Prop-PROP-PRÖP",
       "autocomplete is case-sensitive when first letter is uppercased");
 
     info("test autocomplete for 'window.insensitiveTestCase.PROP'");
     matches = (await client.autocomplete("window.insensitiveTestCase.PROP")).matches;
     is(matches.join("-"), "PROP",
       "autocomplete is case-sensitive when first letter is uppercased");
 
     info("test autocomplete for 'window.insensitiveTestCase.prö'");
@@ -416,17 +416,17 @@
       `'da"ta"test'|'da\\'ta\\'test'|'da\`ta\`test'|'data-test'|'dataTest'`,
       "autocomplete returns the expected items, wrapped in the same quotes the user entered");
     is(res.isElementAccess, true);
 
     info("test autocomplete for 'window.elementAccessTestCase[`d'");
     res = await client.autocomplete("window.elementAccessTestCase[`d");
     is(
       res.matches.join("|"),
-      "`da\"ta\"test`|`da'ta'test`|`da\\`ta\\`test`|`data-test`|`dataTest`",
+      "`da'ta'test`|`da\"ta\"test`|`da\\`ta\\`test`|`data-test`|`dataTest`",
       "autocomplete returns the expected items, wrapped in the same quotes the user entered");
     is(res.isElementAccess, true);
 
     info(`test autocomplete for '['`);
     res = await client.autocomplete(`[`);
     is(res.matches, null, "it does not return anything");
 
     info(`test autocomplete for '[1,2,3'`);