Bug 1470103 - Fix autocomplete popup sizing issue in the console; r=jdescottes. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 18 Jul 2018 15:14:29 +0200
changeset 821462 95cf1a6c6f21ee253756f4f26a2fab89887f14b6
parent 821326 54c01f0092e1f1edcede61fd08042e99d76d5c30
push id117102
push userbmo:nchevobbe@mozilla.com
push dateMon, 23 Jul 2018 11:49:51 +0000
reviewersjdescottes
bugs1470103
milestone63.0a1
Bug 1470103 - Fix autocomplete popup sizing issue in the console; r=jdescottes. The popup was properly sized (and positionned), if it was previously closed, which could led to layout issues when changing the autocomplete popup (e.g. by typing a "."). We now always call openPopup when there's items in the popup so it's always properly sized. I'm expecting some performance overhead here, but I'm working on another bug that should make the popup faster. A test is added to make sure we do resize the popup as needed. MozReview-Commit-ID: K7dOllMrq0b
devtools/client/webconsole/components/JSTerm.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_width.js
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -1262,17 +1262,18 @@ class JSTerm extends Component {
     const items = matches.map(match => ({ preLabel: lastPart, label: match }));
     popup.setItems(items);
 
     const completionType = this.lastCompletion.completionType;
     this.lastCompletion = {
       value: inputValue,
       matchProp: lastPart,
     };
-    if (items.length > 0 && !popup.isOpen) {
+
+    if (items.length > 0) {
       let popupAlignElement;
       let xOffset;
       let yOffset;
 
       if (this.editor) {
         popupAlignElement = this.node.querySelector(".CodeMirror-cursor");
         // We need to show the popup at the ".".
         xOffset = -1 * lastPart.length * this._inputCharWidth;
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -193,16 +193,17 @@ skip-if = verify
 [browser_jsterm_autocomplete_in_chrome_tab.js]
 [browser_jsterm_autocomplete_in_debugger_stackframe.js]
 [browser_jsterm_autocomplete_inside_text.js]
 [browser_jsterm_autocomplete_native_getters.js]
 [browser_jsterm_autocomplete_nav_and_tab_key.js]
 [browser_jsterm_autocomplete_paste_undo.js]
 [browser_jsterm_autocomplete_return_key_no_selection.js]
 [browser_jsterm_autocomplete_return_key.js]
+[browser_jsterm_autocomplete_width.js]
 [browser_jsterm_autocomplete-properties-with-non-alphanumeric-names.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]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_width.js
@@ -0,0 +1,73 @@
+/* -*- 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";
+
+// Test that the autocomplete popup is resized when needed.
+
+const TEST_URI = `data:text/html;charset=utf-8,
+<head>
+  <script>
+    /* Create prototype-less object so popup does not contain native
+     * Object prototype properties.
+     */
+    window.x = Object.create(null, Object.getOwnPropertyDescriptors({
+      ["y".repeat(10)]: 1,
+      ["z".repeat(20)]: 2
+    }));
+    window.xx = 1;
+  </script>
+</head>
+<body>Test</body>`;
+
+add_task(async function() {
+  // Run test with legacy JsTerm
+  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: popup } = jsterm;
+
+  const onPopUpOpen = popup.once("popup-opened");
+
+  info(`wait for completion suggestions for "x"`);
+  EventUtils.sendString("x");
+
+  await onPopUpOpen;
+
+  ok(popup.isOpen, "popup is open");
+
+  const expectedPopupItems = ["x", "xx"];
+  is(popup.items.map(i => i.label).join("-"), expectedPopupItems.join("-"),
+    "popup has expected items");
+
+  const originalWidth = popup._tooltip.container.clientWidth;
+  ok(originalWidth > 2 * jsterm._inputCharWidth,
+    "popup is at least wider than the width of the longest list item");
+
+  info(`wait for completion suggestions for "x."`);
+  let onAutocompleteUpdated = jsterm.once("autocomplete-updated");
+  EventUtils.sendString(".");
+  await onAutocompleteUpdated;
+
+  is(popup.items.map(i => i.label).join("-"), ["y".repeat(10), "z".repeat(20)].join("-"),
+    "popup has expected items");
+  const newPopupWidth = popup._tooltip.container.clientWidth;
+  ok(newPopupWidth > originalWidth, "The popup width was updated");
+  ok(newPopupWidth > 20 * jsterm._inputCharWidth,
+    "popup is at least wider than the width of the longest list item");
+
+  info(`wait for completion suggestions for "x"`);
+  onAutocompleteUpdated = jsterm.once("autocomplete-updated");
+  EventUtils.synthesizeKey("KEY_Backspace");
+  await onAutocompleteUpdated;
+
+  is(popup._tooltip.container.clientWidth, originalWidth,
+    "popup is back to its original width");
+}