[webkit-reviews] review denied: [Bug 25573] misspelling underline cleanup : [Attachment 30029] patch WITH layout test rebaseline

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 6 11:48:31 PDT 2009


mitz at webkit.org has denied 's request for review:
Bug 25573: misspelling underline cleanup
https://bugs.webkit.org/show_bug.cgi?id=25573

Attachment 30029: patch WITH layout test rebaseline
https://bugs.webkit.org/attachment.cgi?id=30029&action=review

------- Additional Comments from mitz at webkit.org
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 43246)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,17 @@
> +2009-05-05  John Grabowski  <set EMAIL_ADDRESS environment variable>

You should put your email address there.

> +	Unify use of CG-common routine for drawLineForMisspellingOrBadGrammar.
> +	Cleanup for WebKit, but required for Chromium happiness.

There are tabs up there. They need to be changed into spaces so that the patch
can be checked in.

> +static const Color& spellingPatternColor() {
> +    static const Color spellingColor(255, 0, 0);
> +    return spellingColor;
> +}
> +
> +static const Color& grammarPatternColor() {
> +    static const Color grammarColor(0, 128, 0);
> +    return grammarColor;
> +}

WebKit coding style is that the brace at the beginning of a function body goes
on its own line.
I think you should use DEFINE_STATIC_LOCAL for the colors.


More information about the webkit-reviews mailing list