Bug 20246

Summary:

Implement RenderTheme::systemColor() for Windows

Product:

WebKit

Reporter:

Eric Roman <eroman>

Component:

Platform

Assignee:

Eric Roman <eroman>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aroben, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

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+

Eric Roman

Reported 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.

Attachments
Call through to GetSysColor() for RenderThemeSafari::systemColor() (3.74 KB, patch) 2008-07-31 15:44 PDT, Eric Roman	hyatt: review-	Details Formatted Diff Diff
I see. Moved the change from RenderThemeSafari to RenderThemeWin. (3.68 KB, patch) 2008-08-01 14:24 PDT, Eric Roman	hyatt: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eric Roman

Comment 1 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.

Eric Seidel (no email)

Comment 2 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.

Dave Hyatt

Comment 3 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.

Dave Hyatt

Comment 4 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.

Eric Roman

Comment 5 2008-08-01 14:24:57 PDT

Created attachment 22611 [details] I see. Moved the change from RenderThemeSafari to RenderThemeWin.

Dave Hyatt

Comment 6 2008-08-01 15:01:33 PDT

Comment on attachment 22611 [details] I see. Moved the change from RenderThemeSafari to RenderThemeWin. r=me

Darin Adler

Comment 7 2008-10-12 16:59:26 PDT

http://trac.webkit.org/changeset/37537

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