[Webkit-unassigned] [Bug 52826] [Chromium] Use WebThemeEngine for relevant RenderTheme parts for chromium/linux

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52826


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #79628|review?                     |review-
               Flag|                            |




--- Comment #2 from Tony Chang <tony at chromium.org>  2011-01-20 15:29:25 PST ---
(From update of attachment 79628)
View in context: https://bugs.webkit.org/attachment.cgi?id=79628&action=review

r- for style nits.

Can you upload some of image diffs?  I'm having a hard time seeing the difference.  Maybe it's just a few rgb points off?

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:58
> +      return ChromiumBridge::StateHover;

4 space indent here and above 2 conditions

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:196
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartCheckbox,
> +        getWebThemeState(this, o),
> +        rect,
> +        &extraParams);

I think this should just be one long line (maybe one line of wrapping if it's really long).

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:217
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartRadio,

less wrapping

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:238
> +    extraParams.button.backgroundColor = SkColorSetRGB(0xdd, 0xdd, 0xdd);

Should this color be a constant somewhere?

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:244
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartButton,

less wrapping

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:294
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartMenuList,

less wrapping

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:308
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartSliderTrack,

less wrapping

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:323
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartSliderThumb,

less wrapping

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:346
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartInnerSpinButton,

less wrapping

> Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:372
> +    ChromiumBridge::paintThemePart(
> +        i.context,
> +        ChromiumBridge::PartProgressBar,

less wrapping

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.