From 4c897a56afbfbb23ad94e7ce11a945a4f793e452 Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Wed, 17 Oct 2018 20:55:26 +0000 Subject: [PATCH] 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 --HG-- extra : moz-landing-system : lando --- .../client/webconsole/components/JSTerm.js | 33 +++++++ .../webconsole/test/mochitest/browser.ini | 1 + ..._jsterm_completion_invalid_dot_notation.js | 99 +++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 devtools/client/webconsole/test/mochitest/browser_jsterm_completion_invalid_dot_notation.js diff --git a/devtools/client/webconsole/components/JSTerm.js b/devtools/client/webconsole/components/JSTerm.js index 900524e12fe4..3d8d044340c6 100644 --- a/devtools/client/webconsole/components/JSTerm.js +++ b/devtools/client/webconsole/components/JSTerm.js @@ -19,6 +19,7 @@ loader.lazyRequireGetter(this, "KeyCodes", "devtools/client/shared/keycodes", tr 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"); @@ -1399,6 +1400,38 @@ class JSTerm extends Component { } } + 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) { diff --git a/devtools/client/webconsole/test/mochitest/browser.ini b/devtools/client/webconsole/test/mochitest/browser.ini index d2e132b49288..7c0d8f0a9084 100644 --- a/devtools/client/webconsole/test/mochitest/browser.ini +++ b/devtools/client/webconsole/test/mochitest/browser.ini @@ -212,6 +212,7 @@ skip-if = verify [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] diff --git a/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_invalid_dot_notation.js b/devtools/client/webconsole/test/mochitest/browser_jsterm_completion_invalid_dot_notation.js new file mode 100644 index 000000000000..fc92d3ca21c6 --- /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,

test code completion + `; + +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); +}