[webkit-reviews] review granted: [Bug 102403] [chromium] Copy linux theme related files to default : [Attachment 174477] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 15 10:43:42 PST 2012


Tony Chang <tony at chromium.org> has granted Scott Violet <sky at chromium.org>'s
request for review:
Bug 102403: [chromium] Copy linux theme related files to default
https://bugs.webkit.org/show_bug.cgi?id=102403

Attachment 174477: Patch
https://bugs.webkit.org/attachment.cgi?id=174477&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=174477&action=review


> Source/Platform/ChangeLog:6
> +	   Reviewed by Tony Change.

You should leave this as "NOBODY (OOPS!)" until you get an r+.	webkit-patch or
the cq will fill in the reviewer for you.

> Source/WebCore/rendering/RenderThemeChromiumDefault.h:92
> +    static void setSelectionColors(unsigned activeBackgroundColor,
> +				      unsigned activeForegroundColor,
> +				      unsigned inactiveBackgroundColor,
> +				      unsigned inactiveForegroundColor);

You can either unwrap this into a single line or just indent 4 spaces from the
previous line.	I would probably just leave it on a single line.

> Source/WebKit/chromium/src/WebViewImpl.cpp:3570
> +    RenderThemeChromiumDefault::setSelectionColors(activeBackgroundColor,
> +						      activeForegroundColor,
> +						      inactiveBackgroundColor,
> +						      inactiveForegroundColor);


Same as above, either unwrap (seems easier) or indent 4 spaces from the
previous line.


More information about the webkit-reviews mailing list