Skip to content

Commit

Permalink
Make customizable select <button> display:contents
Browse files Browse the repository at this point in the history
This patch makes the <select> element itself handle focus, interaction,
and box decorations instead of the child <button> element. This removes
a lot of complexity, including delegatesFocus, divergent event handling
and button testing, and :-internal-select-has-child-button.

This was discussed here:
w3c/csswg-drafts#10857

Change-Id: Ia036c6dc08be1b86a50bd96239d08ad19c9a7a6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6001193
Reviewed-by: David Baron <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1383048}
  • Loading branch information
josepharhar authored and chromium-wpt-export-bot committed Nov 14, 2024
1 parent 6176095 commit 7a440cb
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@
<script>
promise_test(async () => {
const select = document.querySelector('select');
const invokerButton = document.getElementById('invoker');
const popoverButton = document.getElementById('popover');

await test_driver.click(invokerButton);
await test_driver.click(select);
assert_true(select.matches(':open'),
'Select should open after clicking the invoker button.');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

<script>
promise_test(async () => {
await test_driver.click(document.querySelector('button'));
await test_driver.click(document.querySelector('select'));
await new Promise(requestAnimationFrame);
}, 'This test passes if it does not crash.');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@
button.focus();
assert_not_equals(document.activeElement, button,
'select button should not be focusable when select is disabled.');

await test_driver.click(button);
assert_false(select.matches(':open'),
'select should not be open after clicking button when disabled.');
}
}, `${id}: <select disabled> should prevent focus and activation with appearance:base-select.`);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,7 @@

promise_test(async t => {
addCloseCleanup(t);
// TODO(http://crbug.com/1350299): When focus for custom buttons is fixed,
// then we shouldn't need to explicitly focus the custom button like this
// anymore.
const customButton = select.querySelector('button');
if (customButton) {
customButton.focus();
} else {
select.focus();
}
select.focus();
assert_false(select.matches(':open'),
'The select should initially be closed.');
await test_driver.send_keys(document.activeElement, Space);
Expand All @@ -85,7 +77,7 @@
'The select should be open after pressing space.');

await test_driver.send_keys(document.activeElement, Escape);
assert_equals(document.activeElement, customButton ? customButton : select,
assert_equals(document.activeElement, select,
'After dismissing the popover, the invoker button should be focused again.');
}, `${id}: When the listbox is closed, spacebar should open the listbox.`);

Expand Down Expand Up @@ -131,15 +123,7 @@
promise_test(async t => {
addCloseCleanup(t);

// TODO(http://crbug.com/1350299): When focus for custom buttons is fixed,
// then we shouldn't need to explicitly use the custom button like this
// anymore.
const customButton = select.querySelector('button');
if (customButton) {
await test_driver.send_keys(customButton, Enter);
} else {
await test_driver.send_keys(select, Enter);
}
await test_driver.send_keys(select, Enter);
await new Promise(requestAnimationFrame);
assert_false(select.matches(':open'),
'Enter should not open the listbox when outside a form.');
Expand All @@ -150,11 +134,7 @@
event.preventDefault();
formWasSubmitted = true;
}, {once: true});
if (customButton) {
await test_driver.send_keys(customButton, Enter);
} else {
await test_driver.send_keys(select, Enter);
}
await test_driver.send_keys(select, Enter);
await new Promise(requestAnimationFrame);
assert_true(formWasSubmitted,
'Enter should submit the form when the listbox is closed.');
Expand Down Expand Up @@ -214,14 +194,8 @@
promise_test(async t => {
addCloseCleanup(t);

const customButton = select.querySelector('button');
if (customButton) {
customButton.focus();
await test_driver.send_keys(customButton, Space);
} else {
select.focus();
await test_driver.send_keys(select, Space);
}
select.focus();
await test_driver.send_keys(select, Space);
assert_true(select.matches(':open'),
'Space should open the listbox.');
assert_equals(document.activeElement, select.querySelector('.one'),
Expand All @@ -234,12 +208,7 @@
// TODO(crbug.com/40263709): Remove this blur when focus on close is improved.
document.activeElement.blur();

if (customButton) {
customButton.focus();
await test_driver.send_keys(customButton, Space);
} else {
await test_driver.send_keys(select, Space);
}
await test_driver.send_keys(select, Space);
assert_true(select.matches(':open'),
'Second space should open the listbox.');
assert_equals(document.activeElement, select.querySelector('.one'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,8 @@

promise_test(async () => {
await test_driver.click(label);
if (button) {
assert_equals(document.activeElement, button,
'select child button should be the active element after clicking the label.');
} else {
assert_equals(document.activeElement, select,
'select should be the active element after clicking the label.');
}
assert_equals(document.activeElement, select,
'select should be the active element after clicking the label.');
assert_false(select.matches(':open'),
'select picker should be closed after clicking the label.');
}, `${id}: Clicking the label should focus the select button without opening the picker.`);
Expand Down

0 comments on commit 7a440cb

Please sign in to comment.