Bug 1349416 - Hide HTMLTooltip on mouseup rather than on click;r=birtles
☠☠ backed out by fe0afb180042 ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 03 Aug 2018 07:05:43 +0000
changeset 826505 c4f8eccf14dcdc14dee9aa597c8a42ed31c75555
parent 826504 b7e80d3a6810f1b98fc0703a0772b6657dfb1ed9
child 826506 ec5ee2990e219b1cd1c95053a21878d69ddad52c
push id118355
push userwisniewskit@gmail.com
push dateSat, 04 Aug 2018 00:24:59 +0000
reviewersbirtles
bugs1349416
milestone63.0a1
Bug 1349416 - Hide HTMLTooltip on mouseup rather than on click;r=birtles A click event is only fired if the element on which mousedown and mouseup happened are the same. If this is not the case, the HTMLTooltip will not be able to hide itself. Listening to mouseup makes the behavior more consistent but forces the hide() method to be always asynchronous, while before it was only asynchronous for tooltips using the XULWrapper. Depends on D2660 Differential Revision: https://phabricator.services.mozilla.com/D2661
devtools/client/shared/widgets/tooltip/HTMLTooltip.js
--- a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
@@ -325,16 +325,17 @@ function HTMLTooltip(toolboxDoc, {
 
   // The top window is used to attach click event listeners to close the tooltip if the
   // user clicks on the content page.
   this.topWindow = this._getTopWindow();
 
   this._position = null;
 
   this._onClick = this._onClick.bind(this);
+  this._onMouseup = this._onMouseup.bind(this);
   this._onXulPanelHidden = this._onXulPanelHidden.bind(this);
 
   this._toggle = new TooltipToggle(this);
   this.startTogglingOnHover = this._toggle.start.bind(this._toggle);
   this.stopTogglingOnHover = this._toggle.stop.bind(this._toggle);
 
   this.container = this._createContainer();
 
@@ -451,16 +452,17 @@ HTMLTooltip.prototype = {
     this.doc.defaultView.clearTimeout(this.attachEventsTimer);
     this.attachEventsTimer = this.doc.defaultView.setTimeout(() => {
       if (this.autofocus) {
         this.focus();
       }
       // 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);
   },
 
   /**
    * Recalculate the dimensions and position of the tooltip in response to
    * changes to its content.
    *
@@ -696,45 +698,55 @@ HTMLTooltip.prototype = {
    */
   async hide() {
     this.doc.defaultView.clearTimeout(this.attachEventsTimer);
     if (!this.isVisible()) {
       this.emit("hidden");
       return;
     }
 
-    this.topWindow.removeEventListener("click", this._onClick, true);
+    // Wait for potential click events before removing listeners.
+    await new Promise(resolve => this.topWindow.setTimeout(resolve, 0));
+
+    this.removeEventListeners();
+
     this.container.classList.remove("tooltip-visible");
     if (this.useXulWrapper) {
       await this._hideXulWrapper();
     }
 
     this.emit("hidden");
 
     const tooltipHasFocus = this.container.contains(this.doc.activeElement);
     if (tooltipHasFocus && this._focusedElement) {
       this._focusedElement.focus();
       this._focusedElement = null;
     }
   },
 
+  removeEventListeners: function() {
+    this.topWindow.removeEventListener("click", this._onClick, true);
+    this.topWindow.removeEventListener("mouseup", this._onMouseup, true);
+  },
+
   /**
    * Check if the tooltip is currently displayed.
    * @return {Boolean} true if the tooltip is visible
    */
   isVisible: function() {
     return this.container.classList.contains("tooltip-visible");
   },
 
   /**
    * Destroy the tooltip instance. Hide the tooltip if displayed, remove the
    * tooltip container from the document.
    */
   destroy: function() {
     this.hide();
+    this.removeEventListeners();
     this.container.remove();
     if (this.xulPanelWrapper) {
       this.xulPanelWrapper.remove();
     }
     this._toggle.destroy();
   },
 
   _createContainer: function() {
@@ -761,27 +773,40 @@ HTMLTooltip.prototype = {
     return container;
   },
 
   _onClick: function(e) {
     if (this._isInTooltipContainer(e.target)) {
       return;
     }
 
+    if (this.consumeOutsideClicks && e.button === 0) {
+      // Consume only left click events (button === 0).
+      e.preventDefault();
+      e.stopPropagation();
+    }
+  },
+
+  /**
+   * Hide the tooltip on mouseup rather than on click because the surrounding markup
+   * may change on mousedown in a way that prevents a "click" event from being fired.
+   * If the element that received the mousedown and the mouseup are different, click
+   * will not be fired.
+   */
+  _onMouseup: function(e) {
+    if (this._isInTooltipContainer(e.target)) {
+      return;
+    }
+
     // If the disable autohide setting is in effect, ignore.
     if (Services.prefs.getBoolPref("ui.popup.disable_autohide", false)) {
       return;
     }
 
     this.hide();
-    if (this.consumeOutsideClicks && e.button === 0) {
-      // Consume only left click events (button === 0).
-      e.preventDefault();
-      e.stopPropagation();
-    }
   },
 
   _isInTooltipContainer: function(node) {
     // Check if the target is the tooltip arrow.
     if (this.arrow && this.arrow === node) {
       return true;
     }