Bug 1609942 - Display autocomplete popup at the top of the input. r=j…

…descottes,Honza.

This prevents the popup to cover the eager evaluation result.
In order for the popup to be able to appear outside of the
toolbox, we pass the useXulWrapper option to the HTMLTooltip.

Differential Revision: https://phabricator.services.mozilla.com/D62431

--HG--
extra : moz-landing-system : lando

devtools/client/shared/autocomplete-popup.js

@@ -27,6 +27,8 @@ let itemIdCounter = 0;
  *        An object consiting any of the following options:
  *        - listId {String} The id for the list <LI> element.
  *        - position {String} The position for the tooltip ("top" or "bottom").
+ *        - useXulWrapper {Boolean} If the tooltip is hosted in a XUL document, use a
+ *          XUL panel in order to use all the screen viewport available (defaults to false).
  *        - autoSelect {Boolean} Boolean to allow the first entry of the popup
  *          panel to be automatically selected when the popup shows.
  *        - onSelect {String} Callback called when the selected index is updated.
@@ -37,10 +39,10 @@ function AutocompletePopup(toolboxDoc, options = {}) {
   EventEmitter.decorate(this);
 
   this._document = toolboxDoc;
-
   this.autoSelect = options.autoSelect || false;
   this.listId = options.listId || null;
   this.position = options.position || "bottom";
+  this.useXulWrapper = options.useXulWrapper || false;
 
   this.onSelectCallback = options.onSelect;
   this.onClickCallback = options.onClick;
@@ -99,15 +101,18 @@ AutocompletePopup.prototype = {
       return this._tooltip;
     }
 
-    this._tooltip = new HTMLTooltip(this._document);
+    this._tooltip = new HTMLTooltip(this._document, {
+      useXulWrapper: this.useXulWrapper,
+    });
+
     this._tooltip.panel.classList.add(
       "devtools-autocomplete-popup",
       "devtools-monospace"
     );
     // Stop this appearing as an alert to accessibility.
     this._tooltip.panel.setAttribute("role", "presentation");
     this._tooltip.panel.appendChild(this.list);
-    this._tooltip.setContentSize({ height: Infinity });
+    this._tooltip.setContentSize({ height: "auto" });
 
     return this._tooltip;
   },
@@ -145,27 +150,34 @@ AutocompletePopup.prototype = {
    *        The position of item to select.
    * @param {Object} options: Check `selectItemAtIndex` for more information.
    */
-  openPopup: function(anchor, xOffset = 0, yOffset = 0, index, options) {
+  openPopup: async function(anchor, xOffset = 0, yOffset = 0, index, options) {
     // Retrieve the anchor's document active element to add accessibility metadata.
     this._activeElement = anchor.ownerDocument.activeElement;
 
     // We want the autocomplete items to be perflectly lined-up with the string the
     // user entered, so we need to remove the left-padding and the left-border from
     // the xOffset.
     const leftBorderSize = 1;
-    this.tooltip.show(anchor, {
+
+    // If we have another call to openPopup while the previous one isn't over yet, we
+    // need to wait until it's settled to not be in a compromised state.
+    if (this._pendingShowPromise) {
+      await this._pendingShowPromise;
+    }
+
+    this._pendingShowPromise = this.tooltip.show(anchor, {
       x: xOffset - this._listPadding - leftBorderSize,
       y: yOffset,
       position: this.position,
     });
+    await this._pendingShowPromise;
+    this._pendingShowPromise = null;
 
-    this.tooltip.once("shown", () => {
-      if (this.autoSelect) {
-        this.selectItemAtIndex(index, options);
-      }
+    if (this.autoSelect) {
+      this.selectItemAtIndex(index, options);
+    }
 
-      this.emit("popup-opened");
-    });
+    this.emit("popup-opened");
   },
 
   /**
@@ -219,6 +231,7 @@ AutocompletePopup.prototype = {
    * Hide the autocomplete popup panel.
    */
   hidePopup: function() {
+    this._pendingShowPromise = null;
     this.tooltip.once("hidden", () => {
       this.emit("popup-closed");
     });
@@ -242,6 +255,7 @@ AutocompletePopup.prototype = {
    * cleanup.
    */
   destroy: function() {
+    this._pendingShowPromise = null;
     if (this.isOpen) {
       this.hidePopup();
     }

devtools/client/shared/test/browser.ini

@@ -63,6 +63,7 @@ support-files =
   !/devtools/client/debugger/test/mochitest/helpers.js
   !/devtools/client/debugger/test/mochitest/helpers/context.js
 
+[browser_autocomplete_popup_consecutive-show.js]
 [browser_autocomplete_popup.js]
 [browser_browserloader_mocks.js]
 [browser_css_angle.js]

devtools/client/shared/test/browser_autocomplete_popup_consecutive-show.js

@@ -0,0 +1,57 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that calling `showPopup` multiple time does not lead to invalid state.
+
+add_task(async function() {
+  const AutocompletePopup = require("devtools/client/shared/autocomplete-popup");
+
+  info("Create an autocompletion popup");
+  const [, , doc] = await createHost();
+  const input = doc.createElement("input");
+  doc.body.appendChild(input);
+
+  const autocompleteOptions = {
+    position: "top",
+    autoSelect: true,
+    useXulWrapper: true,
+  };
+  const popup = new AutocompletePopup(doc, autocompleteOptions);
+  const items = [{ label: "a" }, { label: "b" }, { label: "c" }];
+  popup.setItems(items);
+
+  input.focus();
+
+  let onAllEventsReceived = waitForNEvents(popup, "popup-opened", 3);
+  // Note that the lack of `await` on those function calls are wanted.
+  popup.openPopup(input, 0, 0, 0);
+  popup.openPopup(input, 0, 0, 1);
+  popup.openPopup(input, 0, 0, 2);
+  await onAllEventsReceived;
+
+  ok(popup.isOpen, "popup is open");
+  is(
+    popup.selectedIndex,
+    2,
+    "Selected index matches the one that was set last when calling openPopup"
+  );
+
+  onAllEventsReceived = waitForNEvents(popup, "popup-opened", 2);
+  // Note that the lack of `await` on those function calls are wanted.
+  popup.openPopup(input, 0, 0, 1);
+  popup.openPopup(input);
+  await onAllEventsReceived;
+
+  ok(popup.isOpen, "popup is open");
+  is(
+    popup.selectedIndex,
+    0,
+    "First item is selected, as last call to openPopup did not specify an index and autoSelect is true"
+  );
+
+  const onPopupClose = popup.once("popup-closed");
+  popup.hidePopup();
+  await onPopupClose;
+});

devtools/client/themes/common.css

@@ -132,7 +132,6 @@ button::selection {
   padding: 0;
   overflow-x: hidden;
   max-height: 20rem;
-  height: 100%;
   box-sizing: border-box;
 }
 

devtools/client/webconsole/components/Input/JSTerm.js

@@ -116,6 +116,7 @@ class JSTerm extends Component {
       showEditorOnboarding: PropTypes.bool,
       autocomplete: PropTypes.bool,
       showEvaluationSelector: PropTypes.bool,
+      autocompletePopupPosition: PropTypes.string,
     };
   }
 
@@ -173,8 +174,9 @@ class JSTerm extends Component {
       onSelect: this.onAutocompleteSelect.bind(this),
       onClick: this.acceptProposedCompletion.bind(this),
       listId: "webConsole_autocompletePopupListBox",
-      position: "bottom",
+      position: this.props.autocompletePopupPosition,
       autoSelect: true,
+      useXulWrapper: true,
     };
 
     const doc = this.webConsoleUI.document;
@@ -563,6 +565,14 @@ class JSTerm extends Component {
         this.setEditorWidth(null);
       }
     }
+
+    if (
+      nextProps.autocompletePopupPosition !==
+        this.props.autocompletePopupPosition &&
+      this.autocompletePopup
+    ) {
+      this.autocompletePopup.position = nextProps.autocompletePopupPosition;
+    }
   }
 
   /**
@@ -934,7 +944,7 @@ class JSTerm extends Component {
    *        }
    * @fires autocomplete-updated
    */
-  updateAutocompletionPopup(data) {
+  async updateAutocompletionPopup(data) {
     if (!this.editor) {
       return;
     }
@@ -1003,7 +1013,7 @@ class JSTerm extends Component {
       const xOffset = -1 * matchProp.length * this._inputCharWidth;
       const yOffset = 5;
       const popupAlignElement = this.props.serviceContainer.getJsTermTooltipAnchor();
-      popup.openPopup(popupAlignElement, xOffset, yOffset, 0, {
+      await popup.openPopup(popupAlignElement, xOffset, yOffset, 0, {
         preventSelectCallback: true,
       });
     } else if (items.length < minimumAutoCompleteLength && popup.isOpen) {
@@ -1387,6 +1397,7 @@ function mapStateToProps(state) {
     autocompleteData: getAutocompleteState(state),
     showEditorOnboarding: state.ui.showEditorOnboarding,
     showEvaluationSelector: state.ui.showEvaluationSelector,
+    autocompletePopupPosition: state.prefs.eagerEvaluation ? "top" : "bottom",
   };
 }
 

devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_commands.js

@@ -12,13 +12,11 @@ add_task(async function() {
   const { jsterm } = hud;
   const { autocompletePopup } = jsterm;
 
-  const onPopUpOpen = autocompletePopup.once("popup-opened");
-
   info(`Enter ":"`);
   jsterm.focus();
+  let onAutocompleUpdated = jsterm.once("autocomplete-updated");
   EventUtils.sendString(":");
-
-  await onPopUpOpen;
+  await onAutocompleUpdated;
 
   const expectedCommands = [":help", ":screenshot"];
   is(
@@ -27,7 +25,7 @@ add_task(async function() {
     "popup contains expected commands"
   );
 
-  let onAutocompleUpdated = jsterm.once("autocomplete-updated");
+  onAutocompleUpdated = jsterm.once("autocomplete-updated");
   EventUtils.sendString("s");
   await onAutocompleUpdated;
   checkInputCompletionValue(

devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_disabled.js

@@ -27,19 +27,15 @@ async function performTests_false() {
   await wait(2000);
   ok(!popup.isOpen, "popup is not open");
 
-  let onPopUpOpen = popup.once("popup-opened");
-
   info("Check that Ctrl+Space opens the popup when preference is false");
-  onPopUpOpen = popup.once("popup-opened");
+  let onUpdated = jsterm.once("autocomplete-updated");
   EventUtils.synthesizeKey(" ", { ctrlKey: true });
-  await onPopUpOpen;
+  await onUpdated;
 
   ok(popup.isOpen, "popup opens on Ctrl+Space");
-
-  const onUpdated = jsterm.once("autocomplete-updated");
-
   ok(popup.getItems().length > 0, "'w' gave a list of suggestions");
 
+  onUpdated = jsterm.once("autocomplete-updated");
   EventUtils.synthesizeKey("in");
   await onUpdated;
   ok(popup.getItems().length == 2, "'win' gave a list of suggestions");

devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_getters_cache.js

@@ -85,19 +85,19 @@ add_task(async function() {
   info(
     "Type a space, then backspace and ensure the autocomplete popup is displayed"
   );
-  let onAutocompleteUpdate = jsterm.once("autocomplete-updated");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
   EventUtils.synthesizeKey(" ");
-  await onAutocompleteUpdate;
+  await onPopUpOpen;
   is(autocompletePopup.isOpen, true, "Autocomplete popup is still opened");
   is(
     getAutocompletePopupLabels(autocompletePopup).join("-"),
     "baz-bloop",
     "popup has expected items"
   );
 
-  onAutocompleteUpdate = jsterm.once("autocomplete-updated");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
   EventUtils.synthesizeKey("KEY_Backspace");
-  await onAutocompleteUpdate;
+  await onPopUpOpen;
   is(autocompletePopup.isOpen, true, "Autocomplete popup is still opened");
   is(
     getAutocompletePopupLabels(autocompletePopup).join("-"),

devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_null.js

@@ -23,9 +23,9 @@ add_task(async function() {
   execute(hud, "globalThis.nullVar = null;");
 
   info(`Check completion suggestions for "null"`);
-  const onPopUpOpen = popup.once("popup-opened");
+  let onAutocompleteUpdated = jsterm.once("autocomplete-updated");
   EventUtils.sendString("null", hud.iframeWindow);
-  await onPopUpOpen;
+  await onAutocompleteUpdated;
   ok(popup.isOpen, "popup is open");
   const expectedPopupItems = ["null", "nullVar"];
   is(
@@ -35,7 +35,7 @@ add_task(async function() {
   );
 
   info(`Check completion suggestions for "null."`);
-  let onAutocompleteUpdated = jsterm.once("autocomplete-updated");
+  onAutocompleteUpdated = jsterm.once("autocomplete-updated");
   EventUtils.sendString(".", hud.iframeWindow);
   await onAutocompleteUpdated;
   is(popup.itemCount, 0, "popup has no items");

devtools/client/webconsole/test/browser/browser_jsterm_completion_bracket_cached_results.js

@@ -88,9 +88,9 @@ add_task(async function() {
   for (const test of tests) {
     info(test.description);
 
-    const onPopUpOpen = autocompletePopup.once("popup-opened");
+    let onPopupUpdate = jsterm.once("autocomplete-updated");
     EventUtils.sendString(test.initialInput);
-    await onPopUpOpen;
+    await onPopupUpdate;
 
     is(
       getAutocompletePopupLabels(autocompletePopup).join("|"),
@@ -107,7 +107,7 @@ add_task(async function() {
       expectedItems,
       expectedCompletionText,
     } of test.sequence) {
-      const onPopupUpdate = jsterm.once("autocomplete-updated");
+      onPopupUpdate = jsterm.once("autocomplete-updated");
       EventUtils.sendString(char);
       await onPopupUpdate;