596600 - Window-level mouse exit events fail to NotifyMouseOut when sent to a different ESM than the currently hovered one

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

See Bug 593959

Hover works in a different way than :active. Only one document can be
in :active, but :hover works in nested subdocuments.

bug 130078 may have just exposed the :hover handling we have for web content to
also chrome-content.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Markus, any chance you could write a testcase.

<xul:browser> and <html:iframe> should work nowadays the same way.

Comment 2

[Markus Stange [:mstange]]

Thinking about this again, I've realized that this doesn't have to do much with bug 593959 at all, and not with releasing the mouse either. I'll update the title.

Comment 3

[Markus Stange [:mstange]]

Repeating the steps to reproduce from bug 593959 comment 3:
1. Be on a Mac.
2. Make sure "Tabs on Top" is *not* checked.
3. Deactivate / unfocus the Firefox window.
4. Move the mouse to the title bar.
5. Move the mouse down into the content area, passing over a tab on the way down. 

Most of the time, the tab you passed over will now still be in :hover state, and a tooltip opens that only goes away when you move your mouse over it.

So how is this related to the bug summary:

Mac Widget sends a window-level mouse exit event when the mouse moves into the content area because the <browser> has clickthrough="never" set on it. However, this event is sent at the point where the mouse is *after it has already exited chrome*, so it's now over content and the event is sent to the content ESM. The content ESM gets confused because the hovered element is stored in the chrome ESM, so NotifyMouseOut doesn't do anything in the content ESM.

Comment 4

[Markus Stange [:mstange]]

I don't know if it's possible to create a testcase for this. The clickthrough attribute only works on XUL (which is now blocked) and it only has effect on Mac.

Comment 5

[Markus Stange [:mstange]]

Attachment: v1

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 505705 [details] [diff] [review]
v1

Why does mac send window level mouse exit when the mouse isn't exiting
window?

Does this affect badly to performance if we have very deep document hierarchy.
Say 100 nested iframes or something. Do we need to call the parent
ESM both for NS_MOUSE_MOVE and NS_MOUSE_EXIT?
The latter one doesn't happen too often, but the first one is very common.


The patch is not safe. parentESM may be deleted while handling GenerateMouseEnterExit(). You should keep parentShell->GetPresContext()
in a strong pointer.

Comment 7

[Markus Stange [:mstange]]

Attachment: alternative patch

(In reply to comment #6)
> Comment on attachment 505705 [details] [diff] [review]
> v1
> 
> Why does mac send window level mouse exit when the mouse isn't exiting
> window?

Because we want to behave as if the mouse wasn't over the window, e.g. we want to clear :hover states and send mouse out events.

There are two cases we send window-level mouse exit events even though technically the mouse is still over the window:
Case 1 is when the window is inactive and the user moves the mouse from an element that allows click-through to an element that doesn't. Both of these elements can be inside the same window. For example, click-through is allowed over Firefox chrome but blocked over content.
Case 2 is when the window starts out active with the mouse over a click-through blocked area (e.g. Firefox content area), and then is made inactive (for example using Cmd+Tab). When deactivating the window, all :hover states in the window should be cleared, so we send a window-level mouse exit event.

Here's a patch that uses a different approach from the last one: Instead of first looking for the ESM under the mouse and then traversing back up, we just always use the root ESM for window-level mouse exit events in PresShell::HandleEvent.

I'll send this to the tryserver in a minute.

Comment 8

[Markus Stange [:mstange]]

Tryserver is green.

Comment 9

[Markus Stange [:mstange]]

The test uses enablePrivilege, so I guess I need to convert it to a chrome test?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Yes, or wait ted's patch to enable DOMWindowUtils in SpecialPowers

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 517734 [details] [diff] [review]
alternative patch

Waiting for updated tests.

Comment 12

[Markus Stange [:mstange]]

Attachment: without enablePrivilege

Duh, the test already was a chrome test, so the enablePrivilege call was completely unnecessary. I've removed it.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

So what if you have a window which doesn't have any borders and that
window has content page. Then you hover over some content element
(which is right at the edge of the page) and move mouse out from the window.
Does the hover get cleared?

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Waiting answer for the question above.
(I don't have a macbook with me atm, so can't test.)

Comment 15

[Markus Stange [:mstange]]

Attachment: also test :hover clearance

(In reply to comment #13)
> So what if you have a window which doesn't have any borders and that
> window has content page. Then you hover over some content element
> (which is right at the edge of the page) and move mouse out from the window.
> Does the hover get cleared?

Yes, it does, because nsEventStateManager::NotifyMouseOut recurses down into subdocuments and calls SetContentState(nsnull, NS_EVENT_STATE_HOVER) when we're moving out of everything.
I've added that scenario to the test.

Comment 16

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/81bd19f483d0

Comment 17

[Vlad [QA]]

Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0b3

Comment 18

[Daniel Holbert [:dholbert]]

This bug's test "test_bug596600.xul" appears to be one of the largest sources of randomorange right now -- it's failing intermittently multiple times per day (an average of twice per day since it's been checked in).

The randomorange is tracked in bug 661099.  Anything we can do to fix/improve the situation there?