[webkit-reviews] review denied: [Bug 21271] Maxlength limits value : [Attachment 24091] Work in progress

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Oct 11 14:45:01 PDT 2008

Darin Adler <darin at apple.com> has denied Rob Buis <rwlbuis at gmail.com>'s request
for review:
Bug 21271: Maxlength limits value

Attachment 24091: Work in progress

------- Additional Comments from Darin Adler <darin at apple.com>
The patch looks OK.

Note that constrainValue does more than just limit length. It also cuts off
strings at the first control character (characters in the range 0-8 or 9-31).
We'll need some kind of test about that too to ensure that it's OK to remove
that behavior. I'm worried that we will now allow control characters in a field
that weren't allowed before. Maybe these characters could even be used to
submit misleading forms?

I'm not sure that defaultEventHandler code does the right thing when we already
have a string that's too long. Do the other browsers allow you to overtype
characters if the string is too long? It's a little strange that typing over a
letter won't behave the same as deletion of that letter followed by typing a
new letter. We should make test cases that cover edge cases like that.

WIth the reduced use of the constrainValue function, we might want to rename it
and remove the overload that doesn't take a maximum length argument. It's not
really a generic "constrain value" function any more if it's really just
enforcing maximum length after typing. Maybe the entire body of the
isBeforeTextInsertedEvent() if statement should be moved into a function and
that could be merged with constrainValue?

More information about the webkit-reviews mailing list