Bug 1473841 - Transform property access on a dot-notation invalid name into an element access; r=bgrins.
☠☠ backed out by 638dee1dfab1 ☠ ☠
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 17 Oct 2018 20:55:26 +0000
changeset 500317 0e8706e5d2ffc8a43f45ae6a7f6e6b286e44e3ca
parent 500316 826cd78b94cb08b178956660832c4596543a23b0
child 500318 32cff4bae8200974970704733601e14345058a6f
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1473841
milestone64.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 1473841 - Transform property access on a dot-notation invalid name into an element access; r=bgrins. This patch turns property access that would result in Syntax error (e.g. `x.data-test`) into element access (e.g. `x["data-test"]`) when accepting a completion value in the console input. In order to do that, we use Reflect to parse a custom expression where we try to define the property the user is going to accept. If this throws, this means we need to modify the input into an element access. A test is added to make sure this works as expected. Differential Revision: https://phabricator.services.mozilla.com/D8952
devtools/client/webconsole/components/JSTerm.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_jsterm_completion_invalid_dot_notation.js
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -14,16 +14,17 @@ loader.lazyRequireGetter(this, "Debugger
 loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
 loader.lazyRequireGetter(this, "AutocompletePopup", "devtools/client/shared/autocomplete-popup");
 loader.lazyRequireGetter(this, "PropTypes", "devtools/client/shared/vendor/react-prop-types");
 loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true);
 loader.lazyRequireGetter(this, "KeyCodes", "devtools/client/shared/keycodes", true);
 loader.lazyRequireGetter(this, "Editor", "devtools/client/sourceeditor/editor");
 loader.lazyRequireGetter(this, "Telemetry", "devtools/client/shared/telemetry");
 loader.lazyRequireGetter(this, "saveScreenshot", "devtools/shared/screenshot/save");
+loader.lazyRequireGetter(this, "Reflect", "resource://gre/modules/reflect.jsm", true);
 
 const l10n = require("devtools/client/webconsole/webconsole-l10n");
 
 const HELP_URL = "https://developer.mozilla.org/docs/Tools/Web_Console/Helpers";
 
 function gSequenceId() {
   return gSequenceId.n++;
 }
@@ -1394,16 +1395,48 @@ class JSTerm extends Component {
         const inputAfterCursor = this.getInputValue().substring(inputBeforeCursor.length);
         // If there's not a bracket after the cursor, add it.
         if (!inputAfterCursor.trimLeft().startsWith("]")) {
           completionText = completionText + "]";
         }
       }
     }
 
+    if (
+      this.autocompletePopup.selectedItem ||
+      (!this.autocompletePopup.isOpen && this.autocompletePopup.items.length === 1)
+    ) {
+      const isValidDotNotation = propertyName => {
+        // If the item is a command, we don't want to modify it.
+        if (propertyName.startsWith(":")
+          && propertyName.includes(this.getInputValue().trim())) {
+          return true;
+        }
+
+        let valid = true;
+        try {
+          // In order to know if the property is suited for dot notation, we use Reflect
+          // to parse an expression where we try to access the property with a dot. If it
+          // throws, this means that we need to do an element access instead.
+          Reflect.parse(`({${propertyName}: true})`);
+        } catch (e) {
+          valid = false;
+        }
+        return valid;
+      };
+
+      const selectedItem = this.autocompletePopup.selectedItem
+        || this.autocompletePopup.items[0];
+      const {label, preLabel, isElementAccess} = selectedItem;
+      if (!isElementAccess && !isValidDotNotation(label)) {
+        completionText = `["${label.replace(/"/gi, '\\"')}"]`;
+        numberOfCharsToReplaceCharsBeforeCursor = preLabel.length + 1;
+      }
+    }
+
     this.clearCompletion();
 
     if (completionText) {
       this.insertStringAtCursor(completionText, numberOfCharsToReplaceCharsBeforeCursor);
     }
   }
 
   getInputValueBeforeCursor() {
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -207,16 +207,17 @@ skip-if = verify
 [browser_jsterm_autocomplete-properties-with-non-alphanumeric-names.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_invalid_dot_notation.js]
 [browser_jsterm_completion.js]
 [browser_jsterm_content_defined_helpers.js]
 [browser_jsterm_copy_command.js]
 [browser_jsterm_ctrl_a_select_all.js]
 [browser_jsterm_ctrl_key_nav.js]
 skip-if = os != 'mac' # The tested ctrl+key shortcuts are OSX only
 [browser_jsterm_document_no_xray.js]
 [browser_jsterm_error_docs.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_invalid_dot_notation.js
@@ -0,0 +1,99 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests that accepting a code completion with an invalid dot-notation property turns the
+// property access into an element access (`x.data-test` -> `x["data-test"]`).
+
+"use strict";
+
+const TEST_URI = `data:text/html;charset=utf8,<p>test code completion
+  <script>
+    x = Object.create(null);
+    x.dataTest = 1;
+    x["data-test"] = 2;
+    x['da"ta"test'] = 3;
+    x["DATA-TEST"] = 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 {jsterm} = await openNewTabAndConsole(TEST_URI);
+  const {autocompletePopup} = jsterm;
+
+  info("Ensure we have the correct items in the autocomplete popup");
+  let onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.da");
+  await onPopUpOpen;
+  is(getAutocompletePopupLabels(autocompletePopup).join("|"),
+    "da\"ta\"test|data-test|dataTest|DATA-TEST",
+    `popup has expected items`);
+
+  info("Test that accepting the completion of a property with quotes changes the " +
+    "property access into an element access");
+  checkJsTermCompletionValue(jsterm, `    "ta"test`, `completeNode has expected value`);
+  let onPopupClose = autocompletePopup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Tab");
+  await onPopupClose;
+  checkJsTermValueAndCursor(jsterm, `x["da\\"ta\\"test"]|`,
+    "Property access was transformed into an element access, and quotes were properly " +
+    "escaped");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+
+  jsterm.setInputValue("");
+
+  info("Test that accepting the completion of a property with a dash changes the " +
+    "property access into an element access");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.data-");
+  await onPopUpOpen;
+
+  checkJsTermCompletionValue(jsterm, `       test`, `completeNode has expected value`);
+  onPopupClose = autocompletePopup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Tab");
+  await onPopupClose;
+  checkJsTermValueAndCursor(jsterm, `x["data-test"]|`,
+   "Property access was transformed into an element access");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+
+  jsterm.setInputValue("");
+
+  info("Test that accepting the completion of an uppercase property with a dash changes" +
+    " the property access into an element access with the correct casing");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.data-");
+  await onPopUpOpen;
+
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  checkJsTermCompletionValue(jsterm, `       TEST`, `completeNode has expected value`);
+  onPopupClose = autocompletePopup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Tab");
+  await onPopupClose;
+  checkJsTermValueAndCursor(jsterm, `x["DATA-TEST"]|`,
+   "Property access was transformed into an element access with the correct casing");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+
+  jsterm.setInputValue("");
+
+  info("Test that accepting the completion of an property with a dash, when the popup " +
+    " is not displayed still changes the property access into an element access");
+  await setInputValueForAutocompletion(jsterm, "x.D");
+  checkJsTermCompletionValue(jsterm, `   ATA-TEST`, `completeNode has expected value`);
+
+  EventUtils.synthesizeKey("KEY_Tab");
+
+  checkJsTermValueAndCursor(jsterm, `x["DATA-TEST"]|`,
+   "Property access was transformed into an element access with the correct casing");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+}
+
+function getAutocompletePopupLabels(autocompletePopup) {
+  return autocompletePopup.items.map(i => i.label);
+}