See the attached screen recording (or bug 1932842) for the specific bug we encountered. What's happening there is... 1. A Feature Callout message is set up to show when the new tab page opens. 2. Very quickly after clicking the new tab button, the mouse is now on top of the new tab, triggering the `TabHoverStart` event. 3. This calls `previewPanel.activate()`, which checks `previewPanel._isDisabled()`, which checks if there are open popups and returns false, because at this point, the callout is still in an asynchronous, comparatively slow process of opening. 4. Therefore, the `TabPreviewPanelTimedFunction` timer starts ticking. 5. After a short delay, the Feature Callout is rendered, which triggers a `popupshowing` event, but it's too late to influence the tab preview panel... 6. After the 500ms tooltip timer is finished ticking, the preview panel opens, even though there is already another popup open. This is because it [only checks](https://searchfox.org/mozilla-central/rev/9f9fa5d2cb72b12ac7e168b7f6ee9820f63291e9/browser/components/tabbrowser/content/tab-hover-preview.mjs#167) `_onDisabled` once: on `previewPanel.activate()`, and not on `TabPreviewPanelTimedFunction._target()`. In other words, it checks if there's an open popup before _setting_ the timer, but it doesn't check for popups when the callback is actually executed. 7. This leaves a 500ms window, between hovering the tab and the timer finishing, in which other popups can open without stopping the preview panel from opening. 8. This is more problematic in this specific case, because both popups have a problem with showing while another popup is showing. The preview panel tries to avoid showing if there's another popup open. The Feature Callout doesn't have this type of check exactly. It's set up to allow showing a callout _on top_ of other popups (say if we wanted to call out a new button in the App Menu, we'd need to layer it on top of the panel). But it has its own `popupshowing` event handler that dismisses the callout if another popup opens while it is open. This is to ensure the callout doesn't end up with other popups awkwardly stacked on top of it. 9. So, the callout is opening within the 500ms window, because both the preview panel and the callout are basically triggered by the same user interaction: clicking the new tab button, and thereby ending up with a new tab directly under the mouse cursor. The preview panel timer gets set. The callout shows. The preview panel timer finishes. Since there's no popup check at this point, the preview panel shows. This causes the callout to dismiss itself. And the user can never see it again, as it's limited to 1 impression, and this unfortunately counts as an impression even though it was visible for just a few hundred milliseconds. This should be pretty simple to fix, just making sure we check for popups _after_ the timer finishes: ```diff diff --git a/browser/components/tabbrowser/content/tab-hover-preview.mjs b/browser/components/tabbrowser/content/tab-hover-preview.mjs index a33dd675b2ec2..8eff0147206d9 100644 --- a/browser/components/tabbrowser/content/tab-hover-preview.mjs +++ b/browser/components/tabbrowser/content/tab-hover-preview.mjs @@ -61,7 +61,9 @@ export default class TabHoverPreviewPanel { this._panelOpener = new TabPreviewPanelTimedFunction( () => { - this._panel.openPopup(this._tab, this.#popupOptions); + if (!this._isDisabled()) { + this._panel.openPopup(this._tab, this.#popupOptions); + } }, this._prefPreviewDelay, ZERO_DELAY_ACTIVATION_TIME, ``` Another, maybe supplementary, option would be to clear the timeout on `popupshowing` events: ```diff @@ -297,6 +299,9 @@ export default class TabHoverPreviewPanel { ev.target !== this._panel && (ev.target.nodeName == "panel" || ev.target.nodeName == "menupopup") ) { + if (setMethod === "add") { + this._panelOpener.clear(); + } this._openPopups[setMethod](ev.target); } }); ``` That's a little more awkward, and I'm not sure it adds any extra benefit. But just figured I'd point it out.
Bug 1941391 Comment 0 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
See the attached screen recording (or bug 1932842) for the specific bug we encountered. What's happening there is... 1. A Feature Callout message is set up to show when the new tab page opens. 2. Very quickly after clicking the new tab button, the mouse is now on top of the new tab, triggering the `TabHoverStart` event. 3. This calls `previewPanel.activate()`, which checks `previewPanel._isDisabled()`, which checks if there are open popups and returns false, because at this point, the callout is still in an asynchronous, comparatively slow process of opening. 4. Therefore, the `TabPreviewPanelTimedFunction` timer starts ticking. 5. After a short delay, the Feature Callout is rendered, which triggers a `popupshowing` event, but it's too late to influence the tab preview panel... 6. After the 500ms tooltip timer is finished ticking, the preview panel opens, even though there is already another popup open. This is because it [only checks](https://searchfox.org/mozilla-central/rev/9f9fa5d2cb72b12ac7e168b7f6ee9820f63291e9/browser/components/tabbrowser/content/tab-hover-preview.mjs#167) `_onDisabled` once: on `previewPanel.activate()`, and not on `TabPreviewPanelTimedFunction._target()`. In other words, it checks if there's an open popup before _setting_ the timer, but it doesn't check for popups when the callback is actually executed. 7. This leaves a 500ms window, between hovering the tab and the timer finishing, in which other popups can open without stopping the preview panel from opening. 8. This is more problematic in this specific case, because both popups have a problem with showing while another popup is showing. The preview panel tries to avoid showing if there's another popup open. The Feature Callout doesn't have this type of check exactly. It's set up to allow showing a callout _on top_ of other popups (say if we wanted to call out a new button in the App Menu, we'd need to layer it on top of the panel). But it has its own `popupshowing` event handler that dismisses the callout if another popup opens while it is open. This is to ensure the callout doesn't end up with other popups awkwardly stacked on top of it. 9. So, the callout is opening within the 500ms window, because both the preview panel and the callout are basically triggered by the same user interaction: clicking the new tab button, and thereby ending up with a new tab directly under the mouse cursor. The preview panel timer gets set. The callout shows. The preview panel timer finishes. Since there's no popup check at this point, the preview panel shows. This causes the callout to dismiss itself. And the user can never see it again, as it's limited to 1 impression, and this unfortunately counts as an impression even though it was visible for just a few hundred milliseconds. This should be pretty simple to fix, just making sure we check for popups _after_ the timer finishes: ```diff @@ -61,7 +61,9 @@ export default class TabHoverPreviewPanel { this._panelOpener = new TabPreviewPanelTimedFunction( () => { - this._panel.openPopup(this._tab, this.#popupOptions); + if (!this._isDisabled()) { + this._panel.openPopup(this._tab, this.#popupOptions); + } }, this._prefPreviewDelay, ZERO_DELAY_ACTIVATION_TIME, ``` Another, maybe supplementary, option would be to clear the timeout on `popupshowing` events: ```diff @@ -297,6 +299,9 @@ export default class TabHoverPreviewPanel { ev.target !== this._panel && (ev.target.nodeName == "panel" || ev.target.nodeName == "menupopup") ) { + if (setMethod === "add") { + this._panelOpener.clear(); + } this._openPopups[setMethod](ev.target); } }); ``` That's a little more awkward, and I'm not sure it adds any extra benefit. But just figured I'd point it out.