Closed Bug 1083365 Opened 10 years ago Closed 9 years ago

[e10s] Tooltips don't always go away

Categories

(Firefox :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 40
Tracking Status
e10s m6+ ---
firefox40 --- fixed

People

(Reporter: mccr8, Assigned: handyman)

References

Details

Attachments

(2 files, 3 obsolete files)

On Zimbra, hover over "View" on the right side until the tooltip appears.  Then without clicking or anything move the mouse to the right so it is no longer over the browser.  The tooltip briefly disappears, but then reappears and does not go away.  Then you can move the mouse back to the browser and do things like click on tabs and write text and the tooltip persists.  It only clears when you hover over something else that requires a tooltip.

I can't reproduce in a non-e10s window.  I also can't reproduce on all webpage tool tips.  For instance, I can't reproduce on XKCD.

I'm guessing this is a regression from bug 1061147.
tracking-e10s: --- → ?
Can't reproduce this myself.
I can also reproduce this on Windows (in addition to OSX).  It does not seem to happen for the tooltips from hovering over the browser tabs, and it only seems to happen when going from hovering over the browser to not hovering over the browser.  I can't reproduce it every time, and I'm not sure what the difference is.
I see this a couple times a day, but I've never been able to force it to happen on purpose.
I have a similar problem in Windows. I can reproduce with or without e10s. Attached is a repro video made in a non-e10s window.

gecko.buildID = 20141118030201
gecko.mstone = 36.0a1

I have yahoo mail open in one tab and if that tab is in focus and I move my mouse pointer from that window down to the taskbar (at the bottom of the screen) occasionally I will see the tooltip for some subject even after I have already switched windows.

When I move the mouse normally it is way less likely to happen than when I moved it slow like in the video.
Assignee: nobody → davidp99
Hi David. You mentioned in the meeting that you were out of M6 bugs, so I wanted to make sure this one was still on your radar.
Flags: needinfo?(davidp99)
The hope was that bug 1018639 would be the solution here but... its not.  So I'm circling back to this.  I'm on an MBP -- here are reliable STR:

* Open an e10s window.  Resize to maximize (not fullscreen)
* Go to a bug in bugzilla with a decent amount of discussion
* Scroll so that someone's name is at the very bottom of the window -- the post's author's name (which exposes a tooltip when hovered)
* Hover over the name to expose the tooltip
* Move the mouse down, outside of the screen

Expected result: 
tooltip vanishes

Actual result:
Tooltip stays, even if you expose another window and move the cursor around in it.  The tooltip goes away when you return to the browser or when you mouseover it.
Flags: needinfo?(davidp99)
This block of code:

https://mxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#594

is converting our NS_MOUSE_EXIT into an NS_MOUSE_MOVE before the handler in the ChromeTooltipListener has a chance to see it.  
(That code dates to before hg@1.)
The patch issues a “MozTabExit” message to the document when this happens.  The ChromeTooltipListener is updated to handle that message as well.

I had considered having the TabParent or TabChild take a shortcut and respond by hiding the tooltip themselves but the ChromeTooltipListener operates as kind of a state machine and that just confused it.

Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cb550fdd5b9
Attachment #8593476 - Flags: review?(neil)
Comment on attachment 8593476 [details] [diff] [review]
Create a MozTabExit message to notify listeners of silenced mouseout

Wrong Neil, I think; I only know about nsXULTooltipListener.
Attachment #8593476 - Flags: review?(neil) → review?(enndeakin)
Comment on attachment 8593476 [details] [diff] [review]
Create a MozTabExit message to notify listeners of silenced mouseout

>diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
>--- a/dom/events/EventStateManager.cpp
>+++ b/dom/events/EventStateManager.cpp
>@@ -595,16 +595,19 @@ EventStateManager::PreHandleEvent(nsPres
>     // really an exit --- we may have traversed widget boundaries but
>     // we're still in our toplevel window.
>     if (mouseEvent->exit != WidgetMouseEvent::eTopLevel) {
>       // Treat it as a synthetic move so we don't generate spurious
>       // "exit" or "move" events.  Any necessary "out" or "over" events
>       // will be generated by GenerateMouseEnterExit
>       mouseEvent->message = NS_MOUSE_MOVE;
>       mouseEvent->reason = WidgetMouseEvent::eSynthesized;
>+      nsContentUtils::DispatchChromeEvent(mDocument, static_cast<nsIDocument*>(mDocument),
>+                                          NS_LITERAL_STRING("MozTabExit"),
>+                                          false, false);
>       // then fall through...

I don't think this is right. In fact, my testing shows that we don't seem to be sending mouseout/mouseleave at all on a child page when the mouse leaves it and we should be doing that. We should probably just not be doing this whole block for a child process.
Attachment #8593476 - Flags: review?(enndeakin) → review-
I'm not sure I understand what you are asking for here.  Is this what you were looking for (literally just blocking out the code in the content proc)?

The various mouse events are kind of a mystery to me.  There may be bugs in the NS_MOUSE_EXIT/NS_MOUSE_EXIT_SYNTH/NS_MOUSELEAVE behavior but I don't know exactly what their semantics are supposed to be.  E.g. from searching the code, I had thought NS_MOUSE_EXIT(_SYNTH) was for exiting a widget (in part because of the ESM code that caused this bug -- I still cant guess what it's for).  But I suppose that's a different bug.
Attachment #8593476 - Attachment is obsolete: true
Flags: needinfo?(enndeakin)
Comment on attachment 8594982 [details] [diff] [review]
Do not filter NS_MOUSE_EXIT events in content processes

Maybe this will do...
Flags: needinfo?(enndeakin)
Attachment #8594982 - Flags: review?(enndeakin)
(In reply to David Parks [:handyman] from comment #10)
> Created attachment 8594982 [details] [diff] [review]
> Do not filter NS_MOUSE_EXIT events in content processes
> 
> I'm not sure I understand what you are asking for here.  Is this what you
> were looking for (literally just blocking out the code in the content proc)?
> 
> The various mouse events are kind of a mystery to me.  There may be bugs in
> the NS_MOUSE_EXIT/NS_MOUSE_EXIT_SYNTH/NS_MOUSELEAVE behavior but I don't
> know exactly what their semantics are supposed to be.  E.g. from searching
> the code, I had thought NS_MOUSE_EXIT(_SYNTH) was for exiting a widget (in
> part because of the ESM code that caused this bug -- I still cant guess what
> it's for).  But I suppose that's a different bug.

NS_MOUSE_EXIT means we left a widget boundary. NS_MOUSE_EXIT_SYNTH means the DOM mouseout event firing as the mouse left an element. I filed bug 1158425 on renaming these constants to be clearer of their meaning.
Comment on attachment 8594982 [details] [diff] [review]
Do not filter NS_MOUSE_EXIT events in content processes

I'll just give a feedback+, as I'm not sure about the pointer event handling in the next block. Probably best to ask smaug about it.
Attachment #8594982 - Flags: review?(enndeakin) → feedback+
Comment on attachment 8594982 [details] [diff] [review]
Do not filter NS_MOUSE_EXIT events in content processes

Giving smaug a chance to look at this.

Judging by the info Neil provided, this sounds like a reasonable thing to do.  I'm not 100% sure of the purpose of this special-case behavior in the ESM but I think its due to e10s partitioning the dom across processes and the event being re-generated separately in the content proc (which does happen).  I suppose that makes sense and it would definitely imply that this code shouldn't be applied in content (even if we were worried about the '>1 content processes' case).
Attachment #8594982 - Flags: review?(bugs)
Comment on attachment 8594982 [details] [diff] [review]
Do not filter NS_MOUSE_EXIT events in content processes

Since I have no idea why this patch doesn't regress non-e10s event handling, r-.
Does the patch basically just make parent to do aEvent->message = 0; break; which then leads us to send NS_MOUSE_EXIT to child process?
The patch just happens to change the event handling on the parent side too even if there are no child processes.

Sounds like we do want PostHandleEvent for NS_MOUSE_EXIT, so that it gets processed. 
So, tweak http://mxr.mozilla.org/mozilla-central/source/widget/WidgetEventImpl.cpp#221 so that we don't dispatch DOM event for NS_MOUSE_EXIT and then drop aEvent->message = 0; or something like that.
Attachment #8594982 - Flags: review?(bugs) → review-
Hmm, I missed that one 'else' somehow.
smaug's review found that there is already code to skip the ESM code -- it just needed to be activated by setting the exit flag in the mouseexit event to eTopLevel.

Tests coming in:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a43386f3064d
Attachment #8594982 - Attachment is obsolete: true
Attachment #8598971 - Flags: review?(bugs)
Comment on attachment 8598971 [details] [diff] [review]
Flag NS_MOUSE_EXIT messages as eTopLevel when sending to child process.

Why not set that when the event is created in ESM? Do that please.
Attachment #8598971 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/9d020c85a4ec
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 40
Depends on: 1247420
Blocks: 1247420
No longer depends on: 1247420
No longer blocks: 1247420
Depends on: 1247420
You need to log in before you can comment on or make changes to this bug.