[webkit-reviews] review granted: [Bug 21527] Make CSS scrollbars support :hover and :active : [Attachment 24269] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 10 12:17:14 PDT 2008


Adam Roben (aroben) <aroben at apple.com> has granted Dave Hyatt
<hyatt at apple.com>'s request for review:
Bug 21527: Make CSS scrollbars support :hover and :active
https://bugs.webkit.org/show_bug.cgi?id=21527

Attachment 24269: Patch
https://bugs.webkit.org/attachment.cgi?id=24269&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
Can you comment on the changes to :not handling in the ChangeLog?

 2394	      case CSSSelector::PseudoHover: {
 2395		  ScrollbarPart hoveredPart = scrollbar->hoveredPart();
 2396		  if (part == ScrollbarBGPart)
 2397		      return hoveredPart != NoPart;
 2398		  if (part == TrackBGPart)
 2399		      return hoveredPart == BackTrackPart || hoveredPart ==
ForwardTrackPart || hoveredPart == ThumbPart;
 2400		  return part == hoveredPart;
 2401	      }
 2402	      case CSSSelector::PseudoActive: {
 2403		  ScrollbarPart pressedPart = scrollbar->pressedPart();
 2404		  if (part == ScrollbarBGPart)
 2405		      return pressedPart != NoPart;
 2406		  if (part == TrackBGPart)
 2407		      return pressedPart == BackTrackPart || pressedPart ==
ForwardTrackPart || pressedPart == ThumbPart;
 2408		  return part == pressedPart;
 2409	      }

Maybe it's worth putting this code into a function so we don't have to
duplicate it?

 99 void RenderScrollbar::setPressedPart(ScrollbarPart part)
 100 {
 101	 ScrollbarPart oldPart = m_pressedPart;

Should we check part != m_pressedPart before continuing?

r=me if you land a manual test with it.


More information about the webkit-reviews mailing list