Fix <select> native popup with <datalist> <option>s [chromium/src : main]
David Baron (Gerrit)
David Baron added 1 comment![]( "Open in Gerrit")
select:open > datalist {Is there a reason this needs to be `select:open` rather than just `select`?
(The fixes to make `:open` work right look good, though!)
Related details
- Joey Arhar
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominique Fauteux-Chapleau (Gerrit)
Dominique Fauteux-Chapleau removed dpr...@google.com from this change![]( "Open in Gerrit")
Related details
- Joey Arhar
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 1 comment![]( "Open in Gerrit")
select:open > datalist {Is there a reason this needs to be `select:open` rather than just `select`?
(The fixes to make `:open` work right look good, though!)
If I removed the `:open`, then the datalist popover would always be showing because this would override the rule which sets display:none on popovers, I think.
Related details
- David Baron
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Baron (Gerrit)
David Baron added 3 comments![]( "Open in Gerrit")
OK, this LGTM, except I think you need to find a way to document your dependency on the `CSSPseudoOpenClosed` feature, since that's still `"experimental"` and nothing guarantees (I think) that it will ship first. Maybe comments in `runtime_enabled_features.json5` are what you want? (I don't think you want an actual feature dependency, because I don't think you want to ship it automatically if you ship the new select.)
select_->GetDocument().UpdateStyleAndLayoutForNode(This is a little unfortunate, but I guess it's OK.
select:open > datalist {Joey ArharIs there a reason this needs to be `select:open` rather than just `select`?
(The fixes to make `:open` work right look good, though!)
If I removed the `:open`, then the datalist popover would always be showing because this would override the rule which sets display:none on popovers, I think.
- Joey Arhar
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 1 comment![]( "Open in Gerrit")
OK, this LGTM, except I think you need to find a way to document your dependency on the `CSSPseudoOpenClosed` feature, since that's still `"experimental"` and nothing guarantees (I think) that it will ship first. Maybe comments in `runtime_enabled_features.json5` are what you want? (I don't think you want an actual feature dependency, because I don't think you want to ship it automatically if you ship the new select.)
Good idea, I added a depends_on in runtime enabled features and added a comment explaining why we need it
Related details
- David Baron
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Baron (Gerrit)
David Baron voted Code-Review+1![]( "Open in Gerrit")
- Joey Arhar
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Fix <select> native popup with <datalist> <option>s
In http://crrev.com/1285415 I made the following markup successfully
render options in the native popup in appearance:auto mode:
<select>
<datalist>
<option>option</option>
However, it didn't completely work, at least on MacOS (not sure how
other platforms worked), because the <option>s won't have a computed
style because the <datalist> is display:none even when the native popup
is open. display:none options are not supposed to be included in the
native popup as a way to allow developers to exclude options from being
included in the native popup.
This patch fixes this by adding a select:open rule to make the
<datalist> display:block so that the <option>s can have a computed
style.
- M third_party/blink/renderer/core/html/forms/external_popup_menu_test.cc
- M third_party/blink/renderer/core/html/forms/select_type.cc
- M third_party/blink/renderer/core/html/resources/stylable_select.css
- M third_party/blink/renderer/core/testing/data/core_test_bundle_data.filelist
- A third_party/blink/renderer/core/testing/data/popup/select_with_datalist.html
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
Code-Review: +1 by David Baron
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
luci-bisection@appspot.gserviceaccount.com (Gerrit)
Message from luci-bi...@appspot.gserviceaccount.com![]( "Open in Gerrit")
LUCI Bisection has identified this change as the cause of a test failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/5750847549997056
Sample build with failed test: https://ci.chromium.org/b/8749818271081443553
Affected test(s):
[ninja://:content_shell_wpt/external/wpt/html/semantics/forms/the-select-element/stylable-select/nested-options.tenative.html](https://ci.chromium.org/ui/test/chromium/ninja:%2F%2F:content_shell_wpt%2Fexternal%2Fwpt%2Fhtml%2Fsemantics%2Fforms%2Fthe-select-element%2Fstylable-select%2Fnested-options.tenative.html?q=VHash%3A4419e4895506aff0)
A revert for this change was not created because the builder of the failed test(s) is not being watched by gardeners.
If this is a false positive, please report it at http://b.corp.google.com/createIssue?component=1199205&description=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Ftest-analysis%2Fb%2F5750847549997056&format=PLAIN&priority=P3&title=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F5462883&type=BUG
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |