443178 - Tooltips cause bogus mouse events

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[Kenny Feldman]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0

Summary: HTML links with title attributes lose focus if cursor moves even the slightest bit while the title box is displayed. This causes CSS dropdown menus to collapse. This bug does not happen on the first attempt after launch or restart, but does happen every time afterwards.

The title attribute should pop up when you hover/pause on a link (kind of like a tooltip). This happens correctly. but if you move or wiggle the cursor while the title is showing, then the link loses focus, and the menu collapses.

This affects the hover state of CSS links, and since CSS dropdown menus rely on that, they'll collapse when the focus is lost.

Reproducible: Always

Steps to Reproduce:
(Won't occur on the first try after launch or re-launch, but will occur every time afterwards.)
1. Hover/pause on a link that has a title attribute
2. Wait for title to appear
3. Slightly move or wiggle the cursor while the title is showing, then the link loses focus, and the menu collapses. This affects the hover state of CSS links, and since CSS dropdown menus rely on that, they'll collapse when the focus is lost.
Actual Results:  
HTML links with title attributes lose focus if cursor moves even the slightest bit while the title box is displayed. This causes CSS dropdown menus to collapse. This bug does not happen on the first attempt after launch or restart, but does happen every time afterwards.


Expected Results:  
The title attribute should pop up when you hover/pause on a link (kind of like a tooltip). This happens correctly. but if you move or wiggle the cursor while the title is showing, then the link loses focus, and the menu collapses.

Cursor should be able to be moved so that user can navigate dropdown menus. For single links, the link should remain active as long as cursor is hovering.

This affects the hover state of CSS links, and since CSS dropdown menus rely on that, they'll collapse when the focus is lost.

Comment 1

[Christoph]

I can confirm this, try it on:
http://dev.datenkind.de/ff3/ff3-tooltip-bug.html

This has to be fixed immediately!


greetings

Comment 2

[Kenny Feldman]

Yup. Here's my example too: http://www.kpfdigital.com/firefoxbug/

Comment 3

[Kenny Feldman]

But it doesn't happen on the first attempt after FF is started/restarted. Only happens on the second time forward.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I've confirmed this in Firefox 3.

I also see it in current Camino 1.9.0-branch nightlies and Camino
1.6.1 (a 1.8-branch release).  So this must be a Cocoa widgets bug.

Camino generally uses native widgets (for objects like tooltips and
link-titles).  Firefox never does (all its widgets are customized).
Stuart, do you know if Camino uses native tooltips for link-titles?

In any case, the bug seems to be that link-titles are treated exactly
like tooltips, and are dismissed too "early".

Comment 5

[Stuart Morgan]

Camino never uses actual tooltips, just an NSWindow of our own creation and positioning.

However, I'm unable to reproduce this in Camino 1.6 or a current Camino nightly on two different machines, and one of the test cases explicitly says it doesn't happen in Camino 1.5. Your shortened version of the summary is not what the bug description is actually about, so I suspect that what you reproduced in Camino is not this bug at all; it sounds like you are talking about the tooltip being dismissed (which I'm not sure is even a bug), not the entire menu collapsing.

Comment 6

[Kenny Feldman]

Here's one more observation: While I previously reported that restarting FF3 prevents the bug from happening the FIRST time it's tried, I have just found that merely closing the open window, and then opening a new window will also prevent the bug on the first attempt. This occurs even if there was another FF3 window open first. (Observed this while writing this reply in one window, and trying bug in a second window.)

Note that opening a new tab does not reset the first-try-only success.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #5)

> Camino never uses actual tooltips, just an NSWindow of our own
> creation and positioning.

Thanks for the info.

> However, I'm unable to reproduce this in Camino 1.6 or a current
> Camino nightly

> it sounds like you are talking about the tooltip being dismissed
> (which I'm not sure is even a bug), not the entire menu collapsing

Sorry, you're right.

In FF3 I see both the link-title being dismissed early and the menu
collapsing.  In Camino (1.6.1 and 1.9.0-branch nightlies) I see only
the link-title being dismissed early.

Both may be bugs, but I think the menu collapsing is the more serious
issue.  So that's what I'll put in the summary.

Comment 8

[Samuel Sidler (old account; do not CC)]

Not wanted for 1.9.0.x. Let's get this in 1.9.1.

Comment 9

[Markus Stange [:mstange]]

I'm digging into this. I believe bug 450325, bug 461589 and bug 465522 and this bug have the same underlying problem.

And the problem is that the mousemove event that hides the tooltip has the wrong position. Interestingly, the difference between the wrong position and the actual position is exactly the same as the window's position on the screen.

Now I just need to find out where the incorrect screen-to-window translation is happening.

Comment 10

[Markus Stange [:mstange]]

Attachment: v1

This patch fixes all the bugs I mentioned in the previous comment.

My debugging printfs showed that the first mouse move event after the tooltip is shown is sent twice:
 - first to the NSView in the tooltip window (which is the wrong target for
   this event), and
 - second to the NSView of the window where the mouse is actually over.

Additionally, the coordinates that the second event returns from [theEvent locationInWindow] are wrong (off by the window's position on the screen).

Since the first event is sent to the wrong window, [[mWindow contentView] hitTest:...] returns nil. So we assume that the mouse isn't over any window (which is wrong) and send a mouse exit event with coordinates relative to the tooltip (also wrong).

---

My patch changes ensureCorrectMouseEventTarget: to check for the right window even if no rollup widget is visible. This makes us ignore the first event.

And for some reason it also causes the second event to have sane coordinates, and thus fixes this bug.

Comment 11

[Markus Stange [:mstange]]

Attachment: v2

I just tested the patch on 10.4 and noticed that the "second event" isn't sent there. So we do need to resend the event. This will result in sending the same mouse move event twice on 10.5 but I don't think that will break anything.

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 351275 [details] [diff] [review]
v2

This patch breaks mouse capturing when the mouse is held down.

Comment 13

[Markus Stange [:mstange]]

Furthermore, it makes bug 425556 occur even when no context menu is open.

Comment 14

[Markus Stange [:mstange]]

Attachment: v3

This patch doesn't have the disadvantages of the previous one and additionally fixes bug 425556.

The algorithm that determines whether a window should receive mouse move events is a little different than that in your patch for bug 425556 because it also works for panels and sheets.

I tested this patch on both 10.5 and 10.4 and couldn't find any regressions.

Comment 15

[Markus Stange [:mstange]]

There's one thing that I should clarify.

As far as I can tell, the incorrect mouse position translation I mentioned in comment 9 doesn't happen in Gecko code. It rather looks like it's an Apple bug - the event that's passed to [ChildView mouseMoved:] just carries a bogus location ([event locationInWindow] is wrong). In this patch I'm working around that bug by getting the mouse location from [NSEvent mouseLocation] instead of looking at the event:

 NSPoint nsCocoaUtils::ScreenLocationForEvent(NSEvent* anEvent)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN;
 
+  // Don't trust mouse locations of mouse move events, see bug 443178.
+  if ([anEvent type] == NSMouseMoved)
+    return [NSEvent mouseLocation];
+
   return [[anEvent window] convertBaseToScreen:[anEvent locationInWindow]];

Comment 16

[Markus Stange [:mstange]]

I've noticed one problem with the patch: When I have two Firefox windows open, start dragging a scrollbar in one of the windows and move the mouse over the other window (while still dragging), the mouse events aren't captured by the active window but instead routed to the window under the mouse (so the scrollbar doesn't move).
So I probably have to add an NSLeftMouseDragged somewhere where I check for NSMouseMoved.

Comment 17

[Markus Stange [:mstange]]

Attachment: v4

I've extended the "window accepts mouse event" check to cover all mouse events except NSRightMouseDown, not only mouse move events. This seems to work very reliably.

Comment 18

[Markus Stange [:mstange]]

(Adding dependencies for all the bugs that this patch fixes)

Comment 23

[Nathan]

Is there an estimated timeframe when can we expect to see the patch to fix this bug implemented in Firefox?

Comment 24

[Steven Michaud [:smichaud] (Retired)]

I'd really like to get (back) to this soon ... but I've got a lot of
other things on my plate.

When I do get back to it, I expect I'll want to rewrite Markus' patch,
and that will take at least several days.

So I can't really give an estimate, but it's definitely on my radar.

Comment 25

[Nathan]

Thanks for keeping this bug in mind. :)

I'm much less worried about it now that I realize its just effecting the 1%-2% Mac OS X visitors who come to my site, but I still want my popup menus to work for all Firefox users, no matter what their platform is.

Comment 26

[Markus Stange [:mstange]]

(In reply to comment #24)
> When I do get back to it, I expect I'll want to rewrite Markus' patch,

Does that mean you already have a general opinion of the patch? Could you tell me what you'd like to change?

Comment 27

[Steven Michaud [:smichaud] (Retired)]

At this point it's little more than a feeling that I really need to
spend several days looking through this bug's issues.

Sorry :-(

Comment 28

[Josh Aas]

Markus - if you can update this patch and make sure it passes all tests I'll do the review.

Comment 29

[Markus Stange [:mstange]]

Attachment: v4.1

Unfortunately I can't test whether this fixes bug 504450 comment 20 because I can't reproduce it on 10.4.

Comment 30

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #29)
> Created an attachment (id=394749) [details]
> v4.1
> 
> Unfortunately I can't test whether this fixes bug 504450 comment 20 because I
> can't reproduce it on 10.4.

I can try a tryserver build for you.

Comment 31

[Markus Stange [:mstange]]

Attachment: v4.2

updated for bug 510428

Comment 32

[Markus Stange [:mstange]]

Tryserver build: https://build.mozilla.org/tryserver-builds/[e-mail]-try-97bf8ff6b255/try-97bf8ff6b255-macosx.dmg

It's completely green, but given that native mouse events aren't tested at all, this doesn't mean much.

Comment 33

[Stephen Donner [:stephend] Not actively reading bugmail]

The tryserver build *doesn't* fix the focus issue mentioned in comment 29 (http://www.coteindustries.com/articles/html_articles/tabindex.html) on Mac OS X 10.5.8

Comment 35

[Markus Stange [:mstange]]

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

Comment 38

[Markus Stange [:mstange]]

Comment on attachment 394755 [details] [diff] [review]
v4.2

Requesting approval1.9.2 - this bug has caused web developers a lot of pain, see all the duplicates. The patch has baked for over a month and no regressions were reported. Bug 470845 has added tests for this bug.

Comment 39

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 394755 [details] [diff] [review]
v4.2

a192=beltzner as long as it goes in with the tests from bug 470845

Comment 40

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ea01e15b774a