Skip to content

Commit

Permalink
Fix interactive elements in customizable select picker
Browse files Browse the repository at this point in the history
MenuListSelectType::DefaultEventHandler has code which may focus the
select element, show/hide the picker, and prevent other default event
handlers from running. This code runs when clicking anything in the
picker since we aren't looking at the event path anymore. This results
in interactive elements not being clickable/focusable in the picker.
This patch fixes this by not running any of this code while the base
appearance picker is open.

This likely stopped working when the PopoverButtonNestingBehavior flag
was used in MenuListSelectType::DefaultEventHandler because the old code
there which no longer gets run would also prevent the
DefaultEventHandler from doing anything in this case.

Change-Id: I5240766badaa0025072bf3123e5dd9f18466de99
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6240329
Reviewed-by: Traian Captan <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1420083}
  • Loading branch information
josepharhar authored and chromium-wpt-export-bot committed Feb 13, 2025
1 parent 399d4ad commit 2b5429e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
const option2 = wrapper.querySelector('.option2');
const option3 = wrapper.querySelector('.option3');
promise_test(async (t) => {
assert_false(select.matches(':open'));
assert_false(select.matches(':open'),'select should be closed at the start of the test');
let eventList = [];
function assert_events(expectedEvents,message) {
message = message || "Mismatch";
Expand All @@ -60,8 +60,6 @@
assert_events([
'pointerdown on select at select', 'pointerdown on select at wrapper',
'mousedown on select at select', 'mousedown on select at wrapper',
'focusin on select at select', 'focusin on select at wrapper',
'focusout on select at select', 'focusout on select at wrapper',
'focusin on option1 at select', 'focusin on option1 at wrapper',
'pointerup on select at select', 'pointerup on select at wrapper',
'mouseup on select at select', 'mouseup on select at wrapper',
Expand Down Expand Up @@ -98,7 +96,7 @@

// Press escape, no preventDefault
await test_driver.send_keys(document.activeElement, keys.Escape);
assert_false(select.matches(':open'));
assert_false(select.matches(':open'),'select should be closed escape no preventDefault');
assert_events([
'keydown on option2 at select', 'keydown on option2 at wrapper',
'focusout on option2 at select', 'focusout on option2 at wrapper',
Expand Down Expand Up @@ -126,7 +124,7 @@

// Press enter to select an option, no preventDefault
await test_driver.send_keys(document.activeElement, keys.Enter);
assert_false(select.matches(':open'));
assert_false(select.matches(':open'),'select should be closed enter no preventDefault');
assert_equals(select.value,'two');
assert_events([
'keydown on option2 at select', 'keydown on option2 at wrapper',
Expand All @@ -144,29 +142,39 @@
// Click on an option, with preventDefault
wrapper.addEventListener('mouseup',(e) => e.preventDefault(),{once:true});
assert_equals(select.selectedOptions[0].innerText,'two');
await test_driver.click(option1);
await (new test_driver.Actions()
.pointerMove(1, 1, {origin: option1})
.pointerDown()
.pointerUp())
.send();
assert_true(select.matches(':open'),'click should be cancelled');
assert_events([
'pointerdown on option1 at select', 'pointerdown on option1 at wrapper',
'mousedown on option1 at select', 'mousedown on option1 at wrapper',
'focusout on option2 at select', 'focusout on option2 at wrapper',
'focusin on select at select', 'focusin on select at wrapper',
'focusin on option1 at select', 'focusin on option1 at wrapper',
'pointerup on option1 at select', 'pointerup on option1 at wrapper',
'mouseup on option1 at select', 'mouseup on option1 at wrapper',
'click on option1 at select', 'click on option1 at wrapper'
],'click option, with preventDefault');

// Click on an option, no preventDefault
assert_equals(select.selectedOptions[0].innerText,'two');
await test_driver.click(option1);
assert_false(select.matches(':open'));
await (new test_driver.Actions()
.pointerMove(1, 1, {origin: option1})
.pointerDown()
.pointerUp())
.send();
assert_false(select.matches(':open'),'select should be closed click option no preventDefault');
assert_events([
'pointerdown on option1 at select', 'pointerdown on option1 at wrapper',
'mousedown on option1 at select', 'mousedown on option1 at wrapper',
'pointerup on option1 at select', 'pointerup on option1 at wrapper',
'mouseup on option1 at select', 'mouseup on option1 at wrapper',
'input on select at select', 'input on select at wrapper',
'change on select at select', 'change on select at wrapper',
'focusout on option1 at select', 'focusout on option1 at wrapper',
'focusin on select at select', 'focusin on select at wrapper',
'click on option1 at select', 'click on option1 at wrapper'
],'click option, no preventDefault');
},`Events, ${wrapper.dataset.description}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@

async function closeListbox() {
await test_driver.click(select);
select.focus();
await new Promise(requestAnimationFrame);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<!DOCTYPE html>
<link rel=author href="mailto:[email protected]">
<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>

<select>
<button>invoker</button>
<button id=button>button</button>
<option>option</option>
</select>

<style>
select, ::picker(select) {
appearance: base-select;
}
</style>

<script>
function click(element) {
return (new test_driver.Actions()
.pointerMove(1, 1, {origin: element})
.pointerDown()
.pointerUp())
.send();
}

promise_test(async () => {
const select = document.querySelector('select');
const button = document.getElementById('button');
const input = document.createElement('input');
select.appendChild(input);

await click(select);
assert_true(select.matches(':open'),
'select should open after being clicked.');

await click(button);
assert_true(select.matches(':open'),
'select should stay open after clicking button in picker.');
assert_equals(document.activeElement, button, 'button');

await click(input);
assert_true(select.matches(':open'),
'select should stay open after clicking input in picker.');
assert_equals(document.activeElement, input, 'input');
}, 'Clicking interactive elements inside the select picker should focus them.');
</script>

0 comments on commit 2b5429e

Please sign in to comment.