[webkit-reviews] review granted: [Bug 29292] [HTML5][Forms] Support for <textarea maxlength=N> : [Attachment 39769] Proposed patch (rev.3)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 29292: [HTML5][Forms] Support for <textarea maxlength=N>
https://bugs.webkit.org/show_bug.cgi?id=29292

Attachment 39769: Proposed patch (rev.3)
https://bugs.webkit.org/attachment.cgi?id=39769&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    unsigned maxLength = static_cast<unsigned>(data.maxLength());  //
maxLength() never be negative.

Coding style says one space before the "//".

The grammar here is wrong. It could be "can never be negative" or "never is
negative" instead.

> +    unsigned appendableLength =  maxLength > baseLength ? maxLength -
baseLength : 0;

Extra space here after the equal sign.

Another way to write this is:

    max(maxLength, baseLength) - baseLength

Your way is probably clearer, though.

> +    TextBreakIterator* it = characterBreakIterator(characters(), length());
> +    if (!it)
> +	   return length();

The value length() is not right here. It should be min(length(),
numGraphemeClusters) instead.

I'm going to say review+ because realistically characterBreakIterator will not
return 0 so that comment is not important.