Skip to content

Commit

Permalink
Bug 1858227 [wpt PR 42068] - selectlist: Implement new keyboard behav…
Browse files Browse the repository at this point in the history
…ior, a=testonly

Automatic update from web-platform-tests
selectlist: Implement new keyboard behavior

This patch implements several keyboard behaviors:
- Enter while the listbox is closed should not open the listbox and
  should instead submit the form.
- Enter while the listbox is open should select/commit the currently
  focused option and close the listbox.
- Space should open the listbox.
- Arrow keys while the listbox is open should not commit the newly
  focused value.
- Arrow keys while the listbox is closed should open the listbox.

These were resolved on in OpenUI here:
- openui/open-ui#433 (comment)
- openui/open-ui#386 (comment)
- openui/open-ui#742

This patch also implements the resolution here to stop changing the
visible value of the selected option while the user is switching the
focused option using the arrow keys:

Fixed: 1422275
Change-Id: If5e7328ad739f9c7339dcd17561c57875d4255e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4876791
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1207785}

--

wpt-commits: 020d2129c354423b490e1447f13463829ab92bc0
wpt-pr: 42068
  • Loading branch information
josepharhar authored and moz-wptsync-bot committed Oct 26, 2023
1 parent 34c0464 commit b0dc927
Show file tree
Hide file tree
Showing 3 changed files with 238 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,68 +130,55 @@

promise_test(async () => {
const selectList = document.getElementById("selectList2");
let input_event_count = 0;
let change_event_count = 0;
const events = [];

selectList.addEventListener("input", (e) => {
assert_true(e.composed, "input event should be composed");
assert_equals(input_event_count, 0, "input event should not fire twice");
assert_equals(change_event_count, 0, "input event should not fire before change");
input_event_count++;
assert_true(e.composed, "input event should be composed.");
events.push('input');
});
selectList.addEventListener("change", (e) => {
assert_false(e.composed, "change event should not be composed");
assert_equals(input_event_count, 1, "change event should fire after input");
assert_equals(change_event_count, 0, "change event should not fire twice");
change_event_count++;
assert_false(e.composed, "change event should not be composed.");
events.push('change');
});

await clickOn(selectList);
assert_true(selectList.open);
await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_false(selectList.open);
assert_equals(selectList.value, "one");
assert_equals(input_event_count, 0, "input event shouldn't fire if value wasn't changed");
assert_equals(change_event_count, 0, "change event shouldn't fire if value wasn't changed");
assert_array_equals(events, [], "input and change shouldn't fire if value wansn't changed.");

await clickOn(selectList);
assert_true(selectList.open);
await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "two", "value should change when user switches options with arrow key");
assert_equals(input_event_count, 1, "input event should fire when user switches options with arrow key");
assert_equals(change_event_count, 0, "change event shouldn't fire until popover is closed");
assert_equals(selectList.value, "one", "value shouldn't change when user switches options with arrow key.");
assert_array_equals(events, ['input'], "input event should fire when user switches options with arrow key.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_equals(selectList.value, "two");
assert_equals(input_event_count, 1, "input event should have fired");
assert_equals(change_event_count, 1, "change event should have fired");
}, "<selectlist> should fire input and change events when new option is selected");
assert_array_equals(events, ['input', 'input', 'change'], "input and change should fire after pressing enter.");
}, "<selectlist> should fire input and change events when new option is selected.");

promise_test(async () => {
const selectList = document.getElementById("selectList3");
let input_event_count = 0;
let change_event_count = 0;
const events = [];

selectList.addEventListener("input", (e) => {
assert_true(e.composed, "input event should be composed");
assert_equals(input_event_count, 0, "input event should not fire twice");
assert_equals(change_event_count, 0, "input event should not fire before change");
input_event_count++;
assert_true(e.composed, "input event should be composed.");
events.push('input');
});
selectList.addEventListener("change", (e) => {
assert_false(e.composed, "change event should not be composed");
assert_equals(input_event_count, 1, "change event should fire after input");
assert_equals(change_event_count, 0, "change event should not fire twice");
change_event_count++;
assert_false(e.composed, "change event should not be composed.");
events.push('change');
});

await clickOn(selectList);
assert_true(selectList.open);
await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_array_equals(events, ['input'], "input event should have fired after ArrowDown.");
await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_equals(input_event_count, 1, "input event should have fired");
assert_equals(change_event_count, 1, "change event should have fired");
}, "<selectlist> should fire input and change events even when new selected option has the same value as the old");
assert_array_equals(events, ['input', 'input', 'change'], "input and change should fire after pressing Enter.");
}, "<selectlist> should fire input and change events even when new selected option has the same value as the old.");

promise_test(async () => {
const selectList = document.getElementById("selectList4");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
<!DOCTYPE html>
<link rel=author href="mailto:jarhar@chromium.org">
<link rel=help href="https://bugs.chromium.org/p/chromium/issues/detail?id=1422275">
<link rel=help href="https://github.com/openui/open-ui/issues/433#issuecomment-1452461404">
<link rel=help href="https://github.com/openui/open-ui/issues/386#issuecomment-1452469497">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>

<form></form>

<div id=notform>
<selectlist id=defaultbutton>
<option class=one>one</option>
<option class=two>two</option>
<option class=three>three</option>
</selectlist>

<selectlist id=custombutton>
<button type=selectlist>custom button</button>
<option class=one>one</option>
<option class=two>two</option>
<option class=three>three</option>
</selectlist>
</div>

<script>
const Enter = '\uE007';
const Escape = '\uE00C';
const ArrowLeft = '\uE012';
const ArrowUp = '\uE013';
const ArrowRight = '\uE014';
const ArrowDown = '\uE015';
const Space = ' ';
const form = document.querySelector('form');
const notform = document.getElementById('notform');

for (const id of ['defaultbutton', 'custombutton']) {
const selectlist = document.getElementById(id);

async function closeListbox() {
await test_driver.click(selectlist);
}

function addCloseCleanup(t) {
t.add_cleanup(async () => {
if (selectlist.matches(':open')) {
await closeListbox();
}
if (selectlist.matches(':open')) {
throw new Error('selectlist failed to close!');
}
selectlist.value = 'one';
});
}

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 = selectlist.querySelector('button');
if (customButton) {
customButton.focus();
} else {
selectlist.focus();
}
assert_false(selectlist.matches(':open'),
'The selectlist should initially be closed.');
await test_driver.send_keys(selectlist, Space);
assert_true(selectlist.matches(':open'),
'The selectlist should be open after pressing space.');
}, `${id}: When the listbox is closed, spacebar should open the listbox.`);

promise_test(async t => {
addCloseCleanup(t);
selectlist.value = 'two';
selectlist.focus();
assert_false(selectlist.matches(':open'),
'The selectlist should initially be closed.');

await test_driver.send_keys(selectlist, ArrowLeft);
assert_true(selectlist.matches(':open'),
'Arrow left should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow left should not change the selected value.');
await closeListbox();

await test_driver.send_keys(selectlist, ArrowUp);
assert_true(selectlist.matches(':open'),
'Arrow up should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow up should not change the selected value.');
await closeListbox();

await test_driver.send_keys(selectlist, ArrowRight);
assert_true(selectlist.matches(':open'),
'Arrow right should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow right should not change the selected value.');
await closeListbox();

await test_driver.send_keys(selectlist, ArrowDown);
assert_true(selectlist.matches(':open'),
'Arrow down should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow down should not change the selected value.');
}, `${id}: When the listbox is closed, all arrow keys should open the listbox.`);

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 = selectlist.querySelector('button');
if (customButton) {
await test_driver.send_keys(customButton, Enter);
} else {
await test_driver.send_keys(selectlist, Enter);
}
assert_false(selectlist.matches(':open'),
'Enter should not open the listbox when outside a form.');

form.appendChild(selectlist);
let formWasSubmitted = false;
form.addEventListener('submit', event => {
event.preventDefault();
formWasSubmitted = true;
}, {once: true});
if (customButton) {
await test_driver.send_keys(customButton, Enter);
} else {
await test_driver.send_keys(selectlist, Enter);
}
assert_true(formWasSubmitted,
'Enter should submit the form when the listbox is closed.');
assert_false(selectlist.matches(':open'),
'Enter should not open the listbox when it is in a form.');
}, `${id}: When the listbox is closed, the enter key should submit the form or do nothing.`);

promise_test(async t => {
addCloseCleanup(t);
const optionOne = selectlist.querySelector('.one');
const optionTwo = selectlist.querySelector('.two');
const optionThree = selectlist.querySelector('.three');

selectlist.value = 'two';
await test_driver.click(selectlist);
assert_true(selectlist.matches(':open'),
'The selectlist should open when clicked.');
assert_equals(document.activeElement, optionTwo,
'The selected option should receive initial focus.');

await test_driver.send_keys(document.activeElement, ArrowDown);
assert_equals(document.activeElement, optionThree,
'The next option should receive focus when the down arrow key is pressed.');
assert_equals(selectlist.value, 'two',
'The selectlists value should not change when focusing another option.');

await test_driver.send_keys(document.activeElement, ArrowUp);
assert_equals(document.activeElement, optionTwo,
'The previous option should receive focus when the up arrow key is pressed.');
assert_equals(selectlist.value, 'two',
'The selectlists value should not change when focusing another option.');

await test_driver.send_keys(document.activeElement, ArrowUp);
assert_equals(document.activeElement, optionOne,
'The first option should be selected.');
assert_equals(selectlist.value, 'two',
'The selectlists value should not change when focusing another option.');

await test_driver.send_keys(document.activeElement, Enter);
assert_false(selectlist.matches(':open'),
'The listbox should be closed after pressing enter.');
assert_equals(selectlist.value, 'one',
'The selectlists value should change after pressing enter on a different option.');
}, `${id}: When the listbox is open, the enter key should commit the selected option.`);
}
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,28 @@
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<selectlist id="selectList0">
<div id="selectList0-button0" slot="button" behavior="button" tabindex="0">button</div>
<option>one</option>
<option>two</option>
<option>three</option>
<button id="selectList0-button0" type=selectlist>button</button>
<option class=one>one</option>
<option class=two>two</option>
<option class=three>three</option>
</selectlist>

<selectlist id="selectList1">
<option id="selectList1-child0">one</option>
</selectlist>

<selectlist id="selectList2" disabled>
<div id="selectList2-button0" slot="button" behavior="button" tabindex="0">button</div>
<button id="selectList2-button0" type=selectlist>button</button>
<option disabled>one</option>
<option>two</option>
<option>three</option>
</selectlist>

<selectlist id="selectList3">
<div id="selectList3-button0" slot="button" behavior="button" tabindex="0">button</div>
<option>one</option>
<button id="selectList3-button0" type=selectlist>button</button>
<option class=one>one</option>
<option disabled>two</option>
<option>three</option>
<option class=three>three</option>
</selectlist>
<script>
// See https://w3c.github.io/webdriver/#keyboard-actions
Expand All @@ -52,31 +52,45 @@
assert_false(selectList.open, "selectlist should not be initially open");

await test_driver.send_keys(button, KEY_CODE_MAP.Enter);
assert_true(selectList.open, "Enter key should open selectlist");
assert_false(selectList.open, "Enter key shouldn't open selectlist");
await test_driver.send_keys(button, KEY_CODE_MAP.Space);
assert_true(selectList.open, "Space key should open selectlist");
assert_equals(selectList.value, "one");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "two", "Down arrow should go to next option");
assert_equals(document.activeElement, selectList.querySelector('.two'),
"Down arrow should focus the next option.");
assert_equals(selectList.value, "one", "Down arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "three", "Down arrow should go to next option");
assert_equals(document.activeElement, selectList.querySelector('.three'),
"Down arrow should focus the next option.");
assert_equals(selectList.value, "one", "Down arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "three", "Down arrow should do nothing if already at the last option");
assert_equals(document.activeElement, selectList.querySelector('.three'),
"Down arrow should do nothing if already at the last option.");
assert_equals(selectList.value, "one", "Down arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList.value, "two", "Up arrow should go to the previous option");
assert_equals(document.activeElement, selectList.querySelector('.two'),
"Up arrow should focus the previous option.");
assert_equals(selectList.value, "one", "Up arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList.value, "one", "Up arrow should go to the previous option");
assert_equals(document.activeElement, selectList.querySelector('.one'),
"Up arrow should focus the previous option.");
assert_equals(selectList.value, "one", "Up arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList.value, "one", "Up arrow should do nothing if already at the first option");
assert_equals(document.activeElement, selectList.querySelector('.one'),
"Up arrow should do nothing if already at the first option.");
assert_equals(selectList.value, "one", "Up arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_false(selectList.open, "Enter key should close selectlist");

await test_driver.send_keys(selectList, " ");
await test_driver.send_keys(button, KEY_CODE_MAP.Space);
assert_true(selectList.open, "Space key should open selectlist");

// This behavior is suspicious (since Space key can open the selectlist),
Expand Down Expand Up @@ -111,13 +125,18 @@
assert_false(selectList3.open, "selectlist should not be initially open");

await test_driver.send_keys(selectList3Button, KEY_CODE_MAP.Enter);
assert_true(selectList3.open, "Enter key should open selectlist");
assert_false(selectList3.open, "Enter key shouldn't open selectlist");

await test_driver.send_keys(selectList3Button, KEY_CODE_MAP.Space);
assert_true(selectList3.open, "Space key should open selectlist");
assert_equals(selectList3.value, "one");

await test_driver.send_keys(selectList3, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList3.value, "three", "Down arrow should go to next non-disabled option");
assert_equals(document.activeElement, selectList3.querySelector('.three'),
"Down arrow should go to next non-disabled option");

await test_driver.send_keys(selectList3, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList3.value, "one", "Up arrow should go to the previous non-disabled option");
assert_equals(document.activeElement, selectList3.querySelector('.one'),
"Up arrow should go to the previous non-disabled option");
}, "Validate Enter, Up/Down Arrow keyboard accessibility support for disabled <selectlist>");
</script>

0 comments on commit b0dc927

Please sign in to comment.