Bug 139447 – REGRESSION(r164329): Input fields are not honoring the maxlength attribute

• Component: Forms
• Status: RESOLVED FIXED

[Ryosuke Niwa]

maxlength attribute is not honored when text is inserted in the middle of a text field.

[Ryosuke Niwa]

Created attachment 242923 [details]
Fixes the bug

[Darin Adler]

Comment on attachment 242923 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=242923&action=review

> Source/WebCore/html/TextFieldInputType.cpp:389
> +        int selectedOffsets = selectionStart - element().selectionEnd();

This looks backwards. Computing start - end would give a negative number; not sure why this works. I also think that “offsets” is a confusing name for this. Maybe selectionCodeUnitCount?

> Source/WebCore/html/TextFieldInputType.cpp:390
> +        selectionLength = selectedOffsets ? numGraphemeClusters(innerText.substring(selectionStart, selectedOffsets)) : 0;

Would be nice, some day, to have this use StringView::substring instead of String::substring.

[Ryosuke Niwa]

Created attachment 242939 [details]
Addressed Darin's comments

[Ryosuke Niwa]

(In reply to comment #2)
> Comment on attachment 242923 [details]
> Fixes the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242923&action=review
> 
> > Source/WebCore/html/TextFieldInputType.cpp:389
> > +        int selectedOffsets = selectionStart - element().selectionEnd();
> 
> This looks backwards. Computing start - end would give a negative number;
> not sure why this works. I also think that “offsets” is a confusing name for
> this. Maybe selectionCodeUnitCount?

Oh oops, that's because end == start in my test case. Let me add another test case there...

> > Source/WebCore/html/TextFieldInputType.cpp:390
> > +        selectionLength = selectedOffsets ? numGraphemeClusters(innerText.substring(selectionStart, selectedOffsets)) : 0;
> 
> Would be nice, some day, to have this use StringView::substring instead of
> String::substring.

Indeed.

[Benjamin Poulain]

Comment on attachment 242939 [details]
Addressed Darin's comments

View in context: https://bugs.webkit.org/attachment.cgi?id=242939&action=review

I like both tests.

> Source/WebCore/html/TextFieldInputType.cpp:389
> +        int selectionCodeUnitCount = element().selectionEnd() - selectionStart;

selectionCodeUnitCount is a weird name.

[Ryosuke Niwa]

Created attachment 243062 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 243062 [details]
Patch for landing

Clearing flags on attachment: 243062

Committed r177098: <http://trac.webkit.org/changeset/177098>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.