Backed out 2 changesets (bug 1616847, bug 1609942) for devtools failures on browser_jsterm_autocomplete_getters_cache.js.

CLOSED TREE

Backed out changeset 0ee1ac6bab06 (bug 1609942)
Backed out changeset 2357c3d7aa4a (bug 1616847)

--HG--
extra : histedit_source : 2b51afa3c551d06d8ca8b3a35ab0466be1921400

devtools/client/shared/autocomplete-popup.js

@@ -27,8 +27,6 @@ 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.
@@ -39,10 +37,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;
@@ -101,10 +99,7 @@ AutocompletePopup.prototype = {
       return this._tooltip;
     }
 
-    this._tooltip = new HTMLTooltip(this._document, {
-      useXulWrapper: this.useXulWrapper,
-    });
-
+    this._tooltip = new HTMLTooltip(this._document);
     this._tooltip.panel.classList.add(
       "devtools-autocomplete-popup",
       "devtools-monospace"
@@ -112,7 +107,7 @@ AutocompletePopup.prototype = {
     // Stop this appearing as an alert to accessibility.
     this._tooltip.panel.setAttribute("role", "presentation");
     this._tooltip.panel.appendChild(this.list);
-    this._tooltip.setContentSize({ height: "auto" });
+    this._tooltip.setContentSize({ height: Infinity });
 
     return this._tooltip;
   },
@@ -150,7 +145,7 @@ AutocompletePopup.prototype = {
    *        The position of item to select.
    * @param {Object} options: Check `selectItemAtIndex` for more information.
    */
-  openPopup: async function(anchor, xOffset = 0, yOffset = 0, index, options) {
+  openPopup: function(anchor, xOffset = 0, yOffset = 0, index, options) {
     // Retrieve the anchor's document active element to add accessibility metadata.
     this._activeElement = anchor.ownerDocument.activeElement;
 
@@ -158,26 +153,19 @@ AutocompletePopup.prototype = {
     // user entered, so we need to remove the left-padding and the left-border from
     // the xOffset.
     const leftBorderSize = 1;
-
-    // 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, {
+    this.tooltip.show(anchor, {
       x: xOffset - this._listPadding - leftBorderSize,
       y: yOffset,
       position: this.position,
     });
-    await this._pendingShowPromise;
-    this._pendingShowPromise = null;
 
-    if (this.autoSelect) {
-      this.selectItemAtIndex(index, options);
-    }
+    this.tooltip.once("shown", () => {
+      if (this.autoSelect) {
+        this.selectItemAtIndex(index, options);
+      }
 
-    this.emit("popup-opened");
+      this.emit("popup-opened");
+    });
   },
 
   /**
@@ -231,7 +219,6 @@ AutocompletePopup.prototype = {
    * Hide the autocomplete popup panel.
    */
   hidePopup: function() {
-    this._pendingShowPromise = null;
     this.tooltip.once("hidden", () => {
       this.emit("popup-closed");
     });
@@ -255,7 +242,6 @@ AutocompletePopup.prototype = {
    * cleanup.
    */
   destroy: function() {
-    this._pendingShowPromise = null;
     if (this.isOpen) {
       this.hidePopup();
     }

devtools/client/shared/components/menu/MenuButton.js

@@ -227,7 +227,7 @@ class MenuButton extends PureComponent {
       return;
     }
 
-    this.tooltip.show(this.buttonRef.current, {
+    this.tooltip.updateContainerBounds(this.buttonRef.current, {
       position: this.props.menuPosition,
       y: this.props.menuOffset,
     });

devtools/client/shared/test/browser.ini

@@ -63,7 +63,6 @@ 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

@@ -1,57 +0,0 @@
-/* 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/shared/test/browser_html_tooltip_resize.js

@@ -48,7 +48,7 @@ add_task(async function() {
 
   // Resize the content
   div.style.cssText = "width: 200px; height: 30px";
-  tooltip.show(box1, { position: "top" });
+  tooltip.updateContainerBounds(box1, { position: "top" });
 
   // The panel should have moved 100px to the left and 10px down
   const updatedPanelBounds = tooltip.panel

devtools/client/shared/test/helper_html_tooltip.js

@@ -9,18 +9,21 @@
  */
 
 /**
- * Display an existing HTMLTooltip on an anchor and properly wait for the popup to be
- * repainted.
+ * Display an existing HTMLTooltip on an anchor. After the tooltip "shown"
+ * event has been fired a reflow will be triggered.
  *
  * @param {HTMLTooltip} tooltip
  *        The tooltip instance to display
  * @param {Node} anchor
  *        The anchor that should be used to display the tooltip
  * @param {Object} see HTMLTooltip:show documentation
- * @return {Promise} promise that resolves when reflow and repaint are done.
+ * @return {Promise} promise that resolves when "shown" has been fired, reflow
+ *         and repaint done.
  */
 async function showTooltip(tooltip, anchor, { position, x, y } = {}) {
-  await tooltip.show(anchor, { position, x, y });
+  const onShown = tooltip.once("shown");
+  tooltip.show(anchor, { position, x, y });
+  await onShown;
   return waitForReflow(tooltip);
 }
 

devtools/client/shared/widgets/tooltip/HTMLTooltip.js

@@ -464,9 +464,8 @@ HTMLTooltip.prototype = {
   },
 
   /**
-   * Show the tooltip next to the provided anchor element, or update the tooltip position
-   * if it was already visible. A preferred position can be set.
-   * The event "shown" will be fired after the tooltip is displayed.
+   * Show the tooltip next to the provided anchor element. A preferred position
+   * can be set. The event "shown" will be fired after the tooltip is displayed.
    *
    * @param {Element} anchor
    *        The reference element with which the tooltip should be aligned
@@ -484,50 +483,27 @@ HTMLTooltip.prototype = {
    */
   async show(anchor, options) {
     const { left, top } = this._updateContainerBounds(anchor, options);
-    const isTooltipVisible = this.isVisible();
 
     if (this.useXulWrapper) {
-      if (!isTooltipVisible) {
-        await this._showXulWrapperAt(left, top);
-      } else {
-        this._moveXulWrapperTo(left, top);
-      }
+      await this._showXulWrapperAt(left, top);
     } else {
       this.container.style.left = left + "px";
       this.container.style.top = top + "px";
     }
 
-    if (isTooltipVisible) {
-      return;
-    }
-
     this.container.classList.add("tooltip-visible");
 
     // Keep a pointer on the focused element to refocus it when hiding the tooltip.
     this._focusedElement = this.doc.activeElement;
 
-    if (this.doc.defaultView) {
-      if (this.attachEventsTimer) {
-        this.doc.defaultView.clearTimeout(this.attachEventsTimer);
-      }
-
-      // On Windows and Linux, if the tooltip is shown on mousedown/click (which is the
-      // case for the MenuButton component for example), attaching the events listeners
-      // on the window right away would trigger the callbacks; which means the tooltip
-      // would be instantly hidden. To prevent such thing, the event listeners are set
-      // on the next tick.
-      await new Promise(resolve => {
-        this.attachEventsTimer = this.doc.defaultView.setTimeout(() => {
-          // Update the top window reference each time in case the host changes.
-          this.topWindow = this._getTopWindow();
-          this.topWindow.addEventListener("click", this._onClick, true);
-          this.topWindow.addEventListener("mouseup", this._onMouseup, true);
-          resolve();
-        }, 0);
-      });
-    }
-
-    this.emit("shown");
+    this.doc.defaultView.clearTimeout(this.attachEventsTimer);
+    this.attachEventsTimer = this.doc.defaultView.setTimeout(() => {
+      // Update the top window reference each time in case the host changes.
+      this.topWindow = this._getTopWindow();
+      this.topWindow.addEventListener("click", this._onClick, true);
+      this.topWindow.addEventListener("mouseup", this._onMouseup, true);
+      this.emit("shown");
+    }, 0);
   },
 
   startTogglingOnHover(baseNode, targetNodeCb, options) {
@@ -538,6 +514,27 @@ HTMLTooltip.prototype = {
     this.toggle.stop();
   },
 
+  /**
+   * Recalculate the dimensions and position of the tooltip in response to
+   * changes to its content.
+   *
+   * Parameters are identical to show().
+   */
+  updateContainerBounds(anchor, options) {
+    if (!this.isVisible()) {
+      return;
+    }
+
+    const { left, top } = this._updateContainerBounds(anchor, options);
+
+    if (this.useXulWrapper) {
+      this._moveXulWrapperTo(left, top);
+    } else {
+      this.container.style.left = left + "px";
+      this.container.style.top = top + "px";
+    }
+  },
+
   _updateContainerBounds(anchor, { position, x = 0, y = 0 } = {}) {
     // Get anchor geometry
     let anchorRect = getRelativeRect(anchor, this.doc);
@@ -797,10 +794,7 @@ HTMLTooltip.prototype = {
       return;
     }
 
-    if (this.doc && this.doc.defaultView) {
-      this.doc.defaultView.clearTimeout(this.attachEventsTimer);
-    }
-
+    this.doc.defaultView.clearTimeout(this.attachEventsTimer);
     // If the tooltip is hidden from a mouseup event, wait for a potential click event
     // to be consumed before removing event listeners.
     if (fromMouseup) {

devtools/client/themes/common.css

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

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

@@ -116,7 +116,6 @@ class JSTerm extends Component {
       showEditorOnboarding: PropTypes.bool,
       autocomplete: PropTypes.bool,
       showEvaluationSelector: PropTypes.bool,
-      autocompletePopupPosition: PropTypes.string,
     };
   }
 
@@ -174,9 +173,8 @@ class JSTerm extends Component {
       onSelect: this.onAutocompleteSelect.bind(this),
       onClick: this.acceptProposedCompletion.bind(this),
       listId: "webConsole_autocompletePopupListBox",
-      position: this.props.autocompletePopupPosition,
+      position: "bottom",
       autoSelect: true,
-      useXulWrapper: true,
     };
 
     const doc = this.webConsoleUI.document;
@@ -565,14 +563,6 @@ class JSTerm extends Component {
         this.setEditorWidth(null);
       }
     }
-
-    if (
-      nextProps.autocompletePopupPosition !==
-        this.props.autocompletePopupPosition &&
-      this.autocompletePopup
-    ) {
-      this.autocompletePopup.position = nextProps.autocompletePopupPosition;
-    }
   }
 
   /**
@@ -944,7 +934,7 @@ class JSTerm extends Component {
    *        }
    * @fires autocomplete-updated
    */
-  async updateAutocompletionPopup(data) {
+  updateAutocompletionPopup(data) {
     if (!this.editor) {
       return;
     }
@@ -1013,7 +1003,7 @@ class JSTerm extends Component {
       const xOffset = -1 * matchProp.length * this._inputCharWidth;
       const yOffset = 5;
       const popupAlignElement = this.props.serviceContainer.getJsTermTooltipAnchor();
-      await popup.openPopup(popupAlignElement, xOffset, yOffset, 0, {
+      popup.openPopup(popupAlignElement, xOffset, yOffset, 0, {
         preventSelectCallback: true,
       });
     } else if (items.length < minimumAutoCompleteLength && popup.isOpen) {
@@ -1397,7 +1387,6 @@ 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,11 +12,13 @@ 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 onAutocompleUpdated;
+
+  await onPopUpOpen;
 
   const expectedCommands = [":help", ":screenshot"];
   is(
@@ -25,7 +27,7 @@ add_task(async function() {
     "popup contains expected commands"
   );
 
-  onAutocompleUpdated = jsterm.once("autocomplete-updated");
+  let onAutocompleUpdated = jsterm.once("autocomplete-updated");
   EventUtils.sendString("s");
   await onAutocompleUpdated;
   checkInputCompletionValue(

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

@@ -27,15 +27,19 @@ 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");
-  let onUpdated = jsterm.once("autocomplete-updated");
+  onPopUpOpen = popup.once("popup-opened");
   EventUtils.synthesizeKey(" ", { ctrlKey: true });
-  await onUpdated;
+  await onPopUpOpen;
 
   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,9 +85,9 @@ add_task(async function() {
   info(
     "Type a space, then backspace and ensure the autocomplete popup is displayed"
   );
-  onPopUpOpen = autocompletePopup.once("popup-opened");
+  let onAutocompleteUpdate = jsterm.once("autocomplete-updated");
   EventUtils.synthesizeKey(" ");
-  await onPopUpOpen;
+  await onAutocompleteUpdate;
   is(autocompletePopup.isOpen, true, "Autocomplete popup is still opened");
   is(
     getAutocompletePopupLabels(autocompletePopup).join("-"),
@@ -95,9 +95,9 @@ add_task(async function() {
     "popup has expected items"
   );
 
-  onPopUpOpen = autocompletePopup.once("popup-opened");
+  onAutocompleteUpdate = jsterm.once("autocomplete-updated");
   EventUtils.synthesizeKey("KEY_Backspace");
-  await onPopUpOpen;
+  await onAutocompleteUpdate;
   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"`);
-  let onAutocompleteUpdated = jsterm.once("autocomplete-updated");
+  const onPopUpOpen = popup.once("popup-opened");
   EventUtils.sendString("null", hud.iframeWindow);
-  await onAutocompleteUpdated;
+  await onPopUpOpen;
   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."`);
-  onAutocompleteUpdated = jsterm.once("autocomplete-updated");
+  let 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);
 
-    let onPopupUpdate = jsterm.once("autocomplete-updated");
+    const onPopUpOpen = autocompletePopup.once("popup-opened");
     EventUtils.sendString(test.initialInput);
-    await onPopupUpdate;
+    await onPopUpOpen;
 
     is(
       getAutocompletePopupLabels(autocompletePopup).join("|"),
@@ -107,7 +107,7 @@ add_task(async function() {
       expectedItems,
       expectedCompletionText,
     } of test.sequence) {
-      onPopupUpdate = jsterm.once("autocomplete-updated");
+      const onPopupUpdate = jsterm.once("autocomplete-updated");
       EventUtils.sendString(char);
       await onPopupUpdate;
 

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

@@ -25,7 +25,9 @@ add_task(async function() {
     checkInputValueAndCursorPosition(hud, expected, assertionInfo);
 
   info("Check that lowercased input is case-insensitive");
-  await setInputValueForAutocompletion(hud, "foob");
+  let onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("foob");
+  await onPopUpOpen;
 
   is(
     getAutocompletePopupLabels(autocompletePopup).join(" - "),
@@ -58,10 +60,9 @@ add_task(async function() {
   checkInputCompletionValue(hud, "", "completeNode is empty");
 
   info("Check that the popup is displayed with only 1 matching item");
-  onAutoCompleteUpdated = jsterm.once("autocomplete-updated");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
   EventUtils.sendString(".f");
-  await onAutoCompleteUpdated;
-  ok(autocompletePopup.isOpen, "autocomplete popup is open");
+  await onPopUpOpen;
 
   // Here we want to match "Foo", and since the completion text will only be "oo", we want
   // to display the popup so the user knows that we are matching "Foo" and not "foo".
@@ -80,8 +81,12 @@ add_task(async function() {
   checkInput("fooBar.Foo|", "The input was completed with the correct casing");
   checkInputCompletionValue(hud, "", "completeNode is empty");
 
+  setInputValue(hud, "");
+
   info("Check that Javascript keywords are displayed first");
-  await setInputValueForAutocompletion(hud, "func");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("func");
+  await onPopUpOpen;
 
   is(
     getAutocompletePopupLabels(autocompletePopup).join(" - "),
@@ -96,8 +101,12 @@ add_task(async function() {
   checkInput("function|", "The input was completed as expected");
   checkInputCompletionValue(hud, "", "completeNode is empty");
 
+  setInputValue(hud, "");
+
   info("Check that filtering the cache works like on the server");
-  await setInputValueForAutocompletion(hud, "fooBar.");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("fooBar.");
+  await onPopUpOpen;
   is(
     getAutocompletePopupLabels(autocompletePopup).join(" - "),
     "test - Foo - Test - TEST",

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

@@ -21,7 +21,9 @@ add_task(async function() {
   const { autocompletePopup } = jsterm;
 
   info("Check that filtering the cache works like on the server");
-  await setInputValueForAutocompletion(hud, "x.");
+  const onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.");
+  await onPopUpOpen;
   is(
     getAutocompletePopupLabels(autocompletePopup).join(" - "),
     "foo - foO - fOo - fOO",

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

@@ -284,18 +284,16 @@ add_task(async function() {
     false
   );
 
-  // Wait for the autocomplete popup to be displayed so we know the eager evaluation could
-  // have occured.
-  const onPopupOpen = hud.jsterm.autocompletePopup.once("popup-opened");
-  await setInputValueForAutocompletion(hud, "x + y");
-  await onPopupOpen;
-
   is(
     getEagerEvaluationElement(hud),
     null,
     "There's no eager evaluation element"
   );
-  hud.jsterm.autocompletePopup.hidePopup();
+
+  // Wait for the autocomplete popup to be displayed so we know the eager evaluation could
+  // have occured.
+  await setInputValueForAutocompletion(hud, "x + y");
+  ok(true, "Eager evaluation is disabled");
 
   info("Turn on the eager evaluation");
   toggleConsoleSetting(