Comment 52 for bug 239462

Revision history for this message
In , John Morkel (jmorkel) wrote :

Created attachment 349172
New patch incorporating review suggestions

(In reply to comment #48)
> (From update of attachment 346420 [details])
> > NS_IMETHODIMP
> > nsXULTooltipListener::MouseOut(nsIDOMEvent* aMouseEvent)
> > {
> ...
> > mTooltipTimer->Cancel();
> > mTooltipTimer = nsnull;
> > return NS_OK;
> >+ } else {
> >+ HideTooltip();
> > }
>
> Won't this hide the tooltip on any mouseout event? If someone puts a tooltip on
> a toolbar for instance, a mouseout event will fire every time someone moves the
> mouse out of an element.

I have reverted this section to the old code that checks if the mouse is leaving the target node. This seemed unnecessary to me since the event callback would only trigger if the mouseout event was for the target node.

> >+ // filter out false win32 MouseMove event
> > if (mMouseScreenX == newMouseX && mMouseScreenY == newMouseY)
> >+ return NS_OK;
> >+
> >+ // filter out minor movements due to crappy optical mice and shaky hands
> >+ // to prevent tooltips from hiding prematurely.
> >+ nsCOMPtr<nsIContent> currentTooltip = do_QueryReferent(mCurrentTooltip);
> >+
> >+ if ((currentTooltip) &&
> >+ (abs(mMouseScreenX - newMouseX) <= kTooltipMouseMoveTolerance) &&
> >+ (abs(mMouseScreenY - newMouseY) <= kTooltipMouseMoveTolerance))
>
> I don't see any reason to do this. Also, it won't work if someone moves the
> mouse slowly.

This code implements the exact behaviour that a few people in this bug discussion have requested. It changes the way tooltips behave, but it works well so I've left it there. I've tested it extensively and it works even if the mouse is moved slowly, since it compares the current mouse position to the position when the tooltip was shown.

> >+ // set a flag so that the tooltip is only displayed once until the mouse
> >+ // leaves the node
> >+ mTooltipShown = PR_TRUE;
>
> The tooltip may not actually be shown though, if, for example, a popupshowing
> event prevents it or it's in a hidden tab. Better to do this at the end of
> LaunchTooltip where a check is done for the popup being open.

Noted. I've moved this to LaunchTooltip.