38380 - [ESM/CSS] css a:hover ignored on link with javascript "onMouseover" and "return true;"

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[Doron Rosenberg (IBM)]

If a link has the javascript onMouseover code in it and that document has css
defined, the css for the link is overriden. Attaching test case.

Assigning to style system, but could be js engine. Could not find a smiliar bug.

Comment 1

[Doron Rosenberg (IBM)]

Attachment: testcase of bug

Comment 2

[Jesse Ruderman]

I think what you're saying is that mozilla "forgets" to update a link with the 
a:hover style information if the link has onmouseover code.  Modifying summary 
to make that more clear.

Comment 3

[David Baron :dbaron:]

The correct component here is probably "Event Handling".

Comment 4

[Doron Rosenberg (IBM)]

--> even handling, per David Baron's remark. Thanks!

Comment 5

[Pierre Saslawsky]

The problem comes from the "return true;" at the end of the onMouseOver and 
onMouseOut handlers. If you remove them, it works (I'm going to attach a 
testcase). Reassigned to jst to check the syntax. I'm not a JS/DOM expert and 
don't know whether it's correct to put a "return true" in the handlers.

Comment 6

[Pierre Saslawsky]

Attachment: testcase that works

Comment 7

[Johnny Stenback (:jst)]

This is indeed an event handling problem, from looking at the code the event

state manager sets the hover state *after* the content object has handled the

event and it sets the state only if the eventhandler doesn't return true (i.e.

if the event state is different than nsEventStatus_eConsumeNoDefault), this is

IMO wrong, but I'll let Tom Pixley decide on that...

Comment 8

[Johnny Stenback (:jst)]

*** Bug 42163 has been marked as a duplicate of this bug. ***

Comment 9

[joki (gone)]

Its a hard call but in this case I agree with you, the :hover should fire 
regardless of the return from the DOM handling.  I can think of cases where I'd 
like to be able to control css pseudo-class setting from DOM event code but I 
suppose you can just directly add/remove the pseudo-class and get the same 
effect.

Comment 10

[joki (gone)]

In case I don't get back to this right away the fix is somewhat more difficult 
than it seemed.  The tracking of the nsEventStatus_eConsumeNoDefault event state 
is currently what allows the use of the :hover state on the link, not the text 
underneath it.  This makes it very related to bug 5693

Comment 11

[joki (gone)]

Adding [ESM/CSS] prefix to bugs in EventStateManager with its generated events 
or CSS pseudo-class management.

Comment 12

[Doron Rosenberg (IBM)]

small note - the summary seems a bit misleading, as the return true; part is
missing. Adding this to the summary.

Comment 13

[Jeffrey Baker]

*** Bug 45779 has been marked as a duplicate of this bug. ***

Comment 14

[Jeffrey Baker]

*** Bug 45604 has been marked as a duplicate of this bug. ***

Comment 15

[ckritzer (gone)]

Mass update:  changing qacontact to [e-mail]

Comment 16

[Henri Sivonen (:hsivonen)]

*** Bug 47955 has been marked as a duplicate of this bug. ***

Comment 17

[ckritzer (gone)]

Per heikki, saari, ckritzer:
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration.

Comment 18

[Hixie (not reading bugmail)]

Eol - this is the bug that you hit.

Comment 19

[eol1]

Ian "Hixie" Hickson has told me of a temp fix for this (I am Eol from his above
comments).  Thought I should post it so that others with this problem who
stumble across this bug report can work around it till the bug is actually
fixed.  Change "return true" to "return false".  CSS a:hover attributes will
then work along with the javascript "onMouseover" tag.

Comment 20

[Niko Pavlicek]

I think ALL css/html 4/javascript/DOM bugs mustn't be marked as "Future" because
it is an aim of Mozilla 1.0 (and also Netscape 6) to have a standard compliant
browser, so there shouldn't be any known bugs you hadn't really tried to fix it
when you release it.
Especially not bugs which MANY web designers will see and will angry about!

Comment 21

[Michael C. Maggio]

According to my JavaScript references, returning false to an onMouseOver event 
should void the result of the event.  But Moz obviously does not conform to 
this behavior, otherwise the above workaround would not work.

This appears to be a related issue.

a:hover, of course, should be unaffected by what onMouseOver returns.

Comment 22

[Loco]

Updating QA Contact.

Comment 23

[David :Bienvenu]

OK, I'm nominating this for rtm because it breaks renting DVD's on netflix,
which is a very popular site. See bug 54113

Comment 24

[David :Bienvenu]

*** Bug 54113 has been marked as a duplicate of this bug. ***

Comment 25

[David :Bienvenu]

adding interested parties to cc list.

Comment 26

[David :Bienvenu]

changing priority to p2, otherwise pdt will never look at it.

Comment 27

[Nisheeth Ranjan]

Sorry, folks.  This isn't high priority enough for the rtm branch.  Marking
future, evangwanted, and rtm-.

Comment 28

[scottputterman]

Could someone verify that this causes 54113?  If it does, what would it take to
reconsider fixing this? Is it because it's too complicated of a fix or because
there's not enough engineering time that this was futured and rtm-'d? I really
want Netflix.com to work and I think it would be a shame if it didn't.

Comment 29

[David :Bienvenu]

I would be happy to try to help on this, though I know nothing about any of the
technologies involved. Or is this just considered so unimportant that we
wouldn't take a fix even if we had one?

Comment 30

[David Baron :dbaron:]

Hyatt said on bug 5693:  "IE5 puts the links into :hover even when true is 
returned from onmouseover.  I think we should do the same."

I agree.

Comment 31

[David Hyatt]

I have a trivial patch that fixes this bug.

Comment 32

[David Baron :dbaron:]

BTW, a justification for doing things the way this bug suggests (to respond to 
[e-mail] 's comments on bug 5693) is that :hover is a state rather 
than an action.  According to CSS2,
http://www.w3.org/TR/REC-CSS2/selector.html#dynamic-pseudo-classes ,

  The :hover pseudo-class applies while the user designates an element (with
  some pointing device), but does not activate it.

It happens that an element entering and leaving the :hover state is *somewhat* 
related to mouseover/mouseout events (see bug 5693 for the messy details).  
However, entering or exiting the :hover state is a state change required by the 
CSS spec unrelated to the mouseover/mouseout events.  (Admittedly, this means it 
can't be implemented in terms of the DOM event model, but I don't think it 
should be possible for JS to mess things up so things stay in :hover -- it 
messes up the cascade quite seriously.)

Comment 33

[Hixie (not reading bugmail)]

As David (Hyatt, not Baron!) and I discussed this afternoon, CSS and the DOM
are totally unrelated for this issue. They may be similar, but they do not 
affect each other in any way.

If you want to override the :hover style then don't set it, or set it at the
same time as setting the relevant element style, eg:

   :link, :visited { ... }

...will override any link hover colouring rules present in ua.css, and:

   div, div:hover { ... }

...will ensure that the rules set for div do not change when the div is in the
:hover state (assuming no rule of higher specificity is present in the same 
cascade level).

Comment 34

[Doron Rosenberg (IBM)]

ass this seems to kill some major sites, perhaps this could go into trunk and
then maybe into branch?

Comment 35

[scottputterman]

If hyatt has a trivial fix, can we [rtm need info] this and try to get it into
the branch?  I really want to be able to use Netflix :)

Comment 36

[David :Bienvenu]

yes, please, my queue is almost empty!

Comment 37

[Doron Rosenberg (IBM)]

For renomination, we need hyatt's path and r='s I believe.

Comment 38

[Jerry Baker]

Hyatt can you attach your patch so we can at least test it? Appreciate it.

Comment 39

[Jerry Baker]

Pinging Hyatt

Comment 40

[bugzilla.mozilla]

This seemed to be the right place to post, being the only result in a search 
for a:hover. I have not used Bugzilla before, so apoligies if any are due :o)

I am using the windows binary of M18 on windows 98, and the 'test case that 
works' (referenced in the attachments above) does not work for me.  Neither do 
several other sites I have tested (some with onMouseOver events and some 
without).  Even adding a DocType definition of HTML 4 Transitional (to get 
Mozilla out of quirk rendering mode) does not make the test case work - the 
a:hover pseudo class just does not seem to work for me.

Comment 41

[Doron Rosenberg (IBM)]

[e-mail]: this is a known bug, and was fixed post m18. In
fact, this actually bug depends on that particular bug (5693).  You should try a
new nightly build that you can get off mozilla.org's man page.

Comment 42

[Boris Zbarsky [:bzbarsky]]

*** Bug 62531 has been marked as a duplicate of this bug. ***

Comment 43

[Hixie (not reading bugmail)]

Reassigning QA Contact for all open and unverified bugs previously under Lorca's
care to Gerardo as per phone conversation this morning.

Comment 44

[Tim Powell]

I'm confused by Doron Rosenberg's comments that this was fixed post-m18. It's
still broken with build 2001022805 WinNT (as in the first testcase does not show
the hover effect as it should). Was Hyatt's "trivial patch" ever checked in? Was
there a regression?

Comment 45

[Jesse Ruderman]

*** Bug 70660 has been marked as a duplicate of this bug. ***

Comment 46

[Doron Rosenberg (IBM)]

I was talking about a:hover bug the reporter mentioned. sorry for the confusion

Comment 47

[Heikki Toivonen (remove -bugzilla when emailing directly)]

The netflix page seems to be working fine (tested on a Windows build from this
week).

Gerardo, could we get some test coverage on the current situation, what
platforms etc?

Comment 48

[Tim Powell]

Does the "trivial fix" that Hyatt mentioned last October still exist? Can we get
a look at it and get this one fixed?

The testcase still fails with the 2001040304 build on WinNT4. Netflix may have
changed the site.

Comment 49

[David Hyatt]

Yeah, this is trivial to fix.  Do you want me to take the bug?

Comment 50

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Here you go ;)

Comment 51

[Boris Zbarsky [:bzbarsky]]

*** Bug 77332 has been marked as a duplicate of this bug. ***

Comment 52

[Boris Zbarsky [:bzbarsky]]

removing old keywords.  Nominating for beta1 and mozilla0.9.1, especially as
this has an easy fix!

Comment 53

[Peter Trudelle]

This doesn't fit the profile for 0.9.2, but could still be landed in 'limbo'. 
->0.9.3

Comment 54

[Madhur Bhatia]

QA contact updated

Comment 55

[Jesse Ruderman]

top100: happens on links to external sites at http://www.altavista.com/, because
Altavista uses mouseovers to hide the fact that links to external sites are
routed through a counter script.

Comment 56

[Matthias Versen [:Matti]]

*** Bug 92855 has been marked as a duplicate of this bug. ***

Comment 57

[Jacek Piskozub]

Marking mostfreq at 10 dups.

Comment 58

[David Hyatt]

Attachment: Patch that fixes hover for onmouseout/onmouseover of anchors.

Comment 59

[Brian Ryner (not reading)]

r=bryner

Comment 60

[Chris Waterson]

sr=waterson

Comment 61

[David Hyatt]

Fixed.

Comment 62

[luis rincon]

verified on build 2001-08-06-trunk

Comment 63

[Harald Glatt]

http://www.dvdboard.de/FAQ/ upper frame mouseover still doesn't work - can
someone verify and reopen the bug?

Comment 64

[David Hyatt]

That's failing for a different reason... my guess would be the tag name check
perhaps?

At any rate, it isn't the same bug as this one, which addressed a particular
type of mouseover problem.

Comment 65

[John Morrison]

The other page is doing hovers with js event handlers (which is sheer
genius: why use 'A:hover' when a bunch of fragile code will do!!)
 
The JS is specific to IE's model, although it's not anything that
can't be done in the W3C model (but need to use some other way to set
up an event listener for the document, need to pass |event| as an arg 
to the handlers, need to use "target/relatedTarget" instead of 
"toElement/fromElement", etc.)

But for this bug, the testcase 5/19 is working in 8/10 win32/linux/mac.

Comment 66

[Harald Glatt]

so I'm reopening my old bug 92855 then!

Comment 67

[Marek Z. Jeziorek]

David, could we get this on 0.9.2 branch?

Comment 68

[Andrei Leo]

Doesn't work for me in build 2002012903/Win2000

might be regression connected to the checking-in fix for bug 990099.

Comment 69

[Andrei Leo]

Sorry, the bug number in previous comment should be bug 99009

Comment 70

[Eric Tribou]

I can confirm this bug exists in 0.9.8 Windows (2000) release (Build ID: 2002020406)

Comment 71

[Boris Zbarsky [:bzbarsky]]

I see this in linux nightly trunk build 2002-02-02-22 and linux CVS build from
2002-02-06 as well.  OS -> ALL.  Backing out the patch to bug 99009 does not help.

Comment 72

[Boris Zbarsky [:bzbarsky]]

*** Bug 125221 has been marked as a duplicate of this bug. ***

Comment 73

[jag (Peter Annema)]

-> me, regression, -> 0.9.9, nsbeta1

Comment 74

[jag (Peter Annema)]

-> me, really.

Comment 75

[jag (Peter Annema)]

Attachment: Only skip status setting text, not other side effects

Comment 76

[Johnny Stenback (:jst)]

Comment on attachment 69428 [details] [diff] [review]
Only skip status setting text, not other side effects

sr=jst

Comment 77

[Brian Ryner (not reading)]

Comment on attachment 69428 [details] [diff] [review]
Only skip status setting text, not other side effects

r=bryner

Comment 78

[Peter Trudelle]

nsbeta1+ per ADT triage team

Comment 79

[jag (Peter Annema)]

Checked this in Feb 13.

Comment 80

[Boris Zbarsky [:bzbarsky]]

*** Bug 129828 has been marked as a duplicate of this bug. ***

Comment 81

[David Baron :dbaron:]

For the record, the fix for this bug is modified in the fix for bug 5693 (which
removes the hack in nsGenericHTMLElement::HandleEventForAnchors and thus moves
the fix for this bug into the ESM).

Comment 82

[Hixie (not reading bugmail)]

in the mean time, VERIFIED FIXED using attachment 8389 [details].

Comment 83

[Jerry Baker]

Mass removing self from CC list.

Comment 84

[Jerry Baker]

Now I feel sumb because I have to add back. Sorry for the spam.