[Webkit-unassigned] [Bug 29292] [HTML5][Forms] Support for <textarea maxlength=N>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=29292





--- Comment #4 from TAMURA, Kent <tkent at chromium.org>  2009-09-17 19:32:53 PDT ---
(In reply to comment #2)

Thank you for the helpful comments.

> (From update of attachment 39679 [details])
> > +    if (!hasAttribute(maxlengthAttr))
> > +        return;
> > +    bool ok;
> > +    unsigned maxLength = getAttribute(maxlengthAttr).string().toUInt(&ok);
> > +    if (!ok)
> > +        return;
> 
> It's a shame to do two hash table lookups here for the maxlength attribute. And
> there's no reason for that check. toUInt will give false for "ok" when
> maxlengthAttr is null, so I believe the hasAttribute check is unnecessary extra
> work.

Removed hasAttribute().


> > +    int currentLength = InputElement::numGraphemeClusters(toRenderTextControl(renderer())->text().impl());
> > +    int selectionLength = InputElement::numGraphemeClusters(plainText(document()->frame()->selection()->selection().toNormalizedRange().get()).impl());
> 
> I think these functions should just be public functions, perhaps in
> PlatformString.h, rather than members of the InputElement class. It also seems
> inconvenient to have to pass them StringImpl* rather than just a String. As we
> make these functions more public it makes sense to me to make them more normal
> functions and less special-purpose.

Moved the grapheme functions to String class.


> > +    if (newLength < static_cast<int>(proposedValue.length())) {
> > +        String string = proposedValue;
> > +        return string.left(newLength);
> > +    }
> > +    return proposedValue;
> 
> This can just be written:
> 
>     return proposedValue.left(newLength);

Done.


> > +    bool ok;
> > +    unsigned maxLength = getAttribute(maxlengthAttr).string().toUInt(&ok);
> > +    return ok ? maxLength : 0;
> 
> toUInt already guarantees it will return 0 if the string can't be parsed, so
> you can just write:
> 
>     return getAttribute(maxlengthAttr).string().toUInt();

Done.


> > +    static String sanitizeUserInputValue(const String&, int);
> 
> It's not good to leave out the argument name here for the int. It's clear what
> this int would be. So it needs a name.

Added the argument name.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.