300904 - Mouse move events fire even if the window is covered by dashboard or Dock menus

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Ben]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3) Gecko/20050714 Camino/0.9a2
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3) Gecko/20050714 Camino/0.9a2

When you reveal the desktop, using by default the F11 shortcut, the components
of the web page are still affected by mouse movement.  For example if you find
any page with some hyperlinks on, press F11, and move your mouse to where the
link would have been on the screen, the mouse cursor changes, and if you can see
the bottom of the camino window you can see the URL of the page.

This also happens when you use expose's all windows, but it is easier to
demonstrate using a flash page.  All of the mouses movements still change the
flash object relative to where it would have been on the screen.

This only happens if camino was the last application to be in view (It's menu is
at the top) before you use expose's all windows or desktop expose.  I don't know
if this is a bug with os x, however I can't recreate the same problem with safari.

Reproducible: Always

Steps to Reproduce:
Desktop expose:
1. Go to any page with hyperlinks, one with lots works better such as a forum.
2. Do the desktop expose - F11 by default or hot corner
3. Mouse over where the hyperlinks were before you exposed the desktop

All windows expose
1. Go to a flash page, something like H*R's main page (
http://www.homestarrunner.com/main1.html )
2. Expose all windows - F9 by default
3. mouse over where the active flash components were, you will see/hear actions
on the small camino window.

Actual Results:  
The links and flash were still affected even though the window was not there.

Expected Results:  
Nothing

Comment 1

[Mike Pinkerton (not reading bugmail)]

yeah, i see this. i think this is a duplicate of another bug.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm pretty sure it's a dupe, too, but I can't find the original right now.

Comment 3

[Stuart Morgan]

*** Bug 322872 has been marked as a duplicate of this bug. ***

Comment 4

[Chris Lawson (gone)]

For the record, I can't find anything that this is a dupe of either.

cl

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Fx has problems with mouseovers, etc., firing 100% of the time when the app is in the background, but we don't have that problem.  Maybe that's what we were thinking of?  Or one of the tooltip bugs (bug 312411, bug 285601, bug 311220)?

Adding "mouseover" to the summary to perhaps make this easier to find in the future....

Comment 6

[mattwala]

Also happens with the "wait cursor".  It shows up in Dashboard when Camino's loading a page.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Heh, bug 209565 was originally reported with Camino.  Still not sure that's exactly the same bug as this, though.

Comment 8

[jdv]

I just noticed that if you have Terminal as the front-most app and you hit F11 you can hover over where Terminal _was_ and you can see the cursor change as if the window was still there.  Easily seen if you are doing a build in a Terminal window so that stuff is scrolling by.

So, unless I misunderstand this bug, this behaviour does not seem specific to Camino.  Every app I try does this.

Comment 9

[Chris Lawson (gone)]

Obviously can't be a problem with anything before 10.4...

Per comment 8, I wonder if this is simply an OS "bug" -- or rather, intended behaviour, since it seems to be by design.

cl

Comment 10

[Chris Lawson (gone)]

Per further investigation, the Expose and Dashboard cases seem to be separate. I can reproduce the Dashboard portion of this bug in any Gecko browser, but not in apps that use WebKit to render the content area.

I can reproduce the initial behaviour of the Expose portion of this bug in *any* OS X app. Moving the cursor once Expose is triggered (via, say, F11) causes the hand cursor to go away as expected. I'm not sure what non-browser apps I could try to reproduce the rest of the Expose portion in.

cl

Comment 11

[Markus Stange [:mstange]]

Moving to Core: Widget.

Comment 13

[Markus Stange [:mstange]]

Attachment: wip

Need to see how this interacts with the tests from bug 470845.

Comment 14

[Markus Stange [:mstange]]

Attachment: v1

Return of the tracking rects! :)

It turns out that looking at our own views' rects won't be any help in finding out whether the mouse is over a window of a different app that's in front of one of our own windows. Apparently the only thing that can answer this question is a tracking rect.
It should be noted that tracking rects still work, even though we turn off *cursor* rects. And since Compositor removed most child widgets I'm not fearing any performance hit.

In general I've found tracking rects to be very reliable. They just have one major drawback: When you create a tracking rect, it doesn't tell you whether the mouse is already over it. Instead you have to wait for the mouse to leave or enter the existing rect, and only when that has happened, you can be really sure where the mouse is.

That's the only reason for keeping our current mouse-in-view tracking system (which works with NSMouseInRect and [NSView hitTest:]).

I've added a mouseEnterState field to ChildView, which is either "unknown", "inside" or "outside". Whenever our NSView geometry changes it's reset to "unknown" and updated when the tracking rect informs us about mouseEnters and exits.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Markus, will this fix the thing I've seen recently where I'm seeing :hover effects on crash-stats "Query Results" pages even though I'm selecting something in another app's Dock menu (which just happens to be on top of the crash-stats page)?  Or is that bug 209565, or even some other bug?

Comment 16

[Markus Stange [:mstange]]

This will fix it.

Comment 17

[Josh Aas]

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

+- (void)removeTrackingRect
+{
+  if ([self window] && mTrackingRect) {

Is it really necessary to check "[self window]" here? That should never be false when mTrackingRect != 0.

+  [newContentView setAutoresizingMask:(NSViewWidthSizable | NSViewHeightSizable)];

Why does this become necessary with this patch?

According to Apple docs:

"If you use tracking rectangles, you should be sure to remove and reestablish them when you change the frame rectangle of the view object that contains them. If you’re creating a custom subclass of NSView, you can override the setFrame: and setBounds: methods to do this, as shown in “Compatibility Issues.”"

I see you've done this for setFrame:, but what about setBounds:?

Comment 18

[Markus Stange [:mstange]]

Attachment: v2

(In reply to comment #17)
> (From update of attachment 404182 [details] [diff] [review])
> +- (void)removeTrackingRect
> +{
> +  if ([self window] && mTrackingRect) {
> 
> Is it really necessary to check "[self window]" here? That should never be
> false when mTrackingRect != 0.

Good point. Removed the check.

> +  [newContentView setAutoresizingMask:(NSViewWidthSizable |
> NSViewHeightSizable)];
> 
> Why does this become necessary with this patch?

I'm actually a little surprised that it wasn't necessary before it ;-)
Without this, resizing a popup window won't call setFrame or setBounds on the content view, and [view frame] will always return a 0x0 sized rect. I don't know why drawing into them has always worked regardless.

> I see you've done this for setFrame:, but what about setBounds:?

We're not calling setBounds anyway, if I recall correctly, so I thought it was unnecessary. But I guess it doesn't hurt to add it, so I've done that in this patch.

Comment 19

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/3ee95c194798

Comment 20

[Markus Stange [:mstange]]

Backed out because of a DHTML performance regression.

http://hg.mozilla.org/mozilla-central/rev/e3f00dd9617b
http://hg.mozilla.org/mozilla-central/rev/2fbbdaa5f1d7

http://graphs.mozilla.org/graph.html#tests=[{%22machine%22:63,%22test%22:18,%22branch%22:1},{%22machine%22:64,%22test%22:18,%22branch%22:1},{%22machine%22:65,%22test%22:18,%22branch%22:1},{%22machine%22:168,%22test%22:18,%22branch%22:1},{%22machine%22:169,%22test%22:18,%22branch%22:1},{%22machine%22:170,%22test%22:18,%22branch%22:1},{%22machine%22:171,%22test%22:18,%22branch%22:1},{%22machine%22:172,%22test%22:18,%22branch%22:1},{%22machine%22:173,%22test%22:18,%22branch%22:1},{%22machine%22:174,%22test%22:18,%22branch%22:1},{%22machine%22:175,%22test%22:18,%22branch%22:1},{%22machine%22:176,%22test%22:18,%22branch%22:1},{%22machine%22:177,%22test%22:18,%22branch%22:1},{%22machine%22:178,%22test%22:18,%22branch%22:1},{%22machine%22:179,%22test%22:18,%22branch%22:1},{%22machine%22:180,%22test%22:18,%22branch%22:1},{%22machine%22:181,%22test%22:18,%22branch%22:1},{%22machine%22:182,%22test%22:18,%22branch%22:1}]&sel=1256028900,1256201700

Comment 21

[Markus Stange [:mstange]]

Attachment: v3

This uses the private API CGSGetScreenRectForWindow to get the rect of windows that don't belong to our process.

Comment 22

[Markus Stange [:mstange]]

Attachment: v3.1

Remove one superfluous variable.

Comment 23

[Markus Stange [:mstange]]

Comment on attachment 407724 [details] [diff] [review]
v3.1

This is not ready yet.

Comment 24

[Markus Stange [:mstange]]

Attachment: v3.2

Comment 25

[Markus Stange [:mstange]]

I just discovered that these patches don't fix the "reveal desktop" case, so tweaking summary.

Comment 26

[Josh Aas]

Comment on attachment 411399 [details] [diff] [review]
v3.2

Window level 20 happens to be the Dock. You can see this by printing out the value of "kCGDockWindowLevel", which is really this call:

CGWindowLevelForKey(kCGDockWindowLevelKey)

I suggest using kCGDockWindowLevel instead of a constant so if it changes we track that, but cache the value since it is really a call.

Comment 27

[Markus Stange [:mstange]]

Attachment: v4

Comment 28

[Josh Aas]

Comment on attachment 416436 [details] [diff] [review]
v4

>+  extern CGSConnection _CGSDefaultConnection(void);

I'm not a fan of "(void)" signatures, please just use "()".

>+  // We're not on 10.6, so we have to fall back to private API calls.

Make this "10.6+" for accuracy, or re-word the comment to describe the actual API check we are doing.

If I remember correctly, the tracking rect solution was actually more correct than this solution in that it handles some edge-cases better (which window levels are ignored, that sort of thing). This latest version of the patch is probably faster but we'll mishandle some edge cases (likely a good trade-off for now). If this is true, can you please add a comment making it is clear why we chose this solution?

Comment 29

[Markus Stange [:mstange]]

Done.

http://hg.mozilla.org/mozilla-central/rev/e69c8fde7474