1550637 - Fix tooltip hiding when navigating or switching tabs in e10s

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Cameron McCormack (:heycam)]

Sometimes tooltips don't disappear. I think this happens when a tooltip is shown by the foreground tab, and then I switch to another tab. The tooltip stays around, and doesn't disappear until I switch back to the original tab and mouse around the page. I can't reliably reproduce this, but it's happening to me at least once a day.

At the very least we should dismiss tooltips when mousing over them, to avoid them getting stuck on the screen and having to find the originating tab.

I don't know if this is a recent regression but I feel like I've only been seeing it in the last couple of weeks on Nightly.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Cameron McCormack (:heycam) from comment #0)

Sometimes tooltips don't disappear. I think this happens when a tooltip is shown by the foreground tab, and then I switch to another tab. The tooltip stays around, and doesn't disappear until I switch back to the original tab and mouse around the page. I can't reliably reproduce this, but it's happening to me at least once a day.

I believe that you're right on this. For me, I had already switched from a treeherder tab over to a gmail tab, when suddenly a tooltip for a treeherder link appeared in the gmail tab and would not go away until I switched back to the original treeherder tab.

Note that one unique thing about my case was that the tooltip for treeherder did not display until after I had already switched over to gmail.

Comment 2

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Note that one unique thing about my case was that the tooltip for treeherder did not display until after I had already switched over to gmail.

That sounds familiar, when this happens for me I don't recall having seen that tooltip in its proper location. I wonder if some part of the show command is getting stuck somewhere, and doesn't get processed until after the corresponding hide.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:Dolske, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Ryan Wu]

When I move the mosue pointer to a link very quickly and then almost immediately switching to another tab, it would happens. Esspecially when switching tabs with keyboard shortcut or mouse gesture.

Comment 5

[Dave Camp (:dcamp)]

I'm seeing this on Windows, in case that's new information.

Comment 6

[Mike de Boer [:mikedeboer]]

Hi Dão, can you start taking a look at this issue to see if you can a) reproduce it yourself and b) see where the issue might be?
I seem to recall that there's a Windows widget issue about not hiding tooltips, so I'm needinfo-ing Neil as well.
Thanks in advance for your help!

Comment 7

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Dave Camp (:dcamp) from comment #5)

I'm seeing this on Windows, in case that's new information.

And I've been seeing this on Linux, so this may be a cross-platform bug.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

I am pretty sure this is a cross-platform issue. I saw this issue on my Linux box, and I also saw this in mconley's "The Joy of Coding" on his mac. :)

Comment 10

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #6)

Hi Dão, can you start taking a look at this issue to see if you can a) reproduce it yourself and b) see where the issue might be?

I've seen the bug but I have no steps to reproduce it.

layout/xul/nsXULTooltipListener.cpp is responsible for hiding tooltips, I think, so that's where I'd start looking. Maybe we don't get the necessary mouseout event? I'll move this to Core::XUL for now.

Maybe we also show the tooltip at the wrong time to begin with, but that's just a guess (we need steps to reproduce). Could be related to AsyncTabSwitcher.jsm?

As a band-aid fix, I guess front-end code could manually hide the aHTMLTooltip popup when switching tabs. Generally hiding tooltips when mousing over them seems like a good idea too.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 12

[:Gijs (he/him)]

(In reply to Dão Gottwald [::dao] from comment #10)

As a band-aid fix, I guess front-end code could manually hide the aHTMLTooltip popup when switching tabs. Generally hiding tooltips when mousing over them seems like a good idea too.

It turns out there is frontend code for this today, at https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/browser/base/content/browser.js#5013-5029 , which hasn't worked since we turned e10s on, because tooltip.triggerNode is always null. We could probably just fix that code to indiscriminately call hidePopup() if we get a toplevel onlocationchange and the popup is open? Bit wallpaper-y if we think the responsibility should really be somewhere else, but it should work...

Comment 13

[:Gijs (he/him)]

Attachment: Bug 1550637 - fix tooltip hiding when navigating or switching tabs in e10s, r?dao

Comment 14

[Neil Deakin]

The popup is opened at https://searchfox.org/mozilla-central/source/browser/base/content/browser.js#4877 with a null event. We could either change the API to allow a node to be passed, or just create a dummy event with the browser as the target. Then you should be able to compare the triggerNode to the browser being closed.

Comment 15

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #14)

The popup is opened at https://searchfox.org/mozilla-central/source/browser/base/content/browser.js#4877 with a null event. We could either change the API to allow a node to be passed, or just create a dummy event with the browser as the target. Then you should be able to compare the triggerNode to the browser being closed.

I think in the past, the trigger node would have been the node in the content document that had a title attribute or otherwise triggered the tooltip. This is presumably why there's a loop trying to figure out whether the tooltip's trigger node is in a subframe that got navigated and all that... That can't happen in e10s.

I'm not sure it's useful to add more checks to this (somewhat hot) onLocationChange code to make sure the tooltip is associated with the "right" browser. In theory, if you moused over a link in the sidebar or a (non-tabspecific) panel, and that reused this tooltip, and then you navigated the main browser window with the keyboard (or on a timer or whatever) then the tooltip would disappear, but that doesn't really seem like a usecase worth worrying too much about here.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a67659b3fe6
fix tooltip hiding when navigating or switching tabs in e10s, r=dao

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8a67659b3fe6

Comment 18

[Dão Gottwald [:dao]]

While this patch didn't fix the problem entirely (see bug 1539770, bug 1556143), it might still be worth uplifting. Gijs, what do you think?

Comment 19

[:Gijs (he/him)]

So coincidentally I was looking at some of the more recent reports.

I think this code might be even more broken than I thought, and also that xultooltiplistener isn't involved at all.

Specifically, if you look at the xultooltiplistener implementation, 2 things become clear:

1. it's meant to run in the process that includes the actual xul popup manager.
2. it's meant to run in the process that includes the dom nodes that want popups.

These 2 things are never true at the same time in e10s (for tabbed browsers).

AFAICT for e10s tabs, we use a different tooltip. Specifically, working backwards through the stack I found:

https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/browser/base/content/browser.js#4773,4781
https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/dom/ipc/BrowserParent.cpp#1940,1958
https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/dom/ipc/BrowserChild.cpp#3120,3125
https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/docshell/base/nsDocShellTreeOwner.cpp#1157,1168
https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/docshell/base/nsDocShellTreeOwner.cpp#1275
https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/docshell/base/nsDocShellTreeOwner.cpp#1207-1218

So there's a few things here. I think we should:

• potentially revert the change from here but make sure that the work for aHTMLTooltip is only done for non-remote browsers.
• maybe do something similar for the remoteBrowserTooltip that this patch was trying to do for aHTMLTooltip
• make the code in nsDocShellTreeOwner at https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/docshell/base/nsDocShellTreeOwner.cpp#1232-1246 return early if the docshell's isActive prop is not true.

Dão, does that look sane to you? Am I missing something else?

Comment 20

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #19)

So there's a few things here. I think we should:

• potentially revert the change from here but make sure that the work for aHTMLTooltip is only done for non-remote browsers.
• maybe do something similar for the remoteBrowserTooltip that this patch was trying to do for aHTMLTooltip
• make the code in nsDocShellTreeOwner at https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/docshell/base/nsDocShellTreeOwner.cpp#1232-1246 return early if the docshell's isActive prop is not true.

Oh, and:

• cancel the timer in docshelltreeowner when the top docshell is no longer active.

Comment 21

[Dão Gottwald [:dao]]

Sounds good to me. Want to back out and reopen? It might also make sense to make this a meta bug and file separate bugs for the different fixes.

Comment 23

[:Gijs (he/him)]

(Requested a backout in #sheriffs, need to go have lunch and more meetings, but will try to get back to this later today)

Comment 24

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0112218e9a5a
Backed out changeset 8a67659b3fe6 by dev's request

Comment 25

[:Gijs (he/him)]

Attachment: Bug 1550637 - hide tooltip when the docshell navigate, don't show tooltips if the docshell is no longer active, r?nika!,dao!

Comment 26

[:Gijs (he/him)]

Just to clarify the patch here a bit:

(In reply to :Gijs (he/him) from comment #19 and comment #20)

So there's a few things here. I think we should:

• potentially revert the change from here but make sure that the work for aHTMLTooltip is only done for non-remote browsers.

onLocationChange doesn't get a reference to the previous browser, and this doesn't seem worth changing just for the edgecase of non-remote browsers so I've left it for now.

• maybe do something similar for the remoteBrowserTooltip that this patch was trying to do for aHTMLTooltip

This is in the patch, in both tabbrowser and the docshell code, because tab switches don't get to the docshell code, and because location changes in browsers that aren't in the tabbrowser should also work correctly.

• make the code in nsDocShellTreeOwner at https://searchfox.org/mozilla-central/rev/8ed8474757695cdae047150a0eaf94a5f1c96dbe/docshell/base/nsDocShellTreeOwner.cpp#1232-1246 return early if the docshell's isActive prop is not true.

As is this.

• cancel the timer in docshelltreeowner when the top docshell is no longer active.

This isn't, because I also didn't see a good way for the docshelltreeowner to "know" when the docshell's active state changes.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80128e172025
hide tooltip when the docshell navigate, don't show tooltips if the docshell is no longer active, r=nika,dao

Comment 28

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/80128e172025

Comment 29

[:Gijs (he/him)]

Comment on attachment 9073132 [details]
Bug 1550637 - hide tooltip when the docshell navigate, don't show tooltips if the docshell is no longer active, r?nika!,dao!

Beta/Release Uplift Approval Request

• User impact if declined: Tooltips stay on the screen when they shouldn't
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See https://bugzilla.mozilla.org/show_bug.cgi?id=1564540#c0 and https://bugzilla.mozilla.org/show_bug.cgi?id=1539770#c0
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Fairly small changes, it's early in the beta cycle

(nightly verification was in https://bugzilla.mozilla.org/show_bug.cgi?id=1564540#c9 )

• String changes made/needed: nope

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9073132 [details]
Bug 1550637 - hide tooltip when the docshell navigate, don't show tooltips if the docshell is no longer active, r?nika!,dao!

Fix for tooltips sticking around when they shouldn't. Approved for 69.0b6.

Comment 31

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/195538371838

Comment 32

[Hani Yacoub, Desktop QA]

Attachment: tooltip.png

I'm having difficulties reproducing the bug. And I don't see any differences between Firefox Nightly 70.0a1 (2019-07-09) and the latest Firefox Nightly 70.0a1 (2019-07-18).

I followed the exact steps for "https://bugzilla.mozilla.org/show_bug.cgi?id=1564540#c0".

After switching back and forth between the 2 tabs the tooltip remains fixed at the top of the page like in the screenshot attached.
After clicking on the tooltip it disappears, the exact behavior is happening on both builds.

Are there any other steps I'm not aware of or is the bug intermittent?
Thanks.

Comment 33

[Revertron]

I've experienced this bug every day for some months, but for about 2 days I can't reproduce it, even when I try on purpose.

Comment 34

[:Gijs (he/him)]

(In reply to Hani Yacoub from comment #32)

After switching back and forth between the 2 tabs the tooltip remains fixed at the top of the page like in the screenshot attached.

I don't understand what exact steps you're following here. Can you please provide detailed steps of what you're doing and/or a screencast that includes the tooltip first appearing?

Comment 35

[Hani Yacoub, Desktop QA]

https://www.screencast.com/t/Bp0Sc4itia6
I attached a screencast with the exact steps I followed.
The same behavior is happening on Firefox Nightly 68.0a1 (2019-03-28), Firefox Nightly 70.0a1 (2019-07-09) and on the latest Firefox Nightly 70.0a1 (2019-07-22).

Please let me know if I'm doing something wrong. Thanks

Comment 36

[:Gijs (he/him)]

(In reply to Hani Yacoub from comment #35)

https://www.screencast.com/t/Bp0Sc4itia6
I attached a screencast with the exact steps I followed.
The same behavior is happening on Firefox Nightly 68.0a1 (2019-03-28), Firefox Nightly 70.0a1 (2019-07-09) and on the latest Firefox Nightly 70.0a1 (2019-07-22).

Please let me know if I'm doing something wrong. Thanks

OK, this is interesting but it's not the bug that is being fixed here. AFAICT from your screencast, the issue you're seeing is that, as long as you don't move the mouse cursor while the tab with the tooltip is selected, that tooltip appears and stays up, even if you scroll, and then it goes away as soon as you either switch tabs, or (after going back to the tab and letting the tooltip reappear) click or move the mouse. Making scrolling also hide the tooltip is bug 1236458.

The issue in this bug is about the tooltip staying on screen even when you switch tabs (so tooltip from tab A showing up while tab B is selected), even when you then (after switching tabs) move your mouse or use the keyboard (besides shortcuts). In https://bugzilla.mozilla.org/show_bug.cgi?id=1564540#c0 , the important part is: "and see that sometimes hyperlink tooltip from old tab is visible in new tab". This is not the case in your screencast.

Comment 37

[Alice0775 White]

(In reply to Hani Yacoub from comment #35)

https://www.screencast.com/t/Bp0Sc4itia6
I attached a screencast with the exact steps I followed.
The same behavior is happening on Firefox Nightly 68.0a1 (2019-03-28), Firefox Nightly 70.0a1 (2019-07-09) and on the latest Firefox Nightly 70.0a1 (2019-07-22).

Please let me know if I'm doing something wrong. Thanks

I have filed a new bug(Bug 1568154) about the tooltip persistence.
But, I think this is BMO new theme BUG because I can also reproduce the issue on Chrome Dev.

Comment 38

[Hani Yacoub, Desktop QA]

I totally understand the problem here but I can't reproduce the issue and I'm not sure if it's intermittent or maybe because the legacy view of bugzilla has been disabled.

Since the steps are the same as bug "https://bugzilla.mozilla.org/show_bug.cgi?id=1564540#c0" and the bug has been already verified on Nightly and Beta by Virtual_ManPL, couldn't we mark this one also as verified?

Comment 39

[:Gijs (he/him)]

WFM.