Bug 20246

Summary: Implement RenderTheme::systemColor() for Windows
Product: WebKit Reporter: Eric Roman <eroman>
Component: PlatformAssignee: Eric Roman <eroman>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Call through to GetSysColor() for RenderThemeSafari::systemColor()
hyatt: review-
I see. Moved the change from RenderThemeSafari to RenderThemeWin. hyatt: review+

Description Eric Roman 2008-07-31 15:41:17 PDT
CSS system colors (http://msdn.microsoft.com/en-us/library/aa358804.aspx) are hard coded constants on Windows Safari. Instead these should query windows.
Comment 1 Eric Roman 2008-07-31 15:44:43 PDT
Created attachment 22590 [details]
Call through to GetSysColor() for RenderThemeSafari::systemColor()

-I think I need to do a parallel change to RenderThemeWin.cpp, but not sure.
-Not sure what sort of test to write for this, since the expected value will depend on the theme.
Comment 2 Eric Seidel (no email) 2008-08-01 12:23:48 PDT
Comment on attachment 22590 [details]
Call through to GetSysColor() for RenderThemeSafari::systemColor()

Looks OK.  The switch style is probably wrong by WebKit standards.  I'd almost consider using a HashMap here, except that'd be a bunch of memory on the heap for no real need.

I wonder if we need any kind of caching here?  Color does some amount of caching, but I don't think that applies here.

Adam Roben should comment.
Comment 3 Dave Hyatt 2008-08-01 12:25:59 PDT
Comment on attachment 22590 [details]
Call through to GetSysColor() for RenderThemeSafari::systemColor()

The point of RenderThemeSafari is to match the look of OS X and not the look of Windows.
Comment 4 Dave Hyatt 2008-08-01 12:26:53 PDT
RenderThemeWin (if someone used it) would be more suitable for this patch, since that theme is about matching the native Windows look.

Comment 5 Eric Roman 2008-08-01 14:24:57 PDT
Created attachment 22611 [details]
I see. Moved the change from RenderThemeSafari to RenderThemeWin.
Comment 6 Dave Hyatt 2008-08-01 15:01:33 PDT
Comment on attachment 22611 [details]
I see. Moved the change from RenderThemeSafari to RenderThemeWin.

r=me
Comment 7 Darin Adler 2008-10-12 16:59:26 PDT
http://trac.webkit.org/changeset/37537