[ESM/CSS] :hover should be hierarchical

VERIFIED FIXED in mozilla1.0

Status

()

Core
Event Handling
P3
normal
VERIFIED FIXED
18 years ago
3 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

(Depends on: 1 bug, Blocks: 3 bugs, 7 keywords)

Trunk
mozilla1.0
css2, helpwanted, highrisk, regression, relnote, testcase, top100
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Hixie-P1] relnote-devel (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing, URL)

Attachments

(9 attachments, 3 obsolete attachments)

404 bytes, text/html
Details
766 bytes, text/html
Details
547 bytes, text/html
Details
2.25 KB, text/html
Details
1.43 KB, patch
Details | Diff | Splinter Review
4.38 KB, patch
Details | Diff | Splinter Review
5.28 KB, patch
Details | Diff | Splinter Review
734 bytes, text/html
Details
63.51 KB, patch
Brian Ryner (not reading)
: review+
David Hyatt
: superreview+
jesup
: approval+
Details | Diff | Splinter Review
(Assignee)

Description

18 years ago
Right now :active and :hover don't quite work right on non-link elements.  The
problems I see are as follows:

* They don't work if the click or the hover is over an inline element (i.e.,
any text).
* They don't work on multiple elements at once.  That is, a parent and its
child can both have :hover rules.  If the mouse is in the child's area (and in
the parent's, which is usually implied), then both should have their :hover
styles applied.

That is, they should be triggered roughly be events.

:active = between onMouseDown and onMouseUp (except that the onMouseUp doesn't
have to be in the element's area)
:hover = between onMouseOver and onMouseOut
:focus = between onFocus and onBlur

The above URL demonstrates the problem:

http://www.fas.harvard.edu/~dbaron/csstest/sec051103b

and I have a demonstration in javascript that shows how I think it should work
(except for the above comment about :active):

http://www.fas.harvard.edu/~dbaron/tests/nglayout/sec051103b_js

Updated

18 years ago
Status: NEW → ASSIGNED

Comment 1

18 years ago
Okay, maybe I'm just missing something here but its not at all clear to me that
the CSS spec says anything about what you should do with hover on a parent and
hover on a child.  If anyone has a URL to point me to I'd appreciate it.

Peter, you're more involved with this than I.  Any thoughts?

I do agree with the inline elements bit, though.  For DOM purposes text is its
own content and can have its own event handlers so the pseudo class 'events'
are getting grabbed by it.  I'll have to fix it but depending on what we decide
to do for parent/child relationships it might fix itself.
It wouldn't make sense if pseudo-classes only affected the top most element and
not any of their parents.

For example, imagine the following:
   <A ...> Hello <strong> Wonderful </strong> World! </A>

If you applied the rules
   A:hover { font-weight: bold; }
   STRONG:hover { color: red; }

... and your cursor hovers over the link, you expect the whole link to get bold. If you
hover over the strong bit, the link should still be bold, otherwise it looks like the
middle word is not part of the link!

IMHO, the :active and :hover pseudo-classes should apply to all elements that are
_at that point_. That is, it might not necessarily apply straight up the tree.

For example:

   <DIV style="position: fixed; top: 10px; left: 10px; width: 10px; height: 10px;">
   ONE
   </DIV>
   <DIV style="position: fixed; top: 15px; left: 10px; width: 10px; height: 10px;">
   TWO
   </DIV>

The two DIV's overlap, and so both could have ":hover" applied to them at the same
time. If the styles had the rule ":hover { color: red; }", then if you positioned your
cursor carefully, you should be able to make _both_ DIVs go red.

The editor team probably have some code already written to work out what
elements are at a point on the canvas, should that be required.

Comment 3

18 years ago
I have to agree with David regarding :hover. I've raised this issue with the CSS
w/g and have received very little response, but what I have heard concurs with
this opinion (ie: :hover applies to the entire frame geometric nesting under
the mouse, not the content tree, and not just the top most frame or the frame
that accepted the mouse events).

(I think that the way :hover is implemented today is also useful, but not what
was indented for :hover. I think :hover is being overloaded a bit to do too many
things. I'll take that up with the w/g, perhaps we'll invent a new pseudo class
or two.)

:active should work essentially like it does now. ie: simply mousing down on
anything doesn't make it active. The content has to be able to become "active"
in some way that makes sense for the content. Most content can not become
active.

CSS3 has a UI proposal whereby you can stylistically describe what content can
become active, and how (ie: is it a button, a pulldown, a editable field,
etc...). Until then, the current system works.
(Assignee)

Comment 4

18 years ago
I disagree about :active.  I think anything that can have an
onClick/onMouseDown/onMouseUp/etc. handler can be active.  That is, some
element could be like a link, but handled through javascript event handlers.
(I think that's a better technique than creating a dummy link element that
doesn't actually have a real HREF, and putting handlers on that.)

Anyway, right now, active *does* work on block level elements, but only the
topmost one.  That should definitely be changed, either to Peter's solution or
to mine, or something else logical.

So should :active work on:

 - only links?
 - only elements where clicking does something, and only be set on the
   element that handles the click?
 - anything, and the way hover works?
I strongly support the "anything, and the way hover works" point of view.

This allows all the previous possibilities, does not restrict what can be done,
and is the most generic option (implying that less hard coding will be
involved i.e. less time will be spent implementing it).

If someone does not want _all_ their documents' elements to be affected by an
:active rule, then it is simply a matter of making it specific to what is
actually wanted.

In other words, instead of

   :active { color: red; }

...one would use

   a:active { color: red; }

This means that links containing other elements can still be made active. For
example, imagine the following (which is intentionally similar to the :hover
argument I used above):

   <A ...> Hello <strong> Wonderful </strong> World! </A>

If you applied the rules

   A:active { color: red; }
   STRONG:active { font-size: 150%; }

...and you click on the link, you expect the whole link to go red. If you
click over the strong bit, the link should still go red, otherwise it looks
like the middle word is not part of the link!
The following test page should also come in useful:
   http://www.bath.ac.uk/%7epy8ieh/internet/eviltests/pseudoclasses1.html
It also demonstrates some problems you have with :hover and :active on links.
(Assignee)

Comment 7

18 years ago
I've slightly changed my opinion on the meaning of :hover and :active states.  I
think that:

* An element matches :hover when the mouse pointer is within it *or any of its
descendents*.
* An element matches :active when the mouse pointer is depressed within it *or
any of its descendents*.

Think about the following cases and I think you can see why:
* a link with a floating image in it
* a link with a tall image in it (since the height of the link's inline box is
unchanged)
* a link with an absolutely positioned child
etc.

I don't think this applies to :focus, though.

Do you have a Target Milestone for this?  I think this is reasonably important
for DOM applications that use onclick="", etc.
Good point - really, both the geometrics and the ancestry need to be taken into
account. That is, the list of elements matching :hover (:active) should be all
the elements over which the cursor is floating (and clicked) _and_ all their
parents. So if two images-in-links overlap (due to positioning or negative
margins, for example), then both the links will light up.

A simple example:

   IMG { position: absolute; top: 0; left: 0; width: 10px; height: 10px; }
   A { color: blue; }
   A:hover { color: red; }

...with this markup:

   <P><A> Link 1 <IMG SRC="a.gif"> </A></P>
   <P><A> Link 2 <IMG SRC="b.gif"> </A></P>

Whenever the mouse is over the images (the square area 10px by 10px at the top
left of the viewport), both links should turn red.
(Assignee)

Comment 9

18 years ago
I disagree with what Ian said about overlap.  Assuming that, in cases of
overlap, only the element on top gets the events (which are then bubbled...), I
think only the element on top (i.e., receiving the events) should be the one
that matches :active or :hover (along with its ancestors, of course).
What does that do to the following?

   DIV.overlay {
      position: fixed; left: 0; right: 0; top: 0; bottom: 0;
      margin: auto; padding: 0; border: none; z-index: 100;
      background-image: url(animatedbutterflies-lazy.gif);
   }
   DIV.overlay:hover {
      background-image: url(animatedbutterflies-busy.gif);
   }
   DIV.overlay:active {
      background-image: url(animatedbutterflies-frantic.gif);
   }
   :link { color: blue; }
   :link:hover { color: red; }
   :link:active { color: lime; }

...on a document such as:

   <TITLE> Butterfly Haven </TITLE>
   <P> Go to the <A href="next.html"> next page </A> . </P>
   <DIV class="overlay"></DIV>

Are you saying that the <A> link is unclickable, and will not give :hover and
:active feedback?
(Assignee)

Comment 11

18 years ago
The basic problem here is that you're only allowing one element at a time to
match :active or :hover.  (It's reasonable for focus, though.)

I will attach a test case that shows that img:hover doesn't work when it's
inside a link.  Fixing this would probably allow you to get rid of some of the
element-specific code for these pseudo-classes.
(Assignee)

Comment 12

18 years ago
Created attachment 989 [details]
test case for img:hover inside anchors
Summary: problems with :active and :hover on non-links → {css2} problems with :active and :hover on non-links
Whiteboard: [TESTCASE] (py8ieh:syphon testcases for eviltests)
Target Milestone: M12
[Adding some radars...]
[Tentatively setting milestone to M12, post feature cut-off]
*** Bug 13119 has been marked as a duplicate of this bug. ***

Comment 15

18 years ago
Bug 13119 dealt with links within floating elements not receiving focus when a
block-level element was adjacent to the table. Some links on sites such as
http://developer.netscape.com/ and http://www.amazon.com/ cannot receive focus
because of this issue.
Summary: {css2} problems with :active and :hover on non-links → {css2} problems with :active and :hover
We _have_ to do it both ways: all the elements below the cursor and all their
ancestors. Otherwise, links in floats get their events stolen by the blocks
that come after them in the flow.

What we should probably do with the actual click event is pass it to all the
blocks that we made :hover apply to, until one of them eats it. So if we are
over a paragraph in the main flow and a floating link at the same time, they
would be classify as :hover and then :active, but only one of them would trigger
the onclick event -- if the paragraph has an onclick handler then it would be
the paragraph, and otherwise it would be the link (in that order because that
is the stacking order in that case).

But hey, I'm a CSS guy, not a DOM guy, so what you do with the events after
you've made CSS happy is none of my business... ;-)
(Assignee)

Comment 17

18 years ago
No you don't (for that).  The event bugs around floats are separate bugs
(events going to the wrong element).  The events are going to the wrong
elements.

The reason you might have to do it as Ian describes is for pages like
http://www.w3.org/Style/CSS/ , which have negative margins.

Ian - FYI, there are quite a number of bugs on events going to the wrong element
with advanced layout features.
Blocks: 5407

Comment 18

18 years ago
Created attachment 1810 [details]
another test case, this time on a span element
(Assignee)

Comment 19

18 years ago
http://www.w3.org/TR/css3-userint , although it's currently a working draft, is
relevant to this bug.  In particular, see:

http://www.w3.org/TR/css3-userint#pseudo-active
http://www.w3.org/TR/css3-userint#user-input

Comment 20

18 years ago
Moving to m13 because Joki seems to be distracted.

Updated

18 years ago
Target Milestone: M13 → M14
Keywords: css2
Migrating from {css2} to css2 keyword. The {css1}, {css2}, {css3} and {css-moz}
radars should now be considered deprecated in favour of keywords.
I am *really* sorry about the spam...
Target Milestone: M14 → M17
Moving M17. Not required for beta.
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase

Updated

18 years ago
Summary: {css2} problems with :active and :hover → problems with :active and :hover
Blocks: 33736
(Assignee)

Comment 24

18 years ago
See bug 33736 for a discussion of the :hover aspects of this bug.  I think the
:hover problems and the :active problems should be completely separated,
especially considering the CSS3-userint draft (which would cover the :active
issues).

Comment 25

18 years ago
*** Bug 35315 has been marked as a duplicate of this bug. ***

Comment 26

17 years ago
Created attachment 8284 [details]
Test case for A:hover with images inside anchors

Comment 27

17 years ago
I've noticed that the following behaves unexpectedly.

<style>
div:hover{ color: red}
</style>
<div>Divtext</div>

The text it turned red when the line containing the text is moved over, but 
when the text itself is moved over with the mouse, it turns back black.   Is 
this related or should I file a new bug?
(Assignee)

Comment 28

17 years ago
That problem is already covered by this bug.  It is described by the first
bullet point in my initial comment when opening the bug, although I should have
said "node" rather than "element", since the problem occurs with text nodes not
in inline elements (since they steal the :hover state).

Updated

17 years ago
Blocks: 38380

Updated

17 years ago
Blocks: 12518

Comment 29

17 years ago
Adding [ESM/CSS] prefix to bugs in EventStateManager with its generated events 
or CSS pseudo-class management.
Summary: problems with :active and :hover → [ESM/CSS] problems with :active and :hover

Comment 30

17 years ago
Updating Milestone to M18.
Target Milestone: M17 → M18

Comment 31

17 years ago
*** Bug 46213 has been marked as a duplicate of this bug. ***
As per meeting with ChrisD today, taking QA.
QA Contact: janc → py8ieh=bugzilla

Comment 33

17 years ago
I'm seeing this on Linux too, build from 2000-07-29. My site uses a :hover to
change some grey text to black for emphasis and the mozilla bug is bl**dy annoying.

Suggest moving OS to All?

Updated

17 years ago
OS: Windows 95 → All
Keywords: correctness, nsbeta3
Whiteboard: [TESTCASE] (py8ieh:syphon testcases for eviltests) → (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
I am the virtual joki.
Assignee: joki → heikki
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Per discusion with Nisheeth, marking nsbeta3+. Will email ekrock to verify.
Whiteboard: (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3+](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
Heikki -- Just to summarise all our discussions above and thus save you the
confusion of trying to work out exactly how to fix this bug:

The :hover and :active pseudo-classes should match the node hit and all 
ancestor nodes. It should not match any descendant nodes. The 'node hit' should
be the one which would receive any onclick event if the user happened to click
at that very moment.

If you need testcases, give us a shout.
Hardware: PC → All
(Assignee)

Comment 37

17 years ago
No, I think that's only true of :hover.  :active should probably wait for the
CSS3 rules for handling :active, and stay as-is (??) until then.
Bug triage with nisheeth & ekrock: nsbeta3-. This is CSS2 issue that we 
have not promised full coverage. Adding relnote3 keyword.
Keywords: relnote3
Whiteboard: [nsbeta3+](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3-](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
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.
Target Milestone: M18 → Future
Eek! :active is CSS1. :hover is CSS2 but as currently implemented, we would be
preventing adoption of this feature for as long as Netscape 6.0 is in 
significant use.

In addition, this is one of the most often author-requested features. Our
customers want this a *lot*.

Removing [nsbeta3-] to trigger reevaluation. This is one of those cases where
pulling the feature altogether is the alternative to fixing it, but pulling
:hover and :active is likely to get us all shot.
Whiteboard: [nsbeta3-](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing

Comment 41

17 years ago
As I understand it, this feature (conformance) could single-handedly remove the
need for all the damned javascript navigation rollovers by simply defining rules
for :hover and :active in CSS. I would be annoyed if this gets left out of
Netscape 6 release and would fully support it being nsbeta3+ed.

As it is implemented right now, the bug sticks out like a saw(sp?) thumb on
pages like mine: Linuxnewbie.com, move your mouse over the news items on the
right and see that only the whitespace to the right of the text activates the
black emphasis. I'd welcome any corrections if I've actually written it wrong,
but from what I've heard, it's moz being broken. Please fix ! :)
Triaging with Nisheeth: nsbeta3+. Reassigning to joki 'cos he's back from 
sabbatical.

The test cases from David (in the first comment) are great!
Assignee: heikki → joki
Status: ASSIGNED → NEW
Whiteboard: (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3+](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
Target Milestone: Future → ---

Comment 43

17 years ago
Created attachment 13678 [details]
Illustrates broken use of :hover
(Assignee)

Comment 44

17 years ago
Regarding what needs to be done to fix this bug:

Because of the CSS3-userint draft, I'm not sure what should be done with
:active.  I used to think it should work like hover, but now I think it should
probably be limited.  I'm not sure how we would want to do it, though.

:hover is reasonably simple.  It needs to apply to multiple elements at the same
time.  It is explained in bug 33736 and especially bug 45822.
(Assignee)

Updated

17 years ago
Blocks: 44567
I believe we should ignore CSS3 *draft* specifications at this stage. They are
very much in flux (the UI draft even more than others) and should in any case
be backwards compatible. 

The only problem that needs fixing here is to make :active and :hover match
the node hit and all ancestor nodes. It should not match any descendant nodes. 
The 'node hit' should be the one which would receive any onclick event if the 
user happened to click at that very moment.

When fixing this, make sure that :hover'ing or :active'ating a text node (e.g.,
the textual contents of a <P> element) still propagates the :hover to all the
ancestor elements.
also when fixing this, please make sure that the :hover propages from anonymous 
content to it's binding element (and it's ancestors).

That would allow us to for example remove the javascript for xbl demo #2

Comment 47

17 years ago
Okay.  This bug has been around for quite some time so I'm going to close it 
out.  Here is the status:

Basic nested hover should be working now.  All of the attached testcases work 
just fine.  The www.linuxnewbie.com site which is mentioned in here somewhere 
has pretty complex hover behavior and only partially works.  I don't know why 
but I'm opening a new bug about it (bug #52766) since the reasons it doesn't 
work seem to be more complex than the other cases.

:active has not been changed.  For the moment it is working as designed.

So I'm going to mark this fixed and close it.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 48

17 years ago
Works on Linux 2000091506. Adding verifyme keyword.
Keywords: verifyme

Comment 49

17 years ago
All divs from the last attachment
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=13678 keep the outset
border for WinNT 2000091505 across the anchor and code elements, and across the
text of the div.
*** Bug 45822 has been marked as a duplicate of this bug. ***

Comment 51

17 years ago
reopening this because it caused a significant regression in UI performance.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 52

17 years ago
Marking nsbeta3- to get off the beta 3+ radar.  Please nominate this for rtm if
we figure out a low risk fix that does not cause performance degradation while
mousing over chrome.  I believe hyatt is working on coming up with such a fix.
CCing him.
Whiteboard: [nsbeta3+](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3-](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing

Comment 53

17 years ago
*** Bug 54480 has been marked as a duplicate of this bug. ***

Comment 54

17 years ago
Another problem with the :hover patch (besides the common parent check not 
working) is that it is using the content tree and not the frame tree.  This 
means that interleaved anonymous content will not get put into :hover (and it 
should).  This code should not be doing a common parent check by walking content 
nodes.  It should be walking the frame tree.

Comment 55

17 years ago
If this doesn't make it in for RTM, I hope hover as a whole gets disabled... As
it stands right now, the half-support generates some odd-looking behavior.

Comment 56

17 years ago
:hover only applies to elements, so one trivial fix (that I made for :active a 
week or two ago) that would probably really improve things without going the 
whole nine yards would be to never put a node into :hover... look for its 
element parent and put that into :hover.
Keywords: rtm
BTW, as far as I can tell this actually *regressed*, as well as being backed
out. It appears that :link:hover no longer works, for instance (see 
http://www.webstandards.org/css/macie/linkdemo2.html ). This is very much
not a good thing.

We really should fix joki's performance problem for RTM. Our current state is
very bad. Especially since this is one web authors' most liked parts of CSS.

Comment 58

17 years ago
*** Bug 54672 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Blocks: 41819
*** Bug 54836 has been marked as a duplicate of this bug. ***

Comment 60

17 years ago
cc self
Blocks: 53599

Comment 61

17 years ago
Marking rtm- and futuring.  This does not meet the criteria for a stop ship rtm bug.
Target Milestone: --- → Future
RELEASE NOTE ITEM:
   In this release, Mozilla does not correctly support the ':hover' psuedo-
   class, not even on the <a> element as supported in pre-release versions
   and in other browsers. Mozilla will only consider an element to be in
   the :hover state if the mouse cursor is hovering directly over an element,
   that is, not over a child node (such as text). There is no workaround.

Nisheeth: Considering this bug has no workaround and occurs on many (if not
most) top100 sites, could you reconsider? We 'just' need to fix the performance
issues here, and Hyatt seemed to think that that was quite straight forward.

Joki: Do you have any idea how to fix this?
Severity: normal → blocker
Keywords: regression, relnote, top100
Summary: [ESM/CSS] problems with :active and :hover → [ESM/CSS] problems with :hover

Comment 63

17 years ago
Can I toss in an extra please, or will that mean it won't get fixed (per 
Brendan's comments in n.p.m.general) :-)

Comment 64

17 years ago
We should at the very least ensure that the nearest enclosing element gets a 
proper :hover.  This can be done without doing full hierarchical hover.

Comment 65

17 years ago
If it doesn't get fixed, can I suggest a change to the relnotes?

RELEASE NOTE ITEM:
   In this release, Mozilla does not correctly support the ':hover' psuedo-
   class, not even on the <a> element as supported in pre-release versions
   and in other browsers.

   Mozilla will only consider an element to be in the :hover state if the mouse  
   cursor is hovering directly over an element, that is, not over a child node 
   (such as text). There is no workaround.

Talking about child elements will confuse even some intermediate Web developers 
who will surely come looking why their simple CSS won't work. Since this is most 
used on <a> elements, and these have to have children to be visible - so, in 
effect, ":hover" does not work at all.

Putting the details in a separate paragraphs just serves to make a distinction 
between the effect and the cause. The reader sees ":hover doesn't work" and that 
is taken in as one unit. They then see the child element explanation later - as 
a different block of information. This way they do not needlessly get confused 
when they just wanted to see that it doesn't work.

Comment 66

17 years ago
Since (as per other comments) the fix seems to be straightforward, it should be
definitely done for rtm. Many pages use hover effects to increase useability
while minimizing load time.
And btw, every reviewer will characterize this as a big defect in Mozilla/NS 6,
and right he is. After all, this feature is a) widely used and b) already worked
in Mozilla.

PLEASE, fix this.

Comment 67

17 years ago
I like to point out that :hover is not completely broken in html form controls.

In my test case for bug 41819
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=12896), the :hover for
html form controls: radios, checkboxes, buttons (input, submit, reset, button)
and selects work fine.  However, :hover in the other controls do not work correctly.

I would like to suggest that if you can not fix :hover you need to disable it in
these form controls as well.  BUT, I think that this would be a HUGE LOSS for
mozilla.  As a regular participant in the
comp.infosystems.www.authoring.stylesheets, missing :hover support in netscape
has to be the most consistently asked question.  IMHO, no :hover in mozilla (and
netscape6) seams unbelievable and unacceptable considering all the great and
real pseudo conformance since really the beginning of mozilla development.  This
is a most bitter pill to swallow as a web developer on the eve of releasing this
product after years of development.

I can already hear the web standards project getting a new press release ready
praising mozilla for a great browser with super style support and then ending
the article dumbstruck as to why no :hover, yanked a couple of months before
release. A sad day for web developers everywhere. :-(

PLEASE Fix this.

Comment 68

17 years ago
If :hover isn't re-enabled to some degree, the reviewers are going to have a
field day.  E.g, they will ask, as the UI is based upon XUL, why :hover works
fine in the UI (i.e., the Personal Toolbar) but not in the layout of pages.  You
clearly recognize the importance of response to mouseover events in the UI; why
take that feature away from us content authors? I suspect the only reason you
folks thinks this is a minor feature is because you don't look at websites in
browsers other than NS4.x enough.

Comment 69

17 years ago
Please stop spamming!

the following are clear:
No one likes as is.
This needs to be fixed
some of us are tired of the comments which aren't helpful.

Here's a hint: I'm adding keyword: helpwanted.

If you want to help, please contribute, otherwise this is not helping any 
developer.

Yes we'll be reamed if we don't fix this.

What you people may not be aware of is that :hover and :active DO NOT work well 
in xul either. The bug is rather visible in classic in the personal toolbar.

If this is so easy to fix, please click 
http://bugzilla.mozilla.org/createattachment.cgi?id=5693 aka "Create a new 
attachment (proposed patch)", select the patch radio button and attach your 
fix.

Joki/Heiki: I'm resetting the milestone target. If you feel any inclination to 
future it, please please find someone else @netscape.com or a mozilla.org 
contributor who has the knowledge to fix this (I don't, but maybe: 
sjaensch@gmx.net, ryhan_ut@yahoo.com, or ptrourke@ziplink.net do) instead of 
futuring this.
Keywords: helpwanted
Target Milestone: Future → ---

Comment 70

17 years ago
I'll take a look at this.
Assignee: joki → hyatt
Status: REOPENED → NEW
*** Bug 53460 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Blocks: 46683

Comment 72

17 years ago
rtm+ need info, have a partial fix that should make hover work adequately for
N6, sorely needed for tree performance.  small, low-risk patch, fixes highly
visible performance degradation.
Whiteboard: [nsbeta3-](py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3-][rtm+ need info] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
Target Milestone: --- → M19

Comment 73

17 years ago
Created attachment 16179 [details] [diff] [review]
Patch that fixes basic hover (without fixing full hierarchical hover)

Comment 74

17 years ago
David, I've asked Chris Saari to review the patch as he is the resident expert
while joki is away.

Please close bug 55067 once you check in your patch and future this bug.  Bug
55067 was opened to track the fixing of hover without re-enabling the
hierarchical hover notifications.

Thanks!  I'm re-assigning bug 55067 to you.

Comment 75

17 years ago
This patch looks fine to me (other than the printf, which Hyatt said he has 
removed). I'm assuming that targetContent won't be null at this point.
couldn't that previous slow fix be applyed for everything but chrome (or just 
html)? It would be very unfortunate if :hover dosn't work for all descendents 
when desining webpages.
*** Bug 55670 has been marked as a duplicate of this bug. ***

Comment 78

17 years ago
This has had a=waterson and r=saari.  Ready to go.
Status: NEW → ASSIGNED
Whiteboard: [nsbeta3-][rtm+ need info] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3-][rtm+] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing

Comment 79

17 years ago
rtm++
Whiteboard: [nsbeta3-][rtm+] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [nsbeta3-][rtm++] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing

Comment 80

17 years ago
Ok, fixed on branch and trunk.  Removing [rtm++] and futuring.
Keywords: nsbeta3
Summary: [ESM/CSS] problems with :hover → [ESM/CSS] :hover should be hierarchical
Whiteboard: [nsbeta3-][rtm++] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
Target Milestone: M19 → Future

Comment 81

17 years ago
perhaps i'm completely wrong, but does hierarchical :hover also means, without
it you can't have a JavaScript-OnMouseOver-event and a :hover on, for example,
links, check out http://www.los-angeles2019.de/navbar.html; the links have a
OnMouseOver (the status text is changing) and they should have a :hover but they
don't have in Mozilla. If that's true I think it is important to check this bug
COMPLETELY because I won't be the last webdesigner with such a combination!

Comment 82

17 years ago
Your links don't go into :hover because you return true in your onmouseover 
handlers.

I quote from page 201 of the OReilly Javascript reference.

"...[returning true from an onMouseOver handler] tells the browser that it 
should not perform its own default action for the event..."

Since :hover is a default action, returning true from the mouseover handler 
indicates to the browser that it should not put the link into :hover.

Comment 83

17 years ago
Question is, though, is :hover really considered a default action?  It seems 
kind of bogus to me that in order to keep us from replacing the status text you 
have to sacrifice :hover capability.

I wonder if this is a bug.  Ian or dbaron, do you have any thoughts?

Comment 84

17 years ago
IE5 puts the links into :hover even when true is returned from onmouseover.  I 
think we should do the same.
(Assignee)

Comment 85

17 years ago
The issue is arguable - please argue on bug 38380 instead of here.  I tend to 
think :hover should still work regardless of the DOM event handlers.
regarding hierarchical hover:

Is there a reason that the previous slower patch can't be used on html elements?
Please reevaluate for rtm!

Comment 87

17 years ago
imo, :hover IS a default action, and i am supposed to be writing a complete 
testcase suite for return false/return true. Can someone offer a reason that 
return false; would hurt current users?
(Assignee)

Comment 88

17 years ago
Please discuss that on bug 38380, not here.

Comment 89

17 years ago
opps, I think your new patch broke the pointer cursor (as reported today in bug
51220).  I noticed any link that has :hover no longer displays the correct
pointer (hand) normally displayed for links.

Comment 90

17 years ago
Opps wrong bug.

The bug reported today is bug 56094 not 51220 which I think is caused by the new
patch.

Sorry about the SPAM.

Comment 91

17 years ago
Could someone give a concise reason why this has to be fixed for rtm?

Comment 92

17 years ago
I think that JS image rollovers are one of the most common effects used on the 
Internet today. The ability to do the same thing with CSS would be a boon for us 
lowly Web authors. I think it is one of the features most anticipated by the 
community from a CSS-compliant browser, but I may be mistaken.
Also there exists a patch that fixes this. The problem of the patch is that it 
gave performance problems in XUL. I propose that the patch is modifyed to only 
affect html documents.

Comment 94

17 years ago
You don't want to do that.  As I've said before, there are big problems with the
patch. It ends up thrashing because it does too many notifications.  This has
nothing to do with XUL.  It will do it to HTML too.  Until we resolve the
problems, it should not be turned on anywhere.

I understand your desire to have this feature in Netscape 6, but the patch just
isn't ready yet. It's too late.

Comment 95

17 years ago
rtm-
Whiteboard: (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [rtm-] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
to try to fix this myself:
Would it work (performance put aside) if I stepped up the DOM tree from the 
targetContent node and did a

SetContentState(element, NS_EVENT_STATE_HOVER);

on every element along the way?

Whould that cause a reflow of every element just the onces that actually has 
a :hover rule?
My idea is this: step up the tree and check every element if it is in hover 
state. If it is, we are done. If it isn't, put the element in hover state and 
continue.

Hmm, then there is the problem of moving elements out of hover state...

Comment 98

17 years ago
The code should be stepping up the frame tree, not the content tree.  It misses
anonymous frames right now.

Comment 99

17 years ago
I do not know if this is helpful but I have to point out that the select,
checkbox, radio, and button(s) has had :hover remain enabled.  Even with :hover
supposedly disabled.

Since it seems that these form elements are referencing a different "function"
(sorry i'm not a programmer) for :hover than the code area you've been changing
over the past few weeks, might this other function offer some insights on fixing
:hover with other elements.

To see that these elements are still working with :hover, check out test case #3
in bug 41819.

Again I'm sorry if this is not helpful.  I find it odd that these elements can
still use :hover while everything else does not.
(Assignee)

Comment 100

17 years ago
hyatt:  Why do you say we should be walking the frame tree rather than the
content tree?  I think the content tree is better for at least the following
reasons:

 1) CSS selectors match elements in the document tree.  They never match
anything else.  So the only things that should match :hover are elements.  (But
perhaps there are issues with stylesheets attached to XBL anonymous content?)

 2) Often the content and frame trees do not correspond.  For example, <a
href="http://www.foo.com/"><img align=right src="...">blah blah blah</a>.  If
the user hovers over the image, the link should be highlighted.  The frame for
the image is in the floater list of the anchor's closest block parent, and is
not a child of the a's frame.

If XBL anonymous content is an issue, I don't think walking the frame tree
instead is the solution, since it would break other things.

(Jonas Sicking: A simple algorithm to reduce number of nodes notified would be
to build a list of the path from the "out of" node back to the root, and the
"into" node back to the root, and then compare them in parallel, starting with
the root, and only start notifications when you hit the first difference.)

Comment 101

17 years ago
Ok, we don't have to walk the frame tree, but we do have to deal with
XBL-inserted anonymous content.  Just reinforces the fact that I need to store
this info in the content model somehow.
Exactly where is the problem of the current hierarchical hover patch? I can find
only two problems which both are "just" preformance issues:

* Finding the common ancestor of the "oldHover" and "newHover" isn't very
  optimal

* Finding out if a element is in "hovermode" requires walking content tree.

The only way I can think of to fix the second one (and to make the first one
really fast) is to somehow be able to flag a nsIContent that it is currently
beeing hovered. However both of theese problems are something we could live with
on html Documents (right?)

Hyatt: you said mentioned something about some other problems, could you please
explain exactly what is trashed?

Comment 103

17 years ago
I'm not sure of the exact problem.  I used Quantify to determine that when
mousing around among siblings, the common parent check wasn't working right.
Both loops were entered (when neither should have been), and both loops called
extra ContentStateChangeds.

Created attachment 17314 [details] [diff] [review]
Patch to speed up hierarchical hover
I've attatched a patch that fixes unnessesary ContentStatesChaned calls. The
problem was that there was no check to see if the new hover element was a child
to the current element. Also the parent of the new and old hover element was
always notifyed.

I've also speeded up the common-parent search to be O(n) instead of 0(n^2)

(woohooo my first real mozillapatch :-) )

Comment 106

17 years ago
Let me apply this to my build and do test quantify runs to see what happens.

Comment 107

17 years ago
*** Bug 56976 has been marked as a duplicate of this bug. ***

Comment 108

17 years ago
I appliead this patch to my tree and respun. I wasn unable to see any difference 
in performance whatsoever. This obviously requires a machine that is much slower 
than mine is to see. The point being that it is a fine patch as far as not 
degrading anything or breaking any building on Win2k anyway.
Created attachment 17484 [details] [diff] [review]
patch that fixes hierarchical hover and enables it on html documents
I've attatched a patch that does the same thing as my previous one but also
enables hierarchical hover on all html documents (independently of the value of
the pref).

I really think that the first patch is better but if XUL still is to slow then
we could possibley use the new one. The problem with only doing it on html
documents is that it gives inconsitencies which is always bad.

I cant really test the performace on any of my patches since hierarchical hovers
have always been fast on my mashine, even without patches.

removing rtm- for reevaluation since we now have a couple of bugfixes (and
:hover regressed a couple of days ago)
Keywords: verifyme → patch
Whiteboard: [rtm-] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
hmm.. thinking about this my last patch should be modified to enable 
hierarchical hovers on everything but XUL documents rather then just HTML 
documents.

If it should be used at all, I still recomend the first patch
*** Bug 57145 has been marked as a duplicate of this bug. ***
trudelle/hyatt: could you decide if this bug should be [rtm-] or [rtm+] ? Cheers.
If it is plussed then we should get the last attached patch reviewed ASAP.

Comment 114

17 years ago
I would classify this as moderate risk, and therefore don't think it can be
placed into Netscape 6. IMO (as much as I like the feature) this bug should be
marked rtm-.


Comment 115

17 years ago
making it so, rtm-
Whiteboard: (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [rtm-] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
I agree that the second patch is medium risk but the first one is just 
rearranging some while loops. Isn't that fix really enough or is XUL still too 
slow? I can't try it out myself since I've never had any performance problems.

Comon people, shipping NS6 without support for :hover (yeah yeah I know there's 
a special hack for <a>) is really embarrassing. :hover is one of the most 
missed features from NS4 and one that could really give dynamic userinterfaces. 
Also it really shows the power and speed of gecko (something that marketing 
should like ;-) )

Comment 117

17 years ago
I'm not going to waste time going before the PDT to argue this bug.  They will 
not approve it.  There are much less risky bugs that they're already turning 
away.  

It's not a question of whether or not I want it in NS6.  I'd love to see it in 
NS6, but I already know what PDT will say, and so there's really no point in 
+ing it.
Soory, I thought +ing it ment it was PDT approved. All theese ++ and [-- need 
info] are too confusing for me :-)
Hi Jonas -- The situation is as you say: (1) the second patch is medium risk so 
we can't take that patch for N6 RTM. (2) the first patch has been observed to 
cause performance problems for the product as a whole on some platforms/machines 
(although apparently those problems don't occur on your own machine), so we 
can't take that patch for N6 RTM either. Marking ns601. We'll shoot for trying 
to get this into the first major post-RTM point release. Changing Severity 
from blocker to normal; this issue is not a blocker for product engineering or 
QA work.
Severity: blocker → normal
Keywords: ns601

Updated

17 years ago
Keywords: relnote3
Whiteboard: [rtm-] (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [rtm-] relnote-devel (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
Keywords: ns601 → highrisk, mozilla0.9
Keywords: nsbeta1

Comment 120

17 years ago
->moz0.9
Target Milestone: Future → mozilla0.9

Comment 121

17 years ago
->future, until this is better defined.
Target Milestone: mozilla0.9 → Future
(Assignee)

Comment 122

17 years ago
Considering there's a patch on this bug, it really shouldn't be futured.  Could
either hyatt or joki (or someone else?) review the patch from Jonas Sicking in
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=17484 and explain what
needs to be tested to get it checked in or why it shouldn't be?
dbaron's right.  Clearing Target Milestone and reassigning to trudelle for
better assignment.  We need to get a merged-up version of the patch reviewed and
tested.

/be
Target Milestone: Future → ---

Comment 124

17 years ago
> ------- Additional Comments From David Hyatt 2000-10-16 20:31 -------

> Let me apply this to my build and do test quantify runs to see what happens.

What were the results of this? From what I can tell, we need to know if the
patch is "correct", i.e. does it do what is intended while not breaking other
things, and second whether the performance hit it acceptable. I suspect the
latter relies on other performance work which leads me to wonder what if
anything has changed in that respect.

Although my patch should speed things up a bit I don't think the differance is 
that big.

What really should be done is to add a function like:

nsCSSFrameConstructor::HierarchContentStatesChanged(aPresContext, Content1, 
Content2)

where Content1 should be a grandparent to Content2. A function like that should 
be able to do some really heavy optimizations and would be very useful for 
hierarchical :hover (and hierarchical :active)

I'd offer to implement it after I'm done with some other stuff I'm working on 
right now...

Comment 126

17 years ago
From reading the CSS WG mailing list, it looks like the exact definition of
hierarchical hover is still being hammered out.  I'm not sure we should put an
implementation of it into our code until we are sure that we know how exactly
this feature should work.
could somebody with access to that list please ask them to investigate the 
hierarchicalness of :active as well
It's part of the same discussion. (And :focus, as well.)
Blocks: 54646

Comment 129

17 years ago
Bug 65917 covers :active not being hierarchical.  Is that bug a dup of this 
one?  (If so, the summary of this bug should be changed to include :active, 
:hover, and :focus.)

Comment 130

17 years ago
What would make the most sense to me, thinking of the outer element as either 
<body> or <a href>, and the inner element as <input type=text> or <b>:

- if an inner item is hovered over (and isn't itself focusable?), apply the 
hover rule to the outer element.
- if an outer item is activated by clicking on an inner item, apply the active 
rule to the outer element.  I figure the outer item shouldn't be activated if 
the inner item itself takes focus.
- if an inner item has focus, do *not* apply the focus rule to the outer item.  
(This contradicts Hyatt's comment earlier today in bug 42758.)

Comment 131

17 years ago
->moz1.0, in hopes that the css working group will decide on this issue by then.
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 132

17 years ago
The consensus at the F2F seemed to be that we should leave the behavior
unspecified.  See the end of:
http://lists.w3.org/Archives/Member/w3c-css-wg/2001JanMar/0290.html

Comment 133

16 years ago
David, that's a link most of us can't use :(. Can you provide a more detailed
summary so that implementors and testers can have an idea of what the WG intends?

Following on from that, what are we to do on this? I've just encountered another
case where I want the text in <div>foo</div> to have the div:hover rule applied
and yet it gets switched off. Be nice to get that problem sorted.

Hyatt: does Jonas' patch(s) do what is needed for correctness (whatever that may
be), and is the perf hit acceptable? You haven't given those quantify results
yet, and I'm sure the patch probably needs to be merged with a current tree :)

Comment 134

16 years ago
*** Bug 80651 has been marked as a duplicate of this bug. ***

Comment 135

16 years ago
This seem to apply for onMouseOver on DHTML-layers, too. See http://zoomon.com/
- when moving the mouse over inner content of layers (like on the menuitems on
the webpage above), onMouseOver is not triggered, and I feel that is SERIOUS. I
have reported this bug, don't know if you feel it's a duplication of this one.

Comment 136

16 years ago
Traction needed here. High number of cc's, high number of votes, and we
apparently block a good number of other bugs; David B has explained that the
specs leave this issue up to implementors thus we are free to enable this as we
see fit. dbaron and I just spoke about this on irc and believe that having
:hover effects not happen on anonymous content is a bit silly.

Nominating for 0.9.2. We need to test the patches here, update them as
neccessary, check for correctness and speed impact.

Can we have hyatt's attention once his css re-write stuff has been landed please :)
Keywords: mozilla0.9.2

Comment 137

16 years ago
The issue is not anonymous content.  The issue is performance.  The only 
anonymous content cases that would fail would be XBL-interleaved content, which 
I could live with.  I've written code in the last year that makes it trivial to 
deal with that anyway.

I have planned optimizations to make to ReResolveStyleContext, which I want to 
do in 0.9.2, and I would not want to check this bug in until I make those 
optimizations.

Once I do that, hierarchical hover should be pretty safe to turn on, even if 
it's bone-headed. :)

Updated

16 years ago
Target Milestone: mozilla1.0 → mozilla0.9.3

Comment 138

16 years ago
*** Bug 83032 has been marked as a duplicate of this bug. ***

Comment 139

16 years ago
13 dups. Should be made mostfreq long ago. Marking.
Keywords: mostfreq

Comment 140

16 years ago
*** Bug 87797 has been marked as a duplicate of this bug. ***

Comment 141

16 years ago
*** Bug 91186 has been marked as a duplicate of this bug. ***

Comment 142

16 years ago
Doesn't look like this is getting fixed before the freeze tomorrow night.
Pushing out a milestone.  Please correct if I'm mistaken.
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Updated

16 years ago
Target Milestone: mozilla0.9.4 → mozilla1.0
hyatt: given your comment a few months ago in this bug, any chance of a quick
status update?

Gerv

Comment 144

16 years ago
*** Bug 96984 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

16 years ago
Blocks: 96984
(Assignee)

Comment 145

16 years ago
*** Bug 97814 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

16 years ago
Blocks: 98694

Comment 146

16 years ago
Created attachment 49302 [details]
testcase with <li>

Comment 147

16 years ago
I don't see what's so hierarchical about my testcase, seem's pretty simple, as
far as CSS goes... that case not working just killed the whole effect for a page
I was making. Adding Cc and a vote.
Whiteboard: [rtm-] relnote-devel (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [Hixie-P1][Hixie-1.0] relnote-devel (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing

Updated

16 years ago
Blocks: 104166
Blocks: 103709

Comment 148

16 years ago
->99
Target Milestone: mozilla1.0 → mozilla0.9.9
hyatt: are there any further issues preventing sicking from fixing up his work
and getting it checked in?

I think 0.9.9 is rather late to land this change anyway.

(I'm also rather confused that this bug apparently "blocks" five resolved bugs.)

Gerv

Comment 150

16 years ago
Gerv, yes, there are outstanding issues.

while my patch does some cleanup for the code (giving it some performance 
boost) the main performance problems are still there. To fix the real problem i 
think we need to take advangate of the new stylesystem.

Hyatt?

Comment 152

16 years ago
*** Bug 90146 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Blocks: 52766
*** Bug 115479 has been marked as a duplicate of this bug. ***
Depends on: 115462

Comment 154

16 years ago
Any ramifications for bug 93598 and bug 93602?
Whiteboard: [Hixie-P1][Hixie-1.0] relnote-devel (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing → [Hixie-P1] relnote-devel (py8ieh:syphon testcases for eviltests) hit during nsbeta2 standards compliance testing
*** Bug 122190 has been marked as a duplicate of this bug. ***
*** Bug 122979 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0
(Assignee)

Comment 157

16 years ago
*** Bug 124646 has been marked as a duplicate of this bug. ***

Comment 158

16 years ago
Adding keyword nsbeta1 to several bugs.
Keywords: nsbeta1
*** Bug 124210 has been marked as a duplicate of this bug. ***

Comment 160

16 years ago
nsbeta1- per nav triage team, cc barrowman for possible embedding issues. ->1.2
Keywords: nsbeta1 → nsbeta1-
Target Milestone: mozilla1.0 → mozilla1.2

Updated

16 years ago
Keywords: mozilla1.0+
(Assignee)

Comment 161

15 years ago
Created attachment 78073 [details] [diff] [review]
A patch restoring hierarchical hover with some event state style reresolution performance fixes

This patch restores hierarchical hover and makes style reresolution for event
state changes specific to which event state changed.  I also added a comment
about potentially batching style changes -- that would be nice to do,
particularly since the :hover notifications are given in the reverse of the
order that would be optimal for the style system.  However, the state changes
are *incompatible* with batching as I explain in the comment in
nsCSSStyleSheet.  I'm not sure which is really the better solution.  (I think
there are some interesting ways to do batching -- for example, one could sort
by depth in the content tree (lowest to highest), use that sort to quickly
eliminated duplicates (testing each in the list, from lowest to highest, by
walking its parent chain and the prior elements in the list in parallel).)

Another possibility is to remove HasStateDependentStyle completely, and somehow
separate the matching of state-dependent style rules from other style rules,
and merge the two together.  This seems hard, especially given the current
architecture of ReResolveStyleContext (especially w.r.t. sibling/cousin sharing
and the immutability of the mRuleNode of style contexts).

This patch is not yet well tested.  I just wrote it this afternoon/evening.

What performance issues in particular need to be tested?
(Assignee)

Comment 162

15 years ago
*** Bug 136013 has been marked as a duplicate of this bug. ***

Comment 163

15 years ago
I applied the patch. Here are some problems:

* attachment 989 [details] (still?) doesn't work.
* attachment 8284 [details] doesn't work fully. Only part of the img turns blue.
* In attachment 49302 [details], the list item bullets themselves become bigger when you
hover the <LI>s.

I also visited some popular top100 sites and couldn't notice any outstanding
problems.
#163
> * In attachment 49302 [details], the list item bullets themselves become bigger when
> you hover the <LI>s.

Hmm. Do the CSS specs say that this mustn't happen? Without the patch, when
hovering over the bullets (markers), the text is made bold, so Mozilla seems to
recognize the markers as part of the <li>.
(Assignee)

Comment 165

15 years ago
> * attachment 989 [details] (still?) doesn't work.
> * attachment 8284 [details] doesn't work fully. Only part of the img turns blue.

These are because I haven't yet removed the hack that made :hover work for a
elements despite not being hierarchical.  I need to figure out what does that
and remove/modify it.

> * In attachment 49302 [details], the list item bullets themselves become bigger when you
> hover the <LI>s.

This has nothing to do with the current patch, but, rather, has to do with our
treatment of bullets in general.  According to the CSS2 markers concept it's not
a bug.  I'm not sure what the CSS3 list module says.
(Assignee)

Comment 166

15 years ago
Created attachment 78116 [details] [diff] [review]
patch that also fixes anchor hack

The only difference between this patch and the previous is the changes added to
nsGenericHTMLElement.cpp.
Attachment #78073 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #78116 - Attachment description: patch that fixes anchor hack → patch that also fixes anchor hack
(Assignee)

Comment 167

15 years ago
Actually, those added changes are wrong, perhaps.  At the very least, they
regress bug 38380, which was fixed in a rather odd way (very specific to the
anchors case, rather than the general fix).
(Assignee)

Comment 168

15 years ago
Created attachment 78119 [details] [diff] [review]
patch that also fixes bug 38380 in all cases (not just anchors)
Attachment #78116 - Attachment is obsolete: true
Per CSS3 Lists, bullets should grow on :hover, if the text grows, because the
bullets are glyphs and are in pseudo-elements that are children of the list item
elements. (This is the same as CSS2.)
(Assignee)

Comment 170

15 years ago
Taking.
Assignee: hyatt → dbaron
Status: ASSIGNED → NEW
Target Milestone: mozilla1.2alpha → mozilla1.0
(Assignee)

Comment 171

15 years ago
Hyatt said that the main performance problem this caused in the past was with
XUL menus.  I can pretty easily peg my CPU moving the mouse pointer up and down
over my bookmarks menu either with or without the patch -- the patch doesn't
seem to make the pegging any worse, although I might try to measure (somehow)
the speed at which I can move the mouse without causing skipping of the :hover
(which seems to kick in pretty suddenly for some reason).  However, it feels
like the general performance of the two builds is pretty much the same. 
(Caveat:  I'm using a dual P-733 on Linux, although the second processor doesn't
help here since it's all on one thread.)
(Assignee)

Comment 172

15 years ago
Created attachment 78185 [details] [diff] [review]
a few minor tweaks to the previous patch

Just a bunch of comment fixes, a few changes to parameter names, and a fix for
one case where I might have caused one unnecessary notification for some hover
changes (I changed the if and do...while to just while for the hover
notification loop, since we should check against the commonHoverAncestor before
entering the loop.)
Attachment #78119 - Attachment is obsolete: true
Any chance of getting this into 1.0?
(Assignee)

Comment 174

15 years ago
To summarize, this does the following:
 * Changes the state parameter to ContentStatesChanged to be a PRInt32
   mask rather than a pseudo-class to reflect how the ESM works (since
   there are sometimes state changes of multiple states at once).
 * Makes the HasStateDependentStyle code in the style system use that
   parameter so HasStateDependentStyle can be false more often.  (Also
   removes use of NS_COMFALSE.) 
 * Removes the hierarchical hover pref and turns hierarchical hover on 
   permanently.
 * Fixes the O(N^2) nature of the common ancestor finding and cleans up
   other parts of the hierarchical hover notification code a bit 
   (removing some excess notifications too).
 * Removes the hover hack for anchors (i.e., what made :hover work for
   anchors even though :hover wasn't hierarchical) and fixes bug 38380
   in the general case instead of just for that hack.

Comment 175

15 years ago
Is there any particular type of regressions I should look for or pages I should
look at when trying this patch?

Comment 176

15 years ago
David: Does the patch help bug 65917?
(Assignee)

Comment 177

15 years ago
(Re: comment 175) Mostly things related to dynamic style changes for event
states (active, hover, focus, checked), and particularly hover.

(Re: comment 176) No.
Comment on attachment 78185 [details] [diff] [review]
a few minor tweaks to the previous patch

r=bryner
Attachment #78185 - Flags: review+
(Assignee)

Comment 179

15 years ago
FWIW, I just filed bug 136301 on some menu performance issues I found.
(Assignee)

Comment 180

15 years ago
*** Bug 136484 has been marked as a duplicate of this bug. ***

Comment 181

15 years ago
Looks good to me, too.  r=joki

Comment 182

15 years ago
Comment on attachment 78185 [details] [diff] [review]
a few minor tweaks to the previous patch

sr=hyatt.  Test carefully. :)
Attachment #78185 - Flags: superreview+
(Assignee)

Comment 183

15 years ago
Fix checked in to trunk, 2002-04-10 20:48/49 PDT.  I'll ask for branch approval
after RC1 ships (hopefully early next week).
VERIFIED FIXED on the trunk using the W3C Selectors test suite.

dbaron: dude this is sweet. Your next trip to La Fiesta is on me. :-)
Confirmed on 2002-04-11-03, Win32 Trunk. This rocks!
http://www.w3.org/Style/CSS/Test/CSS3/Selectors/20020115/html/tests/css3-modsel-63.html

On that page, the first paragraph does not turn green when hovered, but the
second does.

Comment 187

15 years ago
Replying to #186
This page uses :not pseudoclass. It is'nt issue of CSS2, but CSS3.
#187
Should have noticed. Sorry.
(Assignee)

Comment 189

15 years ago
http://www.w3.org/Style/CSS/Test/CSS3/Selectors/20020115/html/tests/css3-modsel-63.html
should work, since we support :not.  That's probably bug 96984, though.

Comment 190

15 years ago
dbaron, on the site http://www.aberdeeninc.com, when I move the mouse over the
various links, sometimes I get links that don't underline, and other times, the
underlines don't go away after I move the mouse away.  I don't know if this is
related or not.  Trunk build, 2002041103, Win98.
(Assignee)

Comment 191

15 years ago
The underline repainting issue is filed as bug 136935, although I may mark that
as a duplicate of an existing bug.
(Assignee)

Comment 192

15 years ago
I believe the underlining problems on :hover are actually a regression from
Windows font metrics changes that landed yesterday (bug 76097), not this bug.

Comment 193

15 years ago
Thanks for landing this awesome fix!  I think this should hit a lot of
dependencies as well.

I've just dropped this note on bug #41819.  Any ideas why the filepicker doesn't
react to focus?

----

With the changes from 5693, all of the testcases work 100% as expected, with the
exception of the file picker - it doesn't get the :focus style as expected (but
:hover works).

If the file testcase could be fixed, this bug could be closed.  

---
I need more details here:

- do you mean the Linux filepicker dialog, or <input type=file>?
- in either case, what is it that you are trying to focus (and by what means),
and how is it not showing focus?
Matthew means that when you click in the textfield of an <input type="file"> the
:focus style applied to the <input type="file"> is not applied to that textfield
(or to anything else for that matter).

Comment 196

15 years ago
can this have caused bug 137124, bug 137069, bug 137067 and bug 137247?
(Assignee)

Comment 197

15 years ago
So this fix exposed an existing problem in the nsIStyleRuleSupplier interface
and the way nsStyleSet used it, because the breaking-out-of-the-loop that
HasStateDependentStyled dependended on wasn't supported properly.  I'm surprised
nobody noticed this before, and I'm pretty sure my changes didn't make the
problem any worse beyond optimizing HasStateDependentStyle to check only the
state that changed, which probably tweaked it in a bunch of skins where some
states were triggered only by style rules in XBL bindings.

The fix for that problem is attached to bug 137067.  That fix should also go in
with this one if this goes on the mozilla 1.0 branch.
(Assignee)

Comment 198

15 years ago
Never mind about it being an existing bug.  I assumed that break-out
optimization always worked (I think I might have even written it myself too,
knowing that it didn't).  There's a simpler fix.  See bug 137067 for the details.
(Assignee)

Comment 199

15 years ago
*** Bug 119604 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 200

15 years ago
The fix for bug 137556 should also go in to the branch if this goes on the branch.

I also need to investigate whether bug 130999 and bug 138282 were caused by this
patch.
(Assignee)

Comment 201

15 years ago
NOTE:  The fixes for bug 135776 and bug 137067 should go into the branch when
this does.  I have marked those bugs as fixed since they exist on neither trunk
nor branch.
(Assignee)

Comment 202

15 years ago
Er, that should have said:

NOTE:  The fixes for bug 137556 and bug 137067 should go into the branch when
this does.  I have marked those bugs as fixed since they exist on neither trunk
nor branch.
Comment on attachment 78185 [details] [diff] [review]
a few minor tweaks to the previous patch

a=rjesup@wgate.com for branch checkin.	Please include checkin of the two
regression fixes mentioned
Attachment #78185 - Flags: approval+
(Assignee)

Comment 204

15 years ago
Fix checked in to MOZILLA_1_0_0_BRANCH, 2002-04-24 17:23/24 PDT.  (See comment
183 above for trunk checkin.)
Status: NEW → RESOLVED
Last Resolved: 17 years ago15 years ago
Resolution: --- → FIXED
Keywords: fixed1.0.0

Comment 205

15 years ago
This is one of the suspect of causing 140983. 
VERIFIED FIXED on trunk
Status: RESOLVED → VERIFIED

Comment 207

15 years ago
Verified on the branch build 2002-05-22-08-1.0.0 on Windows 2000 , with the test
cases under the URL :
1)http://www.fas.harvard.edu/~dbaron/csstest/sec051103b 
2)http://www.fas.harvard.edu/~dbaron/tests/nglayout/sec051103b_js 

adding verified1.0.0 to the keyword
Keywords: verified1.0.0

Comment 208

15 years ago
Mass removing self from CC list.

Comment 209

15 years ago
Now I feel sumb because I have to add back. Sorry for the spam.
You need to log in before you can comment on or make changes to this bug.