Bug 1470103 - Fix autocomplete popup sizing issue in the console; r=jdescottes.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 18 Jul 2018 15:14:29 +0200
changeset 427968 59adeffe6843ce9cde478fad8757fca547144588
parent 427967 1958916877ccdd6f20076d39c78c5a0d74e6acd2
child 427969 2d102208c9ba338076cfc00448e3b791435a5ccd
push id34322
push userrgurzau@mozilla.com
push dateTue, 24 Jul 2018 15:47:07 +0000
treeherdermozilla-central@db9d47b49936 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1470103
milestone63.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 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");
+}