Skip to content

Commit

Permalink
Revert "[Reland] Handle appearance value changes during <select> show…
Browse files Browse the repository at this point in the history
…Picker."

This reverts commit 5fa283f55fea44480ce73308cb12de0274867c1e.

Reason for revert:
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/5657193371140096

Sample build with failed test: https://ci.chromium.org/b/8726470617543936881
Affected test(s):
ninja://:blink_wpt_tests/external/wpt/html/semantics/forms/the-select-element/customizable-select/switch-picker-appearance.tentative.html

Original change's description:
> [Reland] Handle appearance value changes during <select> showPicker.
>
> When the appearance value changes as the result of the picker
> opening (e.g. via `::picker(select):open {appearance:auto}` or
> similar), we close the picker to avoid any oscillation or
> nonsensical behavior. This CL implements this change.
>
> The original CL [1] was reverted due to a strange bot failure [2]
> where the new set of promise_tests somehow fail because
> blink_wpt_tests is expecting an expectation file with a single blank
> line. I added a "Failure" expectation to hopefully avoid that, in
> Patchset 3.
>
> [1] https://chromium-review.googlesource.com/c/chromium/src/+/6094831
> [2] https://ci.chromium.org/ui/p/chromium/builders/ci/mac11-arm64-rel-tests/50289/overview
>
> Fixed: 376097114,370536090,367426156
> Bug: 384394713
> Change-Id: I2ab4e43db3de4b7125b564c366fe86bdf9f85cc4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6107689
> Auto-Submit: Mason Freed <[email protected]>
> Reviewed-by: Joey Arhar <[email protected]>
> Reviewed-by: Mason Freed <[email protected]>
> Commit-Queue: Mason Freed <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1402440}

Bug: 384394713
Change-Id: Ic5392451a1ffe93b40c3a9748164bc03abbfc884
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6148450
Reviewed-by: Bohdan Tyshchenko <[email protected]>
Auto-Submit: Danila Kuzmin <[email protected]>
Owners-Override: Danila Kuzmin <[email protected]>
Commit-Queue: Bohdan Tyshchenko <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1402871}
  • Loading branch information
Danila Kuzmin authored and chromium-wpt-export-bot committed Jan 7, 2025
1 parent 7f9ee7f commit b6ca3af
Showing 1 changed file with 14 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
</select>

<style>
#test1::picker(select) {
select#test1::picker(select) {
background-color: red;
}
#test1::picker(select):popover-open {
select#test1::picker(select):popover-open {
background-color: green;
}
</style>
Expand All @@ -33,7 +33,7 @@
t.add_cleanup(() => style.remove());
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
assert_equals(getComputedStyle(select1,'::picker(select)').backgroundColor,red);
style.innerHTML = '#test1::picker(select) {appearance: base-select}';
style.innerHTML = '::picker(select) {appearance: base-select}';
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'base-select');
assert_equals(getComputedStyle(select1,'::picker(select)').backgroundColor,red,'still closed, so popover-open doesn\'t match');

Expand All @@ -57,7 +57,7 @@
document.head.append(style);
t.add_cleanup(() => style.remove());
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
style.innerHTML = '#test1::picker(select) {appearance: auto}';
style.innerHTML = '::picker(select) {appearance: auto}';
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'auto');
await test_driver.bless('showPicker');
select1.showPicker();
Expand All @@ -72,7 +72,7 @@
document.head.append(style);
t.add_cleanup(() => style.remove());
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
style.innerHTML = '#test1::picker(select) {appearance: none}';
style.innerHTML = '::picker(select) {appearance: none}';
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
await test_driver.bless('showPicker');
select1.showPicker();
Expand All @@ -88,8 +88,8 @@
t.add_cleanup(() => style.remove());
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
style.innerHTML = `
#test1::picker(select) {appearance: base-select}
#test1::picker(select):popover-open {appearance: auto}
::picker(select) {appearance: base-select}
::picker(select):popover-open {appearance: auto}
`;
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'base-select');
await test_driver.bless('showPicker');
Expand All @@ -109,17 +109,18 @@
document.head.append(style);
t.add_cleanup(() => style.remove());
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
style.innerHTML = '#test1::picker(select) {appearance: base-select}';
style.innerHTML = '::picker(select) {appearance: none}';
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'base-select');
await test_driver.bless('showPicker');
select1.showPicker();
assert_true(select1.matches(':open'));
style.remove();
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
assert_false(select1.matches(':open'),'changing appearance while the picker is open should close it');
assert_equals(getComputedStyle(select1,'::picker(select)').appearance,'none');
}, 'Switching appearance in JS after picker is open should close the picker');
</script>


<button id=reset>Reset</button>
<select id=test2>
<button><selectedcontent></selectedcontent></button>
Expand All @@ -129,13 +130,13 @@
</select>

<style>
#test2, #test2::picker(select) {
select#test2, ::picker(select) {
appearance: base-select;
}
#test2.controlswitch:open {
select#test2.controlswitch:open {
appearance: auto;
}
#test2.pickerswitch:open::picker(select) {
select#test2.pickerswitch:open::picker(select) {
appearance: auto;
}
</style>
Expand Down Expand Up @@ -199,7 +200,7 @@
assert_equals(getComputedStyle(select2).appearance,'base-select');
t.add_cleanup(() => select2.removeAttribute('style'));
select2.setAttribute('style','appearance:auto');
assert_equals(getComputedStyle(select2).appearance,'auto','appearance should still be auto from inline style');
assert_false(select2.matches(':open'),'Adding inline style should close the picker');
assert_equals(getComputedStyle(select2).appearance,'auto','appearance should still be auto from inline style');
},'The select picker is closed if the <select> inline appearance value is changed while the picker is open');
</script>

0 comments on commit b6ca3af

Please sign in to comment.