WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
237210
[Selection] Selection Range should be clamped by the current value length
https://bugs.webkit.org/show_bug.cgi?id=237210
Summary
[Selection] Selection Range should be clamped by the current value length
zsun
Reported
2022-02-25 06:55:33 PST
https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#set-the-selection-range
Attachments
Patch
(4.25 KB, patch)
2022-02-25 07:12 PST
,
zsun
no flags
Details
Formatted Diff
Diff
Patch
(4.27 KB, patch)
2022-02-28 01:46 PST
,
zsun
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
zsun
Comment 1
2022-02-25 07:12:10 PST
Created
attachment 453212
[details]
Patch
Darin Adler
Comment 2
2022-02-27 12:17:58 PST
Comment on
attachment 453212
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=453212&action=review
> Source/WebCore/html/HTMLTextFormControlElement.cpp:283 > + const int innerTextValueLength = innerTextValue().length(); > + end = std::min(std::max(end, 0), std::max(innerTextValueLength, 0)); > start = std::min(std::max(start, 0), end);
The "const" on the first line is not WebKit coding style. Most local variables are constant, not reassigned, and we typically don’t put "const" in front of all of them, although we could. I suggest writing this instead: constexpr unsigned maxInt = std::numeric_limits<int>::max(); int innerTextValueLength = std::min(innerTextValue().length(), maxInt); end = std::clamp(end, 0, innerTextValueLength); start = std::clamp(start, 0, end);
Darin Adler
Comment 3
2022-02-27 12:19:57 PST
Comment on
attachment 453212
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=453212&action=review
>> Source/WebCore/html/HTMLTextFormControlElement.cpp:283 >> start = std::min(std::max(start, 0), end); > > The "const" on the first line is not WebKit coding style. Most local variables are constant, not reassigned, and we typically don’t put "const" in front of all of them, although we could. I suggest writing this instead: > > constexpr unsigned maxInt = std::numeric_limits<int>::max(); > int innerTextValueLength = std::min(innerTextValue().length(), maxInt); > end = std::clamp(end, 0, innerTextValueLength); > start = std::clamp(start, 0, end);
Here’s a better idea: end = std::clamp(end, 0, clampTo<int>(innerTextValue().length())); start = std::clamp(start, 0, end);
zsun
Comment 4
2022-02-28 01:46:21 PST
Created
attachment 453376
[details]
Patch
EWS
Comment 5
2022-03-01 01:02:20 PST
Committed
r290635
(
247908@main
): <
https://commits.webkit.org/247908@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 453376
[details]
.
Radar WebKit Bug Importer
Comment 6
2022-03-01 01:03:17 PST
<
rdar://problem/89607167
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug