Bug 1616847 - When xulWrapper is visible move it instead of trying to show it again. r=jdescottes.

If the XUL wrapper was opened and a consumer called show again,
the panel wouldn't update its position. This can be fixed by checking the
state of the element, and if it's open move the panel using `_moveXulWrapperTo`.

With this change in place, we can remove the updateContainerBounds methods,
as it has the same arguments and is only used in one place.

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

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

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

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

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.updateContainerBounds(box1, { position: "top" });
+  tooltip.show(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,21 +9,18 @@
  */
 
 /**
- * Display an existing HTMLTooltip on an anchor. After the tooltip "shown"
- * event has been fired a reflow will be triggered.
+ * Display an existing HTMLTooltip on an anchor and properly wait for the popup to be
+ * repainted.
  *
  * @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 "shown" has been fired, reflow
- *         and repaint done.
+ * @return {Promise} promise that resolves when reflow and repaint are done.
  */
 async function showTooltip(tooltip, anchor, { position, x, y } = {}) {
-  const onShown = tooltip.once("shown");
-  tooltip.show(anchor, { position, x, y });
-  await onShown;
+  await tooltip.show(anchor, { position, x, y });
   return waitForReflow(tooltip);
 }
 

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

@@ -464,8 +464,9 @@ HTMLTooltip.prototype = {
   },
 
   /**
-   * 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.
+   * 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.
    *
    * @param {Element} anchor
    *        The reference element with which the tooltip should be aligned
@@ -483,27 +484,50 @@ HTMLTooltip.prototype = {
    */
   async show(anchor, options) {
     const { left, top } = this._updateContainerBounds(anchor, options);
+    const isTooltipVisible = this.isVisible();
 
     if (this.useXulWrapper) {
-      await this._showXulWrapperAt(left, top);
+      if (!isTooltipVisible) {
+        await this._showXulWrapperAt(left, top);
+      } else {
+        this._moveXulWrapperTo(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;
 
-    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);
+    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");
   },
 
   startTogglingOnHover(baseNode, targetNodeCb, options) {
@@ -514,27 +538,6 @@ 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);
@@ -794,7 +797,10 @@ HTMLTooltip.prototype = {
       return;
     }
 
-    this.doc.defaultView.clearTimeout(this.attachEventsTimer);
+    if (this.doc && this.doc.defaultView) {
+      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) {