Improve customizable combobox dismissal prevention [chromium/src : main]
0 views
Skip to first unread message
David Baron (Gerrit)
Jan 9, 2026, 11:21:03 AM (3 days ago) Jan 9
to David Baron, AyeAye, Chromium LUCI CQ, blink-...@chromium.org, blink-rev...@chromium.org
Attention needed from Joey Arhar
David Baron voted and added 3 comments![]( "Open in Gerrit")
Votes added by David Baron
| Code-Review | +1 |
3 comments
Patchset-level comments
File-level comment, Patchset 2 (Latest):
David Baron . unresolved
LGTM with a few comments, although I wonder if it's worth adding any tests for what you're improving here
File third_party/blink/renderer/core/html/forms/html_input_element.cc
Line 2520, Patchset 2 (Latest):
if (auto* datalist = DataList()) {David Baron . unresolved
```suggestion
if (HTMLDataListElement* datalist = DataList()) {
```
File third_party/blink/renderer/core/html/forms/text_field_input_type.cc
Line 719, Patchset 2 (Parent):
// MenuListSelectType::DefaultEventHandler.David Baron . unresolved
Curious if you want to do something similar for that case too?
Related details
Attention is currently required from:
- Joey Arhar
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. |
Joey Arhar (Gerrit)
Jan 9, 2026, 12:59:10 PM (3 days ago) Jan 9
to David Baron, AyeAye, Chromium LUCI CQ, blink-...@chromium.org, blink-rev...@chromium.org
Attention needed from David Baron
Joey Arhar added 3 comments![]( "Open in Gerrit")
Patchset-level comments
David Baron . unresolved
LGTM with a few comments, although I wonder if it's worth adding any tests for what you're improving here
Joey Arhar
As far as I can tell there shouldn't be any behavior changes with this patch
File third_party/blink/renderer/core/html/forms/html_input_element.cc
Line 2520, Patchset 2 (Latest):
if (auto* datalist = DataList()) {David Baron . resolved
```suggestion
if (HTMLDataListElement* datalist = DataList()) {
```
Joey Arhar
Done
File third_party/blink/renderer/core/html/forms/text_field_input_type.cc
Line 719, Patchset 2 (Parent):
// MenuListSelectType::DefaultEventHandler.David Baron . unresolved
Curious if you want to do something similar for that case too?
Joey Arhar
Good point, we could probably remove the call to SetPopoverPointerdownTarget there too, since I already made a similar fix for customizable select. It's already behind a flag though and will get removed as part of the light dismiss rewrite though.
Related details
Attention is currently required from:
- David Baron
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. |
David Baron (Gerrit)
Jan 9, 2026, 3:21:42 PM (3 days ago) Jan 9
to David Baron, AyeAye, Chromium LUCI CQ, blink-...@chromium.org, blink-rev...@chromium.org
Attention needed from Joey Arhar
David Baron voted and added 2 comments![]( "Open in Gerrit")
Votes added by David Baron
| Code-Review | +1 |
Joey ArharLGTM with a few comments, although I wonder if it's worth adding any tests for what you're improving here
As far as I can tell there shouldn't be any behavior changes with this patch
David Baron
Acknowledged
File third_party/blink/renderer/core/html/forms/text_field_input_type.cc
Line 719, Patchset 2 (Parent):
// MenuListSelectType::DefaultEventHandler.David Baron . resolved
Joey ArharCurious if you want to do something similar for that case too?
Good point, we could probably remove the call to SetPopoverPointerdownTarget there too, since I already made a similar fix for customizable select. It's already behind a flag though and will get removed as part of the light dismiss rewrite though.
Attention is currently required from:
- Joey Arhar
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Jan 9, 2026, 6:25:39 PM (3 days ago) Jan 9
to David Baron, AyeAye, Chromium LUCI CQ, blink-...@chromium.org, blink-rev...@chromium.org
Joey Arhar 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. |
Joey Arhar (Gerrit)
Jan 9, 2026, 6:25:45 PM (3 days ago) Jan 9
to David Baron, AyeAye, Chromium LUCI CQ, blink-...@chromium.org, blink-rev...@chromium.org
Attention needed from Joey Arhar
Joey Arhar voted![]( "Open in Gerrit")
Attention is currently required from:
- Joey Arhar
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)
Jan 9, 2026, 6:28:38 PM (3 days ago) Jan 9
to David Baron, AyeAye, blink-...@chromium.org, blink-rev...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Improve customizable combobox dismissal prevention
The existing code which prevents the popover from being shown and then
dismissed in one click was hitting a CHECK due to the light dismiss
rewrite, so this patch migrates the logic to another spot which is also
being used for customizable select.
This patch also adds some logic to try preventing autofill from showing
a picker by signaling that autocomplete should be turned off.
Bug: 453705243
Change-Id: Ica9fbc04c4210206911a9253ef46ffcc8a747918
Reviewed-by: David Baron <dba...@chromium.org>
Reviewed-by: Joey Arhar <jar...@chromium.org>
Commit-Queue: Joey Arhar <jar...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1567253}
Files:
- M third_party/blink/renderer/core/html/forms/html_input_element.cc
- M third_party/blink/renderer/core/html/forms/html_input_element.h
- M third_party/blink/renderer/core/html/forms/text_field_input_type.cc
- M third_party/blink/renderer/core/html/html_element.cc
Change size: M
Delta: 4 files changed, 29 insertions(+), 26 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joey Arhar, +1 by David Baron
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages