Bug 6431 - REGRESSION: style change where :hover changes only an :after style doesn't work
Summary: REGRESSION: style change where :hover changes only an :after style doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Major
Assignee: Beth Dakin
URL: http://css.artnau.com/exemples/safari...
Keywords: HasReduction, InRadar, Regression
: 6924 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-01-08 10:23 PST by arnau siches
Modified: 2006-07-17 17:03 PDT (History)
8 users (show)

See Also:


Attachments
work in progress (2.41 KB, patch)
2006-02-11 10:48 PST, mitz
no flags Details | Formatted Diff | Diff
Patch (9.70 KB, patch)
2006-02-28 14:17 PST, mitz
hyatt: review-
Details | Formatted Diff | Diff
Patch using NoInherit instead of Detach (1.87 KB, patch)
2006-03-23 11:46 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Prettier patch made after talking with Hyatt (1.45 KB, patch)
2006-03-23 13:42 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Even prettier (1.48 KB, patch)
2006-03-23 14:14 PST, Beth Dakin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description arnau siches 2006-01-08 10:23:57 PST
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.
Comment 1 arnau siches 2006-01-12 11:33:53 PST
This bug is resolved in Safari 2.0.3 (with Mac OS X 10.4.4)
Comment 2 Alexey Proskuryakov 2006-01-12 13:14:28 PST
ToT behaves erratically on this test for me - the text doesn't change on each mouse in/out. 
Comment 3 mitz 2006-01-27 01:28:00 PST
See also bug 6821
Comment 4 Joost de Valk (AlthA) 2006-01-30 00:24:35 PST
*** Bug 6924 has been marked as a duplicate of this bug. ***
Comment 5 Joost de Valk (AlthA) 2006-01-30 00:25:33 PST
6924 has a testcase for this as well.
Comment 6 Joost de Valk (AlthA) 2006-01-30 00:26:36 PST
Marking as Regression, as i just realized it is one, hence also P1 major.
Comment 7 mitz 2006-01-30 13:31:05 PST
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.
Comment 8 Darin Adler 2006-02-04 09:23:32 PST
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.
Comment 9 mitz 2006-02-11 10:48:15 PST
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.
Comment 10 mitz 2006-02-28 14:17:28 PST
Created attachment 6780 [details]
Patch
Comment 11 Dave Hyatt 2006-02-28 14:44:39 PST
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.
Comment 12 Alice Liu 2006-03-20 05:56:52 PST
<rdar://problem/4483816>
Comment 13 Beth Dakin 2006-03-23 11:46:27 PST
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.
Comment 14 Beth Dakin 2006-03-23 13:42:02 PST
Created attachment 7261 [details]
Prettier patch made after talking with Hyatt
Comment 15 Beth Dakin 2006-03-23 14:14:21 PST
Created attachment 7262 [details]
Even prettier

An even prettier patch after talking with Hyatt and Darin. Is that comment clear enough?
Comment 16 Darin Adler 2006-03-23 14:16:07 PST
Comment on attachment 7262 [details]
Even prettier

Needs a test, but still: r=me
Comment 17 Eric Seidel (no email) 2006-07-17 17:03:04 PDT
*** Bug 9104 has been marked as a duplicate of this bug. ***