Bug 109230

Summary: REGRESSION(r130089): Scrollbar thumb no longer re-rendered on hover
Product: WebKit Reporter: Mihai Parparita <mihaip>
Component: Layout and RenderingAssignee: Dimitri Glazkov (Google) <dglazkov>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, ap, bdakin, dglazkov, eric, esprehn+autocc, jonlee, macpherson, menard, nbarth, ojan.autocc, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: HasReduction, InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Test case (interactive)
none
Cleanup reftest
none
Test case (interactive) (actual file)
none
Update reftest: also style scrollbar itself
none
Patch none

Description Mihai Parparita 2013-02-07 13:59:39 PST
Test case: http://persistent.info/chromium/test-cases/scrollbar-hover.html

If you hover over the main indicated div, its contents turn green. However, the scrollbar thumb (styled via a #scrollable:hover::-webkit-scrollbar-thumb rule) stays red, instead of also turning green (the thumb only changes color when it itself is hovered over).

This used to work as expected before r130089 (is also the behavior seen in Safari 6.0.2 and Chrome 23).
Comment 1 Alexey Proskuryakov 2013-02-11 18:34:23 PST
Dimitry, do you plan to look into this? Or can r130089 be rolled out?
Comment 2 Alexey Proskuryakov 2013-02-11 18:35:00 PST
Rolling out seems unlikely, given how old this revision is.
Comment 3 Dimitri Glazkov (Google) 2013-02-11 19:31:32 PST
Yikes. I totally missed this somehow. I am not planning to tackle this soon.
Comment 4 Alexey Proskuryakov 2013-02-12 19:03:36 PST
<rdar://problem/13204864>
Comment 5 Dimitri Glazkov (Google) 2013-02-12 19:09:00 PST
Nils, is this something you might be interested in looking into?
Comment 6 Nils Barth 2013-02-12 20:14:51 PST
(In reply to comment #5)
> Nils, is this something you might be interested in looking into?

Sure, I can take a look at it!
Comment 7 Nils Barth 2013-02-20 04:00:36 PST
To add some links; apparently:
r130089: <http://trac.webkit.org/changeset/130089>
which applied Patch 166548 to fix Bug 97953 - Kill transitive effects of SelectorChecker::checkOneSelector.
https://bugs.webkit.org/attachment.cgi?id=166548&action=review
caused this regression.
Comment 8 Dimitri Glazkov (Google) 2013-02-20 07:15:21 PST
(In reply to comment #7)
> To add some links; apparently:
> r130089: <http://trac.webkit.org/changeset/130089>
> which applied Patch 166548 to fix Bug 97953 - Kill transitive effects of SelectorChecker::checkOneSelector.
> https://bugs.webkit.org/attachment.cgi?id=166548&action=review
> caused this regression.

http://www.youtube.com/watch?v=Ya2xifdO_l0 :-\
Comment 9 Dimitri Glazkov (Google) 2013-02-20 09:39:01 PST
I didn't realize I caused this. I'll look into this tonight.
Comment 10 Nils Barth 2013-02-22 05:30:22 PST
Created attachment 189752 [details]
Test case (interactive)

Uploading Mihai's test case for reference.
Comment 11 Nils Barth 2013-02-22 05:34:19 PST
Created attachment 189754 [details]
Cleanup reftest
Comment 12 Nils Barth 2013-02-22 05:38:19 PST
Comment on attachment 189752 [details]
Test case (interactive)

Original manual test obsoleted by reftest.
Comment 13 Nils Barth 2013-02-22 05:40:02 PST
Created attachment 189757 [details]
Test case (interactive) (actual file)

Original test case (for reference; including actual document, not just URL -- oops!)
Comment 14 Nils Barth 2013-02-22 05:40:29 PST
Comment on attachment 189757 [details]
Test case (interactive) (actual file)

Obsoleted by reftest
Comment 15 Nils Barth 2013-02-22 05:55:50 PST
Created attachment 189761 [details]
Update reftest: also style scrollbar itself
Comment 16 Nils Barth 2013-02-22 05:58:23 PST
Just uploaded a reftest.

Looked into this a bit -- as Mihai notes, this looks to be a *re-rendering* issue.
Hover correctly updates the CSS (per DOM Inspector), but the scrollbar thumb pseudo-element isn't re-rendered.
This also affects the scrollbar (background) itself; it's not something special to the thumb.

Dimitri, as you wrote the patch fixing the earlier bug, you're naturally best-placed to address this, but if you'd like me to look into it further I'd be happy to.
(Writing the reftest didn't require understanding what's going on under the hood, so that was easy enough.)
Comment 17 Dimitri Glazkov (Google) 2013-02-22 08:52:51 PST
(In reply to comment #16)
> Just uploaded a reftest.

Bless you. A true Barth.
Comment 18 Dimitri Glazkov (Google) 2013-02-22 08:53:43 PST
This looks promising.
Comment 19 Dimitri Glazkov (Google) 2013-02-22 14:37:27 PST
Created attachment 189832 [details]
Patch
Comment 20 Eric Seidel (no email) 2013-02-22 14:43:55 PST
Comment on attachment 189832 [details]
Patch

er mer gerd
Comment 21 WebKit Review Bot 2013-02-22 17:17:01 PST
Comment on attachment 189832 [details]
Patch

Clearing flags on attachment: 189832

Committed r143819: <http://trac.webkit.org/changeset/143819>
Comment 22 WebKit Review Bot 2013-02-22 17:17:06 PST
All reviewed patches have been landed.  Closing bug.