Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[popover] Improve focus handling
https://bugs.webkit.org/show_bug.cgi?id=255454

Reviewed by Ryosuke Niwa.

A lot of tests are failing because on macOS buttons are not considered to be focusable.
Start allowing this in case tabindex is set on form controls and adjust the tests accordingly.

* LayoutTests/fast/events/focus-label-legend-elements-with-tab-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-not-keyboard-focusable-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-not-keyboard-focusable.html:
* LayoutTests/platform/glib/fast/events/focus-label-legend-elements-with-tab-expected.txt: Removed.
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt:
* LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2-expected.txt:
* LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/popovers/popover-not-keyboard-focusable-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt:
* Source/WebCore/html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::isKeyboardFocusable const):
(WebCore::HTMLFormControlElement::isMouseFocusable const):

Canonical link: https://commits.webkit.org/263527@main
  • Loading branch information
rwlbuis committed Apr 29, 2023
1 parent ccbeb1e commit c5c4b9c
Show file tree
Hide file tree
Showing 18 changed files with 109 additions and 123 deletions.
Expand Up @@ -7,10 +7,11 @@ It should traverse focusable elements in the ascending order from 1 to 7 and the
2. A label element with an input element
3. An input elment inside a focusable label element
4. A label element with just text
5. A fieldset element with tabindex
6. A focusable legend element inside a fieldset element
7. An input element inside a fieldset element
7. An input element inside a fieldset element
6. A focusable legend element inside a fieldset element
5. A fieldset element with tabindex
4. A label element with just text
3. An input elment inside a focusable label element
2. A label element with an input element
Expand Down
@@ -1,10 +1,10 @@
Button1 Button2 Invoker1 Button3 Button4
Invoker after
Show popover
Toggle popover Popover with focusable element Other focusable element
Toggle popover Other focusable element

FAIL Popover focus navigation assert_equals: Hidden popover should be skipped expected Element node <button id="button2">Button2</button> but got Element node <span tabindex="0">Other focusable element</span>
FAIL Circular reference tab navigation assert_equals: Step 2 expected Element node <button id="circular1" autofocus="" popovertarget="popove... but got Element node <span tabindex="0">Other focusable element</span>
FAIL Popover focus navigation assert_equals: Focus should move from invoker into the open popover expected Element node <button id="inside_popover1" tabindex="0">Inside1</button> but got Element node <button id="button3" tabindex="0">Button3</button>
PASS Circular reference tab navigation
PASS Popover focus returns when popover is hidden by invoker
FAIL Popover focus only returns to invoker when focus is within the popover assert_equals: next up is the popover expected Element node <button>focusable element</button> but got Element node <span tabindex="0">Other focusable element</span>
FAIL Popover focus only returns to invoker when focus is within the popover assert_equals: focus does not move because it was not inside the popover expected Element node <span tabindex="0">Other focusable element</span> but got Element node <button popovertarget="focus-return2-p" tabindex="0">Togg...

Expand Up @@ -11,23 +11,23 @@
<script src="resources/popover-utils.js"></script>

<div id=fixup>
<button id=button1>Button1</button>
<button id=button1 tabindex="0">Button1</button>
<div popover id=popover1 style="top:100px">
<button id=inside_popover1>Inside1</button>
<button id=invoker2 popovertarget=popover2>Nested Invoker 2</button>
<button id=inside_popover2>Inside2</button>
<button id=inside_popover1 tabindex="0">Inside1</button>
<button id=invoker2 popovertarget=popover2 tabindex="0">Nested Invoker 2</button>
<button id=inside_popover2 tabindex="0">Inside2</button>
</div>
<button id=button2>Button2</button>
<button popovertarget=popover1 id=invoker1>Invoker1</button>
<button id=button3>Button3</button>
<button id=button2 tabindex="0">Button2</button>
<button popovertarget=popover1 id=invoker1 tabindex="0">Invoker1</button>
<button id=button3 tabindex="0">Button3</button>
<div popover id=popover2 style="top:200px">
<button id=inside_popover3>Inside3</button>
<button id=invoker3 popovertarget=popover3>Nested Invoker 3</button>
<button id=inside_popover3 tabindex="0">Inside3</button>
<button id=invoker3 popovertarget=popover3 tabindex="0">Nested Invoker 3</button>
</div>
<div popover id=popover3 style="top:300px">
Non-focusable popover
</div>
<button id=button4>Button4</button>
<button id=button4 tabindex="0">Button4</button>
</div>
<style>
#fixup [popover] {
Expand Down Expand Up @@ -90,13 +90,13 @@
}, "Popover focus navigation");
</script>

<button id=circular0 popovertarget=popover4>Invoker</button>
<button id=circular0 popovertarget=popover4 tabindex="0">Invoker</button>
<div id=popover4 popover>
<button id=circular1 autofocus popovertarget=popover4 popovertargetaction=hide></button>
<button id=circular2 popovertarget=popover4 popovertargetaction=show></button>
<button id=circular3 popovertarget=popover4></button>
<button id=circular1 autofocus popovertarget=popover4 popovertargetaction=hide tabindex="0"></button>
<button id=circular2 popovertarget=popover4 popovertargetaction=show tabindex="0"></button>
<button id=circular3 popovertarget=popover4 tabindex="0"></button>
</div>
<button id=circular4>after</button>
<button id=circular4 tabindex="0">after</button>
<script>
promise_test(async t => {
circular0.focus();
Expand Down Expand Up @@ -129,8 +129,8 @@
</script>

<div id=focus-return2>
<button popovertarget=focus-return2-p>Toggle popover</button>
<div popover id=focus-return2-p>Popover with <button>focusable element</button></div>
<button popovertarget=focus-return2-p tabindex="0">Toggle popover</button>
<div popover id=focus-return2-p>Popover with <button tabindex="0">focusable element</button></div>
<span tabindex=0>Other focusable element</span>
</div>
<script>
Expand Down
@@ -1,26 +1,26 @@

PASS Popover focus test: default behavior - popover is not focused
FAIL Popover button click focus test: default behavior - popover is not focused assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: default behavior - popover is not focused assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: default behavior - popover is not focused
PASS Popover focus test: autofocus popover
FAIL Popover button click focus test: autofocus popover assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus popover assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus popover
PASS Popover focus test: autofocus empty popover
FAIL Popover button click focus test: autofocus empty popover assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus empty popover assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus empty popover
PASS Popover focus test: autofocus popover with button
FAIL Popover button click focus test: autofocus popover with button assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus popover with button assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus popover with button
PASS Popover focus test: autofocus child
FAIL Popover button click focus test: autofocus child assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus child assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus child
PASS Popover focus test: autofocus on tabindex=0 element
FAIL Popover button click focus test: autofocus on tabindex=0 element assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus on tabindex=0 element assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus on tabindex=0 element
PASS Popover focus test: autofocus multiple children
FAIL Popover button click focus test: autofocus multiple children assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus multiple children assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus multiple children
PASS Popover focus test: autofocus popover and multiple autofocus children
FAIL Popover button click focus test: autofocus popover and multiple autofocus children assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button popovertarget="popover-id">Click me</button> but got Element node <button id="priorFocus"></button>
FAIL Popover button click focus test: autofocus popover and multiple autofocus children assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <button tabindex="0" id="priorFocus"></button>
PASS Popover corner cases test: autofocus popover and multiple autofocus children

Expand Up @@ -13,7 +13,7 @@

<div popover data-test='default behavior - popover is not focused' data-no-focus>
<p>This is a popover</p>
<button>first button</button>
<button tabindex="0">first button</button>
</div>

<div popover data-test='autofocus popover' autofocus tabindex=-1 class=should-be-focused>
Expand All @@ -24,29 +24,29 @@

<div popover data-test='autofocus popover with button' autofocus tabindex=-1 class=should-be-focused>
<p>This is a popover</p>
<button>button</button>
<button tabindex="0">button</button>
</div>

<div popover data-test='autofocus child'>
<p>This is a popover</p>
<button autofocus class=should-be-focused>autofocus button</button>
<button autofocus class=should-be-focused tabindex="0">autofocus button</button>
</div>

<div popover data-test='autofocus on tabindex=0 element'>
<p autofocus tabindex=0 class=should-be-focused>This is a popover with autofocus on a tabindex=0 element</p>
<button>button</button>
<button tabindex="0">button</button>
</div>

<div popover data-test='autofocus multiple children'>
<p>This is a popover</p>
<button autofocus class=should-be-focused>autofocus button</button>
<button autofocus>second autofocus button</button>
<button autofocus class=should-be-focused tabindex="0">autofocus button</button>
<button autofocus tabindex="0">second autofocus button</button>
</div>

<div popover autofocus tabindex=-1 data-test='autofocus popover and multiple autofocus children' class=should-be-focused>
<p>This is a popover</p>
<button autofocus>autofocus button</button>
<button autofocus>second autofocus button</button>
<button autofocus tabindex="0">autofocus button</button>
<button autofocus tabindex="0">second autofocus button</button>
</div>

<style>
Expand All @@ -71,11 +71,13 @@
button.remove();
});
popover.id = popoverId;
button.setAttribute('tabindex', '0');
button.setAttribute('popovertarget', popoverId);
return button;
}
function addPriorFocus(t) {
const priorFocus = document.createElement('button');
priorFocus.setAttribute("tabindex", "0");
priorFocus.id = 'priorFocus';
document.body.appendChild(priorFocus);
t.add_cleanup(() => priorFocus.remove());
Expand Down Expand Up @@ -218,6 +220,7 @@

// Same thing, but the button is unrelated (no popovertarget)
button = document.createElement('button');
button.setAttribute("tabindex", "0");
document.body.appendChild(button);
priorFocus.focus();
popover.showPopover();
Expand Down
Expand Up @@ -5,7 +5,7 @@ PASS Canceling pointer events should not keep clicks from light dismissing popov
PASS Clicking inside a popover does not close that popover
PASS Popovers close on pointerup, not pointerdown
PASS Synthetic events can't close popovers
FAIL Moving focus outside the popover should not dismiss the popover assert_equals: Focus should move to a button outside the popover expected Element node <button id="after_p1">Next control after popover1</button> but got Element node <body><button id="b1t" popovertarget="p1">Popover 1</butt...
PASS Moving focus outside the popover should not dismiss the popover
PASS Clicking inside a child popover shouldn't close either popover
PASS Clicking inside a parent popover should close child popover
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover
Expand All @@ -20,7 +20,7 @@ PASS An invoking element that was not used to invoke the popover can still be pa
FAIL Scrolling within a popover should not close the popover promise_test: Unhandled rejection with value: object "Error: Unknown source type "wheel"."
PASS Clicking inside a shadow DOM popover does not close that popover
PASS Clicking outside a shadow DOM popover should close that popover
FAIL Moving focus back to the anchor element should not dismiss the popover assert_equals: Focus should move to the anchor element expected Element node <button id="p8anchor">Popover8 anchor (no action)</button> but got Element node <body><button id="b1t" popovertarget="p1">Popover 1</butt...
PASS Moving focus back to the anchor element should not dismiss the popover
PASS Ensure circular/convoluted ancestral relationships are functional
PASS Ensure circular/convoluted ancestral relationships are functional, with a direct showPopover()
PASS Hide the target popover during "hide all popovers until"
Expand Down
Expand Up @@ -13,7 +13,7 @@

<button id=b1t popovertarget='p1'>Popover 1</button>
<button id=b1s popovertarget='p1' popovertargetaction=show>Popover 1</button>
<button id=p1anchor>Popover1 anchor (no action)</button>
<button id=p1anchor tabindex="0">Popover1 anchor (no action)</button>
<span id=outside>Outside all popovers</span>
<div popover id=p1 anchor=p1anchor>
<span id=inside1>Inside popover 1</span>
Expand All @@ -23,7 +23,7 @@
<div popover id=p2 anchor=b2>
<span id=inside2>Inside popover 2</span>
</div>
<button id=after_p1>Next control after popover1</button>
<button id=after_p1 tabindex="0">Next control after popover1</button>
<style>
#p1 {top: 50px;}
#p2 {top: 120px;}
Expand Down Expand Up @@ -333,7 +333,7 @@

<my-element id="myElement">
<template shadowrootmode="open">
<button id=b7 onclick='showPopover7()'>Popover7</button>
<button id=b7 onclick='showPopover7()' tabindex="0">Popover7</button>
<div popover id=p7 anchor=b7 style="top: 100px;">
<p>Popover content.</p>
<input id="inside7" type="text" placeholder="some text">
Expand Down Expand Up @@ -365,10 +365,10 @@
</script>

<div popover id=p8 anchor=p8anchor>
<button>Button</button>
<button tabindex="0">Button</button>
<span id=inside8after>Inside popover 8 after button</span>
</div>
<button id=p8anchor>Popover8 anchor (no action)</button>
<button id=p8anchor tabindex="0">Popover8 anchor (no action)</button>
<script>
promise_test(async () => {
const popover8 = document.querySelector('#p8');
Expand Down Expand Up @@ -399,7 +399,7 @@
<div popover id=convoluted_p3 anchor=convoluted_anchor>Popover 3
<button popovertarget=convoluted_p4>Open Popover 4</button>
</div>
<button onclick="convoluted_p1.showPopover()">Open convoluted popover</button>
<button onclick="convoluted_p1.showPopover()" tabindex="0">Open convoluted popover</button>
<style>
#convoluted_p1 {top:50px;}
#convoluted_p2 {top:150px;}
Expand Down
@@ -1,8 +1,4 @@
Button 1
This is a popover without a focusable element
Button 1 Button 2

Button 2

FAIL Popover should not be keyboard focusable assert_equals: Keyboard focus should skip the open popover expected Element node <button id="secondfocus">Button 2</button> but got Element node <body><button id="firstfocus">Button 1</button>
<div popo...
PASS Popover should not be keyboard focusable

Expand Up @@ -10,11 +10,11 @@
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>

<button id=firstfocus>Button 1</button>
<button tabindex="0" id=firstfocus>Button 1</button>
<div popover>
<p>This is a popover without a focusable element</p>
</div>
<button id=secondfocus>Button 2</button>
<button tabindex="0" id=secondfocus>Button 2</button>

<script>
promise_test(async () => {
Expand All @@ -34,6 +34,7 @@

// Add a focusable button to the popover and make sure we can focus that
const button = document.createElement('button');
button.setAttribute("tabindex", "0");
popover.appendChild(button);
b1.focus();
popover.showPopover();
Expand Down

This file was deleted.

0 comments on commit c5c4b9c

Please sign in to comment.