Bug 6431 – REGRESSION: style change where :hover changes only an :after style doesn't work

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[arnau siches]

Using pseudo-elements (:after or :before) with generated content, pseudo-class
:hover and positioned as position:absolute causes Safari 2.0.2 to quit unexpectedly.

[arnau siches]

This bug is resolved in Safari 2.0.3 (with Mac OS X 10.4.4)

[Alexey Proskuryakov]

ToT behaves erratically on this test for me - the text doesn't change on each mouse in/out. 

See also bug 6821

[Joost de Valk (AlthA)]

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

[Joost de Valk (AlthA)]

6924 has a testcase for this as well.

[Joost de Valk (AlthA)]

Marking as Regression, as i just realized it is one, hence also P1 major.

The problem seems to be simply that NodeImpl::diff() doesn't consider pseudo styles. If you make the base style for :hover different, recalcStyle() will notice the difference and update correctly. You can verify by adding
        div:hover {
            color: maroon;
            }
to the testcase.

[Darin Adler]

Particularly telling is the comment in RenderStyle::operator== that says:

    // compare everything except the pseudoStyle pointer

Clearly at the very least we need a version of == that *does* compare the pseudoStyle pointer.

Created attachment 6413 [details]
work in progress

The first part of this unfinished patch is about making diff notice :before and :after. It's not quite right yet. The second part is needed because without it, in the detach-attach sequence, the detach resets the hovered state (as part of the fix for bug 5983 and bug 6821), making the attach compute the non-hovered style, causing infinite looping. That part probably needs work too, in case the hovered node does detach permanently.

Created attachment 6780 [details]
Patch

[Dave Hyatt]

Comment on attachment 6780 [details]
Patch

You shouldn't have to do a detach just because :before/:after content changed.

All you have to do is set the new RenderStyle, and the rest will be taken care of by updatePseudoChild.

A change of NoInherit returned from the diff function should be sufficient to cause the right thing to happen.

[Alice Liu]

<rdar://problem/4483816>

[Beth Dakin]

Created attachment 7256 [details]
Patch using NoInherit instead of Detach

Here is a new patch based on Mitz's patch that uses NoInherit instead of Detach.

[Beth Dakin]

Created attachment 7261 [details]
Prettier patch made after talking with Hyatt

[Beth Dakin]

Created attachment 7262 [details]
Even prettier

An even prettier patch after talking with Hyatt and Darin. Is that comment clear enough?

[Darin Adler]

Comment on attachment 7262 [details]
Even prettier

Needs a test, but still: r=me

[Eric Seidel (no email)]

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