144880 - Tooltip listener not always told to hide tooltip

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Adam Lock]

The embedding tooltip listener is not always told to hide the tooltip when the
mouse is moved out of the window.

Comment 1

[Adam Lock]

Tooltips are shown and hidden by processing mouse and keyboard events:

http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsDocShellTreeOwner.cpp#937

Pink, do you want to take this?

Also see:

http://bugscape.netscape.com/show_bug.cgi?id=12954

Comment 2

[Mike Pinkerton (not reading bugmail)]

we don't want to kill the timer when the mouse moves at all (as suggested in the
bugscape bug), only when the mouse leaves the area where the tooltip is "hot".
If the timer fires, then gecko isn't getting the events, and it sounds either
like the embedding app isn't correctly sending the native events to gecko or the
gecko widget code is at fault. Since seamonkey works fine, i'd venture that it's
the embedding app.

I think conrad has tooltips hooked up in PPEmbed, fwiw.

Comment 3

[Michael Dunn]

Internal reference:
http://bugscape.mcom.com/show_bug.cgi?id=12954

Comment 4

[Adam Lock]

Attachment: Patch adds tooltips to mfcembed

This patch turns mfcembed into a tooltip listener and implements a simple
tooltip class.

Comment 5

[Adam Lock]

Comment on attachment 111659 [details] [diff] [review]
Patch adds tooltips to mfcembed

Chak can you review this please?

Note I think mfcembed needs detabbing but I can do that when I check in.

Comment 6

[Chak Nanga]

Comment on attachment 111659 [details] [diff] [review]
Patch adds tooltips to mfcembed

r=chak

Comment 7

[Adam Lock]

The patch can be used to demonstrate an issue that if the cursor is one pixel
from the edge of the view, a tooltip can be displayed which will not be
immediately dismissed if the mouse moves out of the view. The tooltip timer
still hides the tooltip however after a few seconds.

The reason looks as though the nsIDOMMouseListener is not receiving a MouseOut
call so it has no clue to hide the tip. On Win32 there is a
TrackMouseEvent/WM_MOUSELEAVE mechanism to detect when the mouse has left the
window. Perhaps the Win32 nsWindow.cpp impl should use these to generate
NS_MOUSE_EXIT events. The only gotcha is the API is Win98 onwards, so if we
still support Win95, they won't work.

Comment 8

[Adam Lock]

Comment on attachment 111659 [details] [diff] [review]
Patch adds tooltips to mfcembed

Alec can you sr this addition to mfcembed to implement tooltips? I intend to
fix tabs when I check in. Thanks

Comment 9

[Alec Flett]

Comment on attachment 111659 [details] [diff] [review]
Patch adds tooltips to mfcembed

look at the patch - you're mixing tabs and spaces in some of the files, not
sure which you should be using, but other than that this looks fine.
sr=alecf

Comment 10

[Adam Lock]

Fix for mfcembed is checked in. I've also detabbed most of the source code.

Comment 11

[Adam Lock]

I think I have this one figured now. Basically, when you move the cursor out of
the embedding window, the NS_MOUSE_EXIT event fires with a point that may not
intersect any views. The view manager won't dispatch it since it is not over a
view and the event is ignored. Possibly the patch is to short circuit
NS_MOUSE_EXIT behaviour so that it will always be dispatched to the root view in
the associated window even if the point falls outside.

The function that builds a list of views is 
nsViewManager::CreateDisplayList.

nsViewManager::CreateDisplayList(nsView * 0x012ece88, int 0, DisplayZTreeNode *
& 0x00000000, int 0, int 0, int 0, nsView * 0x02d1c6d8, const nsRect *
0x0012fa14, nsView * 0x00000000, int 0, int 0, int 1, int 1) line 3395
nsViewManager::BuildDisplayList(nsView * 0x02d1c6d8, const nsRect & {...}, int
1, int 0) line 2102
nsViewManager::BuildEventTargetList(nsAutoVoidArray & {...}, nsView *
0x02d1c6d8, nsGUIEvent * 0x0012fce8, int 0) line 2132
nsViewManager::HandleEvent(nsView * 0x02d1c6d8, nsGUIEvent * 0x0012fce8, int 0)
line 2171
nsView::HandleEvent(nsViewManager * 0x012ec988, nsGUIEvent * 0x0012fce8, int 0)
line 304
nsViewManager::DispatchEvent(nsViewManager * const 0x012ec988, nsGUIEvent *
0x0012fce8, nsEventStatus * 0x0012fbe4) line 1942 + 23 bytes
HandleEvent(nsGUIEvent * 0x0012fce8) line 83
nsWindow::DispatchEvent(nsWindow * const 0x02d1c99c, nsGUIEvent * 0x0012fce8,
nsEventStatus & nsEventStatus_eIgnore) line 1115 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fce8) line 1136
nsWindow::DispatchMouseEvent(unsigned int 323, unsigned int 0, nsPoint *
0x00000000) line 5376 + 21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 323, unsigned int 0, nsPoint *
0x00000000) line 5633
MouseTrailer::TimerProc(HWND__ * 0x00000000, unsigned int 275, unsigned int
31722, unsigned long 24648281) line 1058
USER32! 77d67b17()
USER32! 77d6cc4b()
USER32! 77d4423d()
USER32! 77d44d38()
CMfcEmbedApp::Run() line 735 + 11 bytes
AfxWinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00000000, char *
0x00141f13, int 1) line 49 + 11 bytes
WinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00000000, char * 0x00141f13,
int 1) line 30
WinMainCRTStartup() line 330 + 54 bytes
KERNEL32! 77e7eb69()

Comment 12

[Adam Lock]

Attachment: Patch

This patch updates the event dispatching code in the view manager. If no views
are found underneath the point of a mouse exit event we call the one supplied.
This ensures mouseleave DOM events are generated and tooltips and status text
is properly cleared.

Possibly something along these lines should be done anyway since the mouse exit
event belongs to the view supplied in the call, not the one the mouse happened
to be over.

Comment 13

[Adam Lock]

Comment on attachment 112434 [details] [diff] [review]
Patch

Robert, can you review this this view manager patch? It ensures that
NS_MOUSE_EXIT events are handled even if the cursor is not over a view at the
time. Thanks

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If the event coordinate doesn't matter, then I think you can just add
NS_MOUSE_EXIT to the big 'if' at line 2155 which just goes ahead and calls
obs->HandleEvent.

Comment 15

[Adam Lock]

I didn't add it to the top because I only want this code to be triggered after
BuildEventTargetList comes back claiming there are no views underneath the
point. Most of the time there will be, but always not in embedding when moving
outside the embedding area.

I'm not sure what values to put in the x & y. DOM2 says mouseout should have a
clientX, clientY but given the edginess of this issue and the pain translating
them properly I just stuck 0, 0 in there for now. I might be able to write a
utility to do the translation but the existing code to do it is all mixed in
with the list building stuff.

I think after looking at this issue that the way that NS_MOUSE_EXIT/mouseout is
handled may need to be changed in general. The existing implementation searches
and sends the event for handling to the views under the cursor, and not the view
that the mouseout is relevant for. Most of the time this probably doesn't
matter, but I suspect that it is more wasteful (building a list of views) and
might lead to the wrong handler being used. Maybe the way bubbling happens is
saving us from seeing this more noticeably.

Comment 16

[David Epstein]

Charles, I added this tooltip support into TestEmbed. Please verify it works
there as well. Thanks.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Some more comments about your patch:

The event's coordinates coming in to nsViewManager::HandleEvent are supposed to
be in the coordinate system of aView. So you shouldn't have to touch them at
all, just pass them into the observer. If this is not true, there must be some
other bug.

You shouldn't test 'obs' in the fallback if vobs is null. If vobs is null (which
should never happen anyway) you should do nothing at all. Dispatching to obs for
a view which isn't even managed by obs is definitely wrong.

About MOUSE_EXIT handling in general:

I think that the event state manager doesn't really care what view it gets the
event for as long as it gets the event. It's in charge of figuring out what
frames have really been exited. (That's why what we have currently usually
works.) And this is why I think it should be correct to put NS_MOUSE_EXIT in the
early clause.

Comment 18

[Adam Lock]

Attachment: Patch 2

This is a simpler patch that adds mouse exit to the top with the other
exceptions. It works with embedding and does not appear to adversely affect
tooltips in Mozilla.

Robert, can you r this please?

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 113540 [details] [diff] [review]
Patch 2

r+sr=roc+moz

Comment 20

[Conrad Carlen (not reading bugmail)]

Comment on attachment 113540 [details] [diff] [review]
Patch 2

r=ccarlen

Comment 21

[Adam Lock]

Comment on attachment 113540 [details] [diff] [review]
Patch 2

Request 1.3b approval.

Fix is small, fixes a topembed and doesn't affect Mozilla or sites such as the
new devedge which are mouse event dependent.

Comment 22

[Asa Dotzler [:asa]]

Comment on attachment 113540 [details] [diff] [review]
Patch 2

a=asa (on beahlf of drivers) for checkin to 1.3beta.

Comment 23

[Adam Lock]

Fix checked in

Comment 24

[Charles Rosendahl]

20030214 MFCEmbed and TestEmbed - verified.