Bug 102939 – [chromium] Change default tap highlight color for Chromium Windows & Linux.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[W. James MacLean]

[chromium] Change default tap highlight color for Chromium Windows & Linux.

[W. James MacLean]

Created attachment 175438 [details]
Patch

[Adam Barth]

Comment on attachment 175438 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175438&action=review

> Source/WebCore/rendering/RenderThemeChromiumSkia.h:50
> +        virtual Color platformTapHighlightColor() const OVERRIDE

The indent in this file is really odd.

Does it matter that defaultTapHighlightColor has type RGBA32 and the return type of this function is Color?

> Source/WebCore/rendering/RenderThemeChromiumSkia.h:52
> +            return RenderThemeChromiumSkia::defaultTapHighlightColor;

I'm surprised that "RenderThemeChromiumSkia::" is necessary.

[W. James MacLean]

Comment on attachment 175438 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175438&action=review

>> Source/WebCore/rendering/RenderThemeChromiumSkia.h:50
>> +        virtual Color platformTapHighlightColor() const OVERRIDE
> 
> The indent in this file is really odd.
> 
> Does it matter that defaultTapHighlightColor has type RGBA32 and the return type of this function is Color?

Yes, I noticed the indenting was strange, and I thought about changing it, but the rest of the CL is so small I thought it would obscure the main intent of the CL. :-)

There is an implicit conversion (platform/Color.h) from RGBA32 -> Color, but I'll make it explicit before I land.

>> Source/WebCore/rendering/RenderThemeChromiumSkia.h:52
>> +            return RenderThemeChromiumSkia::defaultTapHighlightColor;
> 
> I'm surprised that "RenderThemeChromiumSkia::" is necessary.

Actually it's not ... I was just following the style in RenderThemeChromiumAndroid, but it doesn't need to be here. I'll remove it before landing.

[W. James MacLean]

Created attachment 175478 [details]
Patch for landing

[Adam Barth]

Yeah, we should fix the indent in a separate patch

[WebKit Review Bot]

Comment on attachment 175478 [details]
Patch for landing

Clearing flags on attachment: 175478

Committed r135418: <http://trac.webkit.org/changeset/135418>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.