274736 - [FIXr]Hover state remains active when onmouseout removechild is used

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

In the url testcase, I see this happening for the links that have got a
'nicetitle', e.g. hovering over these links give a dhtml tooltip.
When I put my mouse over such a link and then scroll one mousescroll down with
my mousewheel (I have smooth scrolling turned off), the hover state of that link
remains active.
I believe I saw this particular issue with the url testcase, after bug 20022 was
fixed.

However, I'll attach a testcase that shows that this bug was there before that
(I can see it with the testcase even in Mozilla1.0)

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

Comment 2

[Doug Halamay]

I'm seeing this also with the latest Firefox nightly:

Mozilla/5.0 (Windows; compatible; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050319 Firefox/1.0+

Also of interest (whether or not it is exactly related to this bug or not, I'm
not sure) is that if you hover over one of the links in the testcase,
middle-click to open a new tab, and then close the new tab (so that you are back
to the original tab), the hover state will still be active until you mouse into
the content area.

Comment 3

[Martijn Wargers (dead)]

This bug is also WFM in 2005-03-29 build. Most likely fixed by bug 284664.

Comment 4

[Martijn Wargers (dead)]

This has regressed again in current trunk build.
The bug doesn't happen in a 2005-05-03 build, but it does happen in a 2005-05-04
build.
I suspect the fix for bug 292472 might be the culprit.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So the problem here is that we move hover state from the target of the mouseout
to the target of the mouseover.  But the mouseout removes the mouseover target
from the DOM.  In that case, we end up with a null common ancestor; fine.  But
when we go to notify on content state changes, we get the document from the new
hover content (which is not in the document anymore).  So we don't send a
ContentStatesChanged notification.... If I fix that by using the "right"
document, then the red on the "Text" I'm leaving goes away, but the new "Text"
isn't highlighted.

If I just set the hover state to the mouseover node _before_ firing mouseout,
things seem to work.  Is that a reasonable approach?

Comment 6

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

(In reply to comment #5)
> If I just set the hover state to the mouseover node _before_ firing mouseout,
> things seem to work.  Is that a reasonable approach?

It's a bit counter-intuitive for anyone who's relying on side effects of the
hover state, but I can only think of a couple of possibilities for that: hover
state affecting layout affecting offsetX/Y etc, or people using :hover to attach
XBL. Neither of those seems very likely but I guess an alternative solution
would be better if one could be found...

Comment 7

[Boris Zbarsky [:bzbarsky]]

So the reason the new text isn't hightlighted if I just change the way I get the
document is that then the move event generated by the reflow doesn't find the
right document.

More generally, we have this weirdness where the ESM notifies content in other
documents at times...  Do we really need this for focus changes?  Isn't the ESM
in the newly-focused document going to get its own notifications as well?

In any case, I think the safe way to go is to not change the ordering of content
state changes and events but to fix the document-getting in SetContentState.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Sorta-patch

This sort of does the least it can to make this testcase work...

I removed the assert because it's just clearly bogus given that nodes can be
removed from the document by the time we call this stuff on them.  That said,
though, perhaps we should check somewhere here that things are generally still
in the right document.	Calling SetContentState() on a node that's not even in
mDocument anymore seems somewhat odd to me...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 183416 [details] [diff] [review]
Sorta-patch

Requesting approval for a pretty straightforward fix to make sure we update the
:hover state right if the DOM is mutated under us.

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 183416 [details] [diff] [review]
Sorta-patch

moving request out to b3. We're very nearly wrapped up on 1.8b2.

Comment 11

[Mike Shaver (:shaver emeritus)]

Comment on attachment 183416 [details] [diff] [review]
Sorta-patch

a=shaver

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed for 1.8b3.