Closed Bug 425259 Opened 15 years ago Closed 14 years ago

Tooltip can appear while Minefield app is in background

Categories

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

x86
macOS
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: smichaud, Assigned: jaas)

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

STR:

1) Run Minefield and another app.  Open one window in each app.  Make
   sure the other app's window doesn't cover the Minefield window's
   Reload button even when the other app has the focus.

2) Give Minefield the focus, move the mouse over the Reload button,
   and wait for its tooltip to appear.

3) Command-tab to switch to the other app -- the mouse will still be
   over the Minefield window's Reload button, but the tooltip will
   disappear (as it should).

4) Move the mouse very slightly, leaving it still over the Reload
   button -- the tooltip will reappear, which is wrong.

(Markus Stange reported this STR yesterday on IRC.)

This problem regressed with the 2008-03-13-04-trunk nightly, and seems
to be caused by a Cocoa widgets bug (an old one) that was uncovered by
the patch for bug 297080.

In my next comment I'll post a patch that fixes this bug, and doesn't
seem to cause any problems.
Attached patch Provisional fix. (obsolete) — Splinter Review
Here's a patch that fixes this problem, and (as far as I can tell)
doesn't regress bug 297080 or cause any other problems.

But it's very difficult for me to tell what effect (if any) the patch
for bug 297080 had on OS X ... so it's difficult to be sure my patch
doesn't regress anything there.

What's your opinion, Matthew?

(I'll post a tryserver build made with this patch once it finishes
building.)
Assignee: joshmoz → smichaud
Status: NEW → ASSIGNED
Attachment #311875 - Flags: review?(joshmoz)
If the only consequence of this bug is that a tooltip sometimes gets
displayed when it shouldn't, that doesn't seem particularly serious.

But it's really bad form to process inappropriate events while the
browser is in the background.  And other consequences of doing this
might be more serious.

So I'm requesting blocking1.9.

(It helps that we already have a decent fix ... at least as far as I
can tell.)
Flags: blocking1.9?
Yes, blocking 1.9+.  Also, I'm seeing tooltips actually appear when the app is in the background.  Hoping this fixes that problem as well.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
(In reply to comment #1)
> But it's very difficult for me to tell what effect (if any) the patch
> for bug 297080 had on OS X ... so it's difficult to be sure my patch
> doesn't regress anything there.
> 
> What's your opinion, Matthew?

Strange, I can't reproduce this on my debug build (from yesterday afternoon) but I can on a current nightly.  I did discover a similar problem which I can reproduce in the debug build both with and without your patch:

1. Hold the command key down
2. Hover over the reload button until the tooltip appears
3. Press tab to cycle through windows

The tooltip will remain visible even after another application is selected.

There wasn't a lot of change in behaviour on Mac when the fix for bug 297080 landed because we (like other Mac apps) ignore most events when we're not focused, so a lot of the problems showing up on Windows/Linux just couldn't happen.
(In reply to comment #5)
> I did discover a similar problem which I can
> reproduce in the debug build both with and without your patch:
> 
> 1. Hold the command key down
> 2. Hover over the reload button until the tooltip appears
> 3. Press tab to cycle through windows

I can reproduce this one on Linux, too, so it's a cross platform bug.  I'm pretty sure it'll happen on Windows, too, since I don't think we generate mouse exits from focus out events.
(In reply to comment #6)
> (In reply to comment #5)
> > I did discover a similar problem which I can
> > reproduce in the debug build both with and without your patch:
> > 
> > 1. Hold the command key down
> > 2. Hover over the reload button until the tooltip appears
> > 3. Press tab to cycle through windows

That isn't the same bug though. The fix for that would require the platforms specific code to provide some sort of notification to the popup manager when a window is unfocused, similar to the way that Rollup is called.

I can't reproduce the original bug report.

Is this bug that the tooltip appears using those specific steps (with Cmd+tab)? Or that tooltips appear in general when the window is in the background?
> I can't reproduce the original bug report.

Even in a nightly?

> Is this bug that the tooltip appears using those specific steps
> (with Cmd+tab)?  Or that tooltips appear in general when the window
> is in the background?

As far as I'm concerned it's the latter.  If there are other cases
than the one I describe here (in comment #0), I'd like to hear about
them ... especially if my patch (attachment 311875 [details] [diff] [review]) doesn't fix them
:-)
I can reproduce this in nightly builds but not debug builds. Strange.
Here's a gdb trace (made using a build with debug symbols) of this
bug's tooltip being "shown" while the browser is in the background.

So it (apparently) isn't being "shown" as Gecko is processing the exit
event ... though it's definitely being "shown" as a direct consequence
of that exit event having been sent to Gecko while the browser is in
the background.
That stack is ok, but not as useful, as popups are displayed during layout asynchonously. What you actually want is a stack of the call to nsXULTooltipListener::LaunchTooltip.

nsXULTooltipListener::LaunchTooltip is called on a timer which fires 500ms after the last mousemove event, which is because this is the delay between when the user stops the mouse over something and when the tooltip for it should appear.

An already open tooltip should be closed though when the mouseout occurs on the node that triggered it. If a tooltip is pending, a mouseout should cancel the timer.

>> Or that tooltips appear in general when the window is in the background?
>
> As far as I'm concerned it's the latter.

So do you mean the bug can be reproduced as follows?

1. Move Firefox to the background.
2. Hover the mouse over something.
3. A tooltip appears when it shouldn't
>>> Or that tooltips appear in general when the window is in the
>>> background?
>>
>> As far as I'm concerned it's the latter.
>
> So do you mean the bug can be reproduced as follows?
>
> 1. Move Firefox to the background.
> 2. Hover the mouse over something.
> 3. A tooltip appears when it shouldn't

I mean that I'm interested in more cases than the specific one I
reported here (if any can be found).
> What you actually want is a stack of the call to
> nsXULTooltipListener::LaunchTooltip

Will do.
When an app is in the background on Mac OS X, having the mouse over an element should not bring up a tooltip. On Windows, it should. Just wanted to make that clear here.
> On Windows, it should

On Windows it _does_ (on both the trunk and the 1.8 branch) ... but
I'm not sure that's right.  I just did some quick tests with IE 7 and
MS Word (on Windows XP), and neither shows tooltips when it's in the
background.
On Linux (Kubuntu 7.10), Firefox (trunk and branch), Konqueror and
Open Office all show tooltips while in the background.  Could the GTK2
behavior inadvertently have become the standard for Firefox on other
platforms (like OS X and Windows) where it isn't really correct?
(In reply to comment #0)
> This problem regressed with the 2008-03-13-04-trunk nightly, and seems
> to be caused by a Cocoa widgets bug (an old one) that was uncovered by
> the patch for bug 297080.

For the old bug, bug 209565 or bug 270872?  

(Note the latter claims not to be a problem in Camino, but nevertheless we have bug 300904 about essentially the same issue.  The former bug was originally reported against Camino but was mis-triaged into Widget:Mac, so it may well be the one you seek, especially with the Cmd-Tab bit.)
Keywords: regression
Attached file josh gdb 1
XXJOSH sending mouse move event from widgets, number 160
XXXJOSH nsXULTooltipListener::MouseMove called
*tooltip disappears*
XXXJOSH nsXULTooltipListener::MouseMove called
*tooltip appears*

In this sequence that I logged, you can see that nsXULTooltipListener::MouseMove is invoked without widgets sending a mouse moved event. How does that happen?  I set my debugger to break on the nsXULTooltipListener::MouseMove call that puts up the incorrect popup that this bug is about. "josh gdb 1" shows a gdb session with a stack, it shows that nsXULTooltipListener::MouseMove is getting invoked in response to widgets sending a mouse exit event. That seems wrong to me, the exit event from widgets seems right.
Neil Deakin pointed out that this happens because in nsEventStateManager::PreHandleEvent:

829   case NS_MOUSE_EXIT:
830     // If the event is not a top-level window exit, then it's not
831     // really an exit --- we may have traversed widget boundaries but
832     // we're still in our toplevel window.
833     {
834       nsMouseEvent* mouseEvent = static_cast<nsMouseEvent*>(aEvent);
835       if (mouseEvent->exit != nsMouseEvent::eTopLevel) {
836         // treat it as a move so we don't generate spurious "exit"
837         // events Any necessary exit events will be generated by
838         // GenerateMouseEnterExit
839         aEvent->message = NS_MOUSE_MOVE;
Note that changes were made to the code in comment #20 recently, in bug 297080.
So it seems that

http://lxr.mozilla.org/mozilla/source/widget/src/cocoa/nsChildView.mm#2966

should just use eTopLevel instead of eChild, since its a mouse exit when the application isn't focused any more.
Assignee: smichaud → joshmoz
Status: ASSIGNED → NEW
Attachment #311875 - Attachment is obsolete: true
Attachment #311875 - Flags: review?(joshmoz)
Attached patch fix v1.0Splinter Review
child -> top-level fixes the problem
Attachment #313110 - Flags: review?(smichaud)
Comment on attachment 313110 [details] [diff] [review]
fix v1.0

This looks fine to me.

I did some minimal testing (on OS X 10.5.2) and confirmed that it does
fix the bug.
Attachment #313110 - Flags: review?(smichaud) → review+
Attachment #313110 - Flags: superreview?(vladimir)
Attachment #313110 - Flags: superreview?(vladimir) → superreview+
landed on trunk
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.