WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 237525
[selection] Set correct selection range for TEXTAREA when updating default value
https://bugs.webkit.org/show_bug.cgi?id=237525
Summary
[selection] Set correct selection range for TEXTAREA when updating default value
zsun
Reported
2022-03-07 05:38:39 PST
https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#textFieldSelection:concept-textarea/input-relevant-value
Attachments
Patch
(39.34 KB, patch)
2022-03-07 06:21 PST
,
zsun
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(39.33 KB, patch)
2022-03-07 06:52 PST
,
zsun
no flags
Details
Formatted Diff
Diff
Patch
(7.74 KB, patch)
2022-04-29 05:32 PDT
,
zsun
no flags
Details
Formatted Diff
Diff
Patch
(7.67 KB, patch)
2022-05-02 00:59 PDT
,
zsun
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
zsun
Comment 1
2022-03-07 06:21:05 PST
Created
attachment 453971
[details]
Patch
zsun
Comment 2
2022-03-07 06:52:18 PST
Created
attachment 453975
[details]
Patch
Radar WebKit Bug Importer
Comment 3
2022-03-14 06:39:16 PDT
<
rdar://problem/90243598
>
zsun
Comment 4
2022-04-29 05:32:26 PDT
Created
attachment 458582
[details]
Patch
Tim Nguyen (:ntim)
Comment 5
2022-04-29 07:25:11 PDT
Comment on
attachment 458582
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=458582&action=review
> Source/WebCore/html/HTMLTextAreaElement.cpp:408 > + unsigned selectionStartValue = isClamp ? computeSelectionStart() : 0; > + unsigned selectionEndValue = isClamp ? computeSelectionEnd() : 0;
auto
> Source/WebCore/html/HTMLTextFormControlElement.h:86 > + WEBCORE_EXPORT unsigned computeSelectionStart() const; > + WEBCORE_EXPORT unsigned computeSelectionEnd() const;
This doesn't need WEBCORE_EXPORT, you're not using outside of the webcore directory.
Chris Dumez
Comment 6
2022-04-29 07:28:48 PDT
Comment on
attachment 458582
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=458582&action=review
r=me with comments. (also agree with all of Tim's)
> Source/WebCore/html/HTMLTextAreaElement.cpp:139 > + setNonDirtyValue(defaultValue(), TextControlSetValueSelection::Clamp);
Too many spaces after the comma.
> Source/WebCore/html/HTMLTextAreaElement.cpp:406 > + bool isClamp = selection == TextControlSetValueSelection::Clamp;
I think this indicates whether we want to clamp or not. If so, I think "shouldClamp" would be a better variable name.
>> Source/WebCore/html/HTMLTextFormControlElement.h:86 >> + WEBCORE_EXPORT unsigned computeSelectionEnd() const; > > This doesn't need WEBCORE_EXPORT, you're not using outside of the webcore directory.
True but also, why did you make them public. Seems like they could be protected?
Tim Nguyen (:ntim)
Comment 7
2022-04-29 07:30:41 PDT
Comment on
attachment 458582
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=458582&action=review
> Source/WebCore/html/HTMLTextAreaElement.cpp:426 > if (document().focusedElement() == this) { > unsigned endOfString = m_value.length(); > setSelectionRange(endOfString, endOfString); > - } else if (selection == TextControlSetValueSelection::SetSelectionToEnd) { > + } else if (selection == TextControlSetValueSelection::SetSelectionToEnd) { > // We don't change text selection here but need to update caret to > // the end of the text value except for initialize. > unsigned endOfString = m_value.length(); > cacheSelection(endOfString, endOfString, SelectionHasNoDirection); > + } else if (isClamp) { > + unsigned endOfString = m_value.length();
Since all 3 branches use `endOfString`, can we factor it out of the if/elses? Also `unsigned` -> `auto`
zsun
Comment 8
2022-05-02 00:59:13 PDT
Created
attachment 458666
[details]
Patch
Tim Nguyen (:ntim)
Comment 9
2022-05-02 11:34:02 PDT
Comment on
attachment 458666
[details]
Patch I assume this is ready to cq+
EWS
Comment 10
2022-05-02 12:13:17 PDT
Committed
r293673
(
250175@main
): <
https://commits.webkit.org/250175@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 458666
[details]
.
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