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

[bugzilla-daemon at webkit.org]

TAMURA, Kent <tkent at chromium.org> has asked  for review:
Bug 29292: [HTML5][Forms] Support for <textarea maxlength=N>
https://bugs.webkit.org/show_bug.cgi?id=29292

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

------- Additional Comments from TAMURA, Kent <tkent at chromium.org>
> > +	 unsigned maxLength = static_cast<unsigned>(data.maxLength());	//
maxLength() never be negative.
> Coding style says one space before the "//".

Fixed.

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

Fixed.

> > +	 unsigned appendableLength =  maxLength > baseLength ? maxLength -
baseLength : 0;
> Extra space here after the equal sign.

Fixed.

> > +	 TextBreakIterator* it = characterBreakIterator(characters(),
length());
> The value length() is not right here. It should be min(length(),
> numGraphemeClusters) instead.

I don't think so.
Suppose that the string is "\ud800\udc00", of which length() is 2 and has 1
grapheme.  numCharactersInGraphmeClusters(1) with this string should return 2. 
If we specified min(length(), nmGraphemeClusters) to characerBreakIterator(),
the result would be 1.