Bug 20246 - Implement RenderTheme::systemColor() for Windows
: Implement RenderTheme::systemColor() for Windows
Status: RESOLVED FIXED
: WebKit
Platform
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2008-07-31 15:41 PST by
Modified: 2008-10-12 16:59 PST (History)


Attachments
Call through to GetSysColor() for RenderThemeSafari::systemColor() (3.74 KB, patch)
2008-07-31 15:44 PST, Eric Roman
hyatt: review-
Review Patch | Details | Formatted Diff | Diff
I see. Moved the change from RenderThemeSafari to RenderThemeWin. (3.68 KB, patch)
2008-08-01 14:24 PST, Eric Roman
hyatt: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-07-31 15:41:17 PST
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 From 2008-07-31 15:44:43 PST -------
Created an attachment (id=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 From 2008-08-01 12:23:48 PST -------
(From update of attachment 22590 [details])
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 From 2008-08-01 12:25:59 PST -------
(From update of attachment 22590 [details])
The point of RenderThemeSafari is to match the look of OS X and not the look of Windows.
------- Comment #4 From 2008-08-01 12:26:53 PST -------
RenderThemeWin (if someone used it) would be more suitable for this patch, since that theme is about matching the native Windows look.
------- Comment #5 From 2008-08-01 14:24:57 PST -------
Created an attachment (id=22611) [details]
I see. Moved the change from RenderThemeSafari to RenderThemeWin.
------- Comment #6 From 2008-08-01 15:01:33 PST -------
(From update of attachment 22611 [details])
r=me
------- Comment #7 From 2008-10-12 16:59:26 PST -------
http://trac.webkit.org/changeset/37537