Summary: | REGRESSION(r164329): Input fields are not honoring the maxlength attribute | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||
Component: | Forms | Assignee: | Ryosuke Niwa <rniwa> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, benjamin, commit-queue, darin, enrica, kling | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2014-12-09 08:17:29 PST
Created attachment 242923 [details]
Fixes the bug
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. Created attachment 242939 [details]
Addressed Darin's comments
(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. 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. Created attachment 243062 [details]
Patch for landing
Comment on attachment 243062 [details] Patch for landing Clearing flags on attachment: 243062 Committed r177098: <http://trac.webkit.org/changeset/177098> All reviewed patches have been landed. Closing bug. |