[webkit-reviews] review granted: [Bug 25573] misspelling underline cleanup : [Attachment 30061] feedback applied

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 7 15:10:50 PDT 2009


Simon Fraser (smfr) <simon.fraser at apple.com> has granted John Grabowski
<jrg at chromium.org>'s request for review:
Bug 25573: misspelling underline cleanup
https://bugs.webkit.org/show_bug.cgi?id=25573

Attachment 30061: feedback applied
https://bugs.webkit.org/attachment.cgi?id=30061&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> +void GraphicsContext::drawLineForMisspellingOrBadGrammar(const IntPoint&
point, int width, bool grammar)
> +{
> +    if (paintingDisabled())
> +	   return;
> +
> +    // These are the same for misspelling or bad grammar
> +    const int patternHeight = 3; // 3 rows
> +    ASSERT(cMisspellingLineThickness == patternHeight);
> +    const int patternWidth = 4; // 4 pixels
> +    ASSERT(patternWidth == cMisspellingLinePatternWidth);

I know this was copied from the Windows code, but why:
i) not just use the constants, rather than hardcoded values with assertions
that they equal the constants
ii) why do the two assertions use different styles?

r=me if you clean that up. I don't see an issue with just using the constants
directly.


More information about the webkit-reviews mailing list