[webkit-reviews] review denied: [Bug 17483] Implement Scrollbars on Windows (Cairo) : [Attachment 19395] Revise to use proper 17-pixel width scrollbars

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 27 13:51:18 PST 2008


Adam Roben (aroben) <aroben at apple.com> has denied Brent Fulgham
<bfulgham at gmail.com>'s request for review:
Bug 17483: Implement Scrollbars on Windows (Cairo)
http://bugs.webkit.org/show_bug.cgi?id=17483

Attachment 19395: Revise to use proper 17-pixel width scrollbars
http://bugs.webkit.org/attachment.cgi?id=19395&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
+	 (WebCore::RenderThemeWin::determineState): Extend to recognize
+	   scrollbar parts.
+	 (WebCore::RenderThemeWin::paintButton): Add new implementation
+	   for drawing buttons given a graphic context and bounding box.

I don't see any callers of this new RenderThemeWin method, so I don't think you
should add it or change determineState. Maybe I'm missing something?

It would be good if the changes to make RenderThemeWin use the SOFT_LINK macros
could be broken out into their own patch.

+// Note:  Copied from RenderThemeWin.	There should be some way to better
+// encapsulate/share this stuff.  See all RenderThemeSafari for more such
stuff...
+//
+// After discussion with Adam Roben, we will make the ugly choice of copying
logic
+// from RenderThemeWin until the Theme logic is refactored

I think a shorter comment would suffice, such as:

// FIXME: Refactor this soft-linking code in a way that can be shared with
RenderThemeWin

Can you split up the patch as suggested above and re-upload it? That will make
it much easier to review.


More information about the webkit-reviews mailing list