Fix blink::StringImpl::FoldCase() [chromium/src : main]
0 views
Skip to first unread message
Jihad Hanna (Gerrit)
May 29, 2026, 3:48:53 AM (3 days ago) May 29
to android-bu...@system.gserviceaccount.com, Kent Tamura, Kentaro Hara, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Kent Tamura
Jihad Hanna voted and added 4 comments![]( "Open in Gerrit")
Votes added by Jihad Hanna
| Commit-Queue | +1 |
4 comments
Patchset-level comments
File-level comment, Patchset 5:
Kent Tamura . resolved
Jihad HannaUsing `FoldCase()` is an implementation detail of `<datalist>`, and might be changed in the future. Would you add a unit test of `HTMLInputElement::FilteredDataListOptions()` too please?
Done
Commit Message
Line 9, Patchset 5:
This CL fixes an issue where typing in a field results in a different outcome than copy-pasting the typed text into the field, when it comesKent Tamura . resolved
Jihad HannaPlease wrap a paragraph in 72 columns.
Done
Line 25, Patchset 5:
This CL attempts to close the gap between 8-bit and 16-bitKent Tamura . unresolved
Jihad HannaDoes this CL affect CSS `text-transform:lowercase` ?
It does not seem so:
- `text-tranform` calls `ComputedStyle::ApplyTextTransform()`
- The lowercase branch calls `CaseMap::ToLower()`
- I couldn't find a usage of `StringImpl::FoldCase()` from there.
File third_party/blink/renderer/platform/wtf/text/string_impl.cc
Line 488, Patchset 5:
if (IsAllAscii(chars)) {Kent Tamura . resolved
Jihad HannaWe have `StringImpl::ContainsOnlyAsciiOrEmpty()`, and it can be much faster than this.
Done
Related details
Attention is currently required from:
- Kent Tamura
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kent Tamura (Gerrit)
May 31, 2026, 7:08:08 PM (20 hours ago) May 31
to Jihad Hanna, Kent Tamura, android-bu...@system.gserviceaccount.com, Kentaro Hara, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Jihad Hanna
Kent Tamura voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kent Tamura
| Code-Review | +1 |
1 comment
Commit Message
Line 25, Patchset 5:
This CL attempts to close the gap between 8-bit and 16-bitKent Tamura . resolved
Jihad HannaDoes this CL affect CSS `text-transform:lowercase` ?
It does not seem so:
- `text-tranform` calls `ComputedStyle::ApplyTextTransform()`
- The lowercase branch calls `CaseMap::ToLower()`
- I couldn't find a usage of `StringImpl::FoldCase()` from there.
Attention is currently required from:
- Jihad Hanna
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jihad Hanna (Gerrit)
4:36 AM (11 hours ago) 4:36 AM
to Kent Tamura, android-bu...@system.gserviceaccount.com, Kentaro Hara, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Jihad Hanna voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
6:29 AM (9 hours ago) 6:29 AM
to Jihad Hanna, Kent Tamura, android-bu...@system.gserviceaccount.com, Kentaro Hara, chromium...@chromium.org, blink-rev...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Fix blink::StringImpl::FoldCase()
This CL fixes an issue where typing in a field results in a different
outcome than copy-pasting the typed text into the field, when it comes
to datalist option filtering. (Details in the bug report).
There's an case-inconsistency in `blink::StringImpl::FoldCase()`:
1) 16-bit Strings
When a string is represented as 16-bit (which can be the case during
copy-paste), `FoldCase()` used ICU's `u_strFoldCase` (Unicode Full
Case Folding). Under full case folding, `'ß'` (U+00DF) expands to
`"ss"`.
2) 8-bit Strings
When a string is represented as 8-bit (which can be the case for
inline text in the document or typed text), `FoldCase()` simply
performed lowercasing. Under lowercasing, `'ß'` remains `'ß'`.
This CL attempts to close the gap between 8-bit and 16-bit
implementations of the function.
Fixed: 493179860
Change-Id: I6a0b4be9b627af233ecc6c6ff4824ab6707ba2ed
Commit-Queue: Jihad Hanna <jihad...@google.com>
Reviewed-by: Kent Tamura <tk...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1639271}
Files:
- M third_party/blink/renderer/core/html/forms/html_input_element_test.cc
- M third_party/blink/renderer/platform/wtf/text/string_impl.cc
- M third_party/blink/renderer/platform/wtf/text/string_impl_test.cc
Change size: M
Delta: 3 files changed, 102 insertions(+), 46 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Kent Tamura
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages