Bug 29652 - RenderThemeChromiumWin should use true system colors for CSS system colors
Summary: RenderThemeChromiumWin should use true system colors for CSS system colors
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Peter Kasting
Depends on:
Reported: 2009-09-22 14:17 PDT by Peter Kasting
Modified: 2009-09-22 15:05 PDT (History)
0 users

See Also:

patch v1 (3.94 KB, patch)
2009-09-22 14:19 PDT, Peter Kasting
levin: review+
levin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Kasting 2009-09-22 14:17:50 PDT
Right now we fall back to the default hardcoded values in RenderTheme.  This means that e.g. the search buttons on google.com (which have recently changed to using CSS system colors) are illegible in white-on-black system themes.

RenderThemeWin.cpp has some appropriate-looking code, so the fix is easy: copy it.
Comment 1 Peter Kasting 2009-09-22 14:19:13 PDT
Created attachment 39944 [details]
patch v1
Comment 2 David Levin 2009-09-22 14:41:35 PDT
Comment on attachment 39944 [details]
patch v1

Two minor nits to fix up on landing.

> Index: WebCore/rendering/RenderThemeChromiumWin.cpp
> ===================================================================
> +// Map a CSSValue* system color to an index understood by GetSysColor

Please add a period to the end of the sentence.

> +static int cssValueIdToSysColorIndex(int cssValueId)
> +{
> +    switch (cssValueId) {
> +        case CSSValueActiveborder: return COLOR_ACTIVEBORDER;

A case label should line up with its switch statement (see http://webkit.org/coding/coding-style.html)
Comment 3 Eric Seidel (no email) 2009-09-22 14:53:35 PDT
Comment on attachment 39944 [details]
patch v1

Looks sane to me too.  Dave just beat me!
Comment 4 Peter Kasting 2009-09-22 15:05:40 PDT
Both comments addressed and patch landed in r48652.