RESOLVED FIXED Bug 139447
REGRESSION(r164329): Input fields are not honoring the maxlength attribute
https://bugs.webkit.org/show_bug.cgi?id=139447
Summary REGRESSION(r164329): Input fields are not honoring the maxlength attribute
Ryosuke Niwa
Reported 2014-12-09 08:17:29 PST
maxlength attribute is not honored when text is inserted in the middle of a text field.
Attachments
Fixes the bug (4.48 KB, patch)
2014-12-09 08:21 PST, Ryosuke Niwa
no flags
Addressed Darin's comments (6.44 KB, patch)
2014-12-09 10:42 PST, Ryosuke Niwa
no flags
Patch for landing (8.58 KB, patch)
2014-12-10 13:20 PST, Ryosuke Niwa
no flags
Ryosuke Niwa
Comment 1 2014-12-09 08:21:39 PST
Created attachment 242923 [details] Fixes the bug
Darin Adler
Comment 2 2014-12-09 09:01:21 PST
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
Comment 3 2014-12-09 10:42:10 PST
Created attachment 242939 [details] Addressed Darin's comments
Ryosuke Niwa
Comment 4 2014-12-09 10:42:49 PST
(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 5 2014-12-10 12:41:39 PST
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
Comment 6 2014-12-10 13:20:48 PST
Created attachment 243062 [details] Patch for landing
WebKit Commit Bot
Comment 7 2014-12-10 14:09:48 PST
Comment on attachment 243062 [details] Patch for landing Clearing flags on attachment: 243062 Committed r177098: <http://trac.webkit.org/changeset/177098>
WebKit Commit Bot
Comment 8 2014-12-10 14:09:56 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.