Skip to content

Commit ffc70a7

Browse files
nt1mnt1m
committed
[popover] Avoid conflicting interactions in the top layer
https://bugs.webkit.org/show_bug.cgi?id=252297 rdar://105763866 Reviewed by Simon Fraser. - Run "hide all popovers" algorithm every time something is appended to the top layer (since the auto popovers will be covered by the new element) - Don't allow something in the popover showing visibility state to be appended in the top layer, since it is already there (this is similar to how dialogs can't go fullscreen). - Fix test to avoid race conditions by properly waiting for async operations * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-combinations-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions.html: * LayoutTests/platform/ios/TestExpectations: * LayoutTests/platform/gtk/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt: Added. * Source/WebCore/dom/FullscreenManager.cpp: (WebCore::FullscreenManager::requestFullscreenForElement): (WebCore::FullscreenManager::willEnterFullscreen): (WebCore::isInPopoverShowingState): * Source/WebCore/html/HTMLDialogElement.cpp: (WebCore::HTMLDialogElement::showModal): (WebCore::HTMLDialogElement::runFocusingSteps): Canonical link: https://commits.webkit.org/261317@main
1 parent db48c67 commit ffc70a7

9 files changed

Lines changed: 119 additions & 66 deletions

File tree

‎LayoutTests/TestExpectations‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,7 +710,6 @@ webkit.org/b/250171 imported/w3c/web-platform-tests/html/semantics/popovers/popo
710710
webkit.org/b/250171 imported/w3c/web-platform-tests/html/semantics/popovers/popover-anchor-scroll-display.html [ ImageOnlyFailure ]
711711
webkit.org/b/250171 imported/w3c/web-platform-tests/html/semantics/popovers/popover-dialog-crash.html [ Skip ]
712712
webkit.org/b/250171 imported/w3c/web-platform-tests/html/semantics/popovers/popover-manual-crash.html [ Skip ]
713-
webkit.org/b/250171 imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions.html [ Skip ]
714713
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/css-module/charset-2.html [ Skip ]
715714
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/css-module/charset-bom.html [ Skip ]
716715
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/css-module/referrer-policies.sub.html [ Skip ]

‎LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt‎

Lines changed: 26 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,31 @@
1+
This is a popover
12

2-
FAIL Popover focus test: default behavior - popover is not focused assert_false: Opening a modal dialog should hide the popover expected false got true
3-
FAIL Popover button click focus test: default behavior - popover is not focused assert_false: popover should start out hidden expected false got true
4-
PASS Popover corner cases test: default behavior - popover is not focused
5-
FAIL Popover focus test: autofocus popover assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
6-
7-
<div popover="" data-test="autofocus popover" aut...
8-
FAIL Popover button click focus test: autofocus popover assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
9-
10-
<div popover="" data-test="autofocus popover" aut...
11-
PASS Popover corner cases test: autofocus popover
12-
FAIL Popover focus test: autofocus empty popover assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
13-
14-
<div popover="" data-test="autofocus popover" aut...
15-
FAIL Popover button click focus test: autofocus empty popover assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
3+
autofocus button second autofocus button
164

17-
<div popover="" data-test="autofocus popover" aut...
18-
PASS Popover corner cases test: autofocus empty popover
19-
FAIL Popover focus test: autofocus popover with button assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
20-
21-
<div popover="" data-test="autofocus popover" aut...
22-
FAIL Popover button click focus test: autofocus popover with button assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
23-
24-
<div popover="" data-test="autofocus popover" aut...
25-
PASS Popover corner cases test: autofocus popover with button
26-
FAIL Popover focus test: autofocus child assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
27-
28-
<div popover="" data-test="autofocus popover" aut...
29-
FAIL Popover button click focus test: autofocus child assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
5+
FAIL Popover focus test: default behavior - popover is not focused assert_true: expected true got false
6+
FAIL Popover button click focus test: default behavior - popover is not focused assert_equals: focus should return to the prior focus expected Element node <button id="priorFocus"></button> but got Element node <body>
307

318
<div popover="" data-test="autofocus popover" aut...
32-
PASS Popover corner cases test: autofocus child
33-
FAIL Popover focus test: autofocus on tabindex=0 element assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
34-
35-
<div popover="" data-test="autofocus popover" aut...
36-
FAIL Popover button click focus test: autofocus on tabindex=0 element assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
37-
38-
<div popover="" data-test="autofocus popover" aut...
39-
PASS Popover corner cases test: autofocus on tabindex=0 element
40-
FAIL Popover focus test: autofocus multiple children assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
41-
42-
<div popover="" data-test="autofocus popover" aut...
43-
FAIL Popover button click focus test: autofocus multiple children assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
44-
45-
<div popover="" data-test="autofocus popover" aut...
46-
PASS Popover corner cases test: autofocus multiple children
47-
FAIL Popover focus test: autofocus popover and multiple autofocus children assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
48-
49-
<div popover="" data-test="autofocus popover" aut...
50-
FAIL Popover button click focus test: autofocus popover and multiple autofocus children assert_equals: expected Element node <button id="priorFocus"></button> but got Element node <body>
51-
52-
<div popover="" data-test="autofocus popover" aut...
53-
PASS Popover corner cases test: autofocus popover and multiple autofocus children
9+
PASS Popover corner cases test: default behavior - popover is not focused
10+
FAIL Popover focus test: autofocus popover assert_equals: autofocus popover activated by popover.showPopover() expected Element node <div popover="" data-test="autofocus popover" autofocus="... but got Element node <button id="priorFocus"></button>
11+
FAIL Popover button click focus test: autofocus popover assert_false: popover should start out hidden expected false got true
12+
FAIL Popover corner cases test: autofocus popover assert_false: popover should start out hidden expected false got true
13+
FAIL Popover focus test: autofocus empty popover assert_equals: autofocus empty popover activated by popover.showPopover() expected Element node <div popover="" data-test="autofocus empty popover" autof... but got Element node <button id="priorFocus"></button>
14+
FAIL Popover button click focus test: autofocus empty popover assert_false: popover should start out hidden expected false got true
15+
FAIL Popover corner cases test: autofocus empty popover assert_false: popover should start out hidden expected false got true
16+
FAIL Popover focus test: autofocus popover with button assert_equals: autofocus popover with button activated by popover.showPopover() expected Element node <div popover="" data-test="autofocus popover with button"... but got Element node <button id="priorFocus"></button>
17+
FAIL Popover button click focus test: autofocus popover with button assert_false: popover should start out hidden expected false got true
18+
FAIL Popover corner cases test: autofocus popover with button assert_false: popover should start out hidden expected false got true
19+
FAIL Popover focus test: autofocus child assert_equals: autofocus child activated by popover.showPopover() expected Element node <button autofocus="" class="should-be-focused">autofocus ... but got Element node <button id="priorFocus"></button>
20+
FAIL Popover button click focus test: autofocus child assert_false: popover should start out hidden expected false got true
21+
FAIL Popover corner cases test: autofocus child assert_false: popover should start out hidden expected false got true
22+
FAIL Popover focus test: autofocus on tabindex=0 element assert_equals: autofocus on tabindex=0 element activated by popover.showPopover() expected Element node <p autofocus="" tabindex="0" class="should-be-focused">Th... but got Element node <button id="priorFocus"></button>
23+
FAIL Popover button click focus test: autofocus on tabindex=0 element assert_false: popover should start out hidden expected false got true
24+
FAIL Popover corner cases test: autofocus on tabindex=0 element assert_false: popover should start out hidden expected false got true
25+
FAIL Popover focus test: autofocus multiple children assert_equals: autofocus multiple children activated by popover.showPopover() expected Element node <button autofocus="" class="should-be-focused">autofocus ... but got Element node <button id="priorFocus"></button>
26+
FAIL Popover button click focus test: autofocus multiple children assert_false: popover should start out hidden expected false got true
27+
FAIL Popover corner cases test: autofocus multiple children assert_false: popover should start out hidden expected false got true
28+
FAIL Popover focus test: autofocus popover and multiple autofocus children assert_equals: autofocus popover and multiple autofocus children activated by popover.showPopover() expected Element node <div popover="" autofocus="" tabindex="-1" data-test="aut... but got Element node <button id="priorFocus"></button>
29+
FAIL Popover button click focus test: autofocus popover and multiple autofocus children assert_false: popover should start out hidden expected false got true
30+
FAIL Popover corner cases test: autofocus popover and multiple autofocus children assert_false: popover should start out hidden expected false got true
5431

‎LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-combinations-expected.txt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
Visible button
22

3-
FAIL Popover combination: Popover Dialog assert_throws_dom: Calling showModal() on an already-showing Popover should throw InvalidStateError function "() => ex.showModal()" did not throw
4-
FAIL Popover combination: Open Non-modal Popover Dialog assert_throws_dom: Calling showModal() on an already-showing Popover should throw InvalidStateError function "() => ex.showModal()" did not throw
5-
FAIL Popover combination: Fullscreen Popover assert_false: requestFullscreen() should not succeed when the element is an already-showing Popover expected false got true
6-
FAIL Popover combination: Fullscreen Popover Dialog assert_throws_dom: Calling showModal() on an already-showing Popover should throw InvalidStateError function "() => ex.showModal()" did not throw
7-
FAIL Popover combination: Fullscreen Open Non-modal Popover Dialog assert_throws_dom: Calling showModal() on an already-showing Popover should throw InvalidStateError function "() => ex.showModal()" did not throw
3+
FAIL Popover combination: Popover Dialog assert_throws_dom: Calling show() on an already-showing Popover should throw InvalidStateError function "() => ex.show()" did not throw
4+
FAIL Popover combination: Open Non-modal Popover Dialog assert_throws_dom: Calling show() on an already-showing Popover should throw InvalidStateError function "() => ex.show()" did not throw
5+
FAIL Popover combination: Fullscreen Popover assert_true: Invoking element should be able to invoke all popovers: Popover doesn't match :open expected true got false
6+
FAIL Popover combination: Fullscreen Popover Dialog assert_throws_dom: Calling show() on an already-showing Popover should throw InvalidStateError function "() => ex.show()" did not throw
7+
FAIL Popover combination: Fullscreen Open Non-modal Popover Dialog assert_throws_dom: Calling show() on an already-showing Popover should throw InvalidStateError function "() => ex.show()" did not throw
88

‎LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions-expected.txt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
2+
PASS A Popover should close a Popover.
3+
PASS A Modal Dialog should close a Popover.
4+
PASS A Fullscreen Element should close a Popover.
5+
PASS A Popover should *not* close a Modal Dialog.
6+
PASS A Modal Dialog should *not* close a Modal Dialog.
7+
PASS A Fullscreen Element should *not* close a Modal Dialog.
8+
PASS A Popover should *not* close a Fullscreen Element.
9+
PASS A Modal Dialog should *not* close a Fullscreen Element.
10+
FAIL A Fullscreen Element should *not* close a Fullscreen Element. promise_test: Unhandled rejection with value: object "TypeError: Type error"
11+

‎LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions.html‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
<body>
1414
<script>
1515
const types = Object.freeze({
16-
popover: Symbol("Popover API"),
16+
popover: Symbol("Popover"),
1717
modalDialog: Symbol("Modal Dialog"),
1818
fullscreen: Symbol("Fullscreen Element"),
1919
});
@@ -30,17 +30,17 @@
3030
type: types.modalDialog,
3131
closes: [types.popover],
3232
createElement: () => document.createElement('dialog'),
33-
trigger: function() {this.element.showModal();this.showing=true;},
34-
close: function() {this.element.close();this.showing=false;},
35-
isTopLayer: function() {return !!(this.element.isConnected && this.showing);},
33+
trigger: function() {this.element.showModal()},
34+
close: function() {this.element.close()},
35+
isTopLayer: function() {return this.element.matches(':modal')},
3636
},
3737
{
3838
type: types.fullscreen,
3939
closes: [types.popover],
4040
createElement: () => document.createElement('div'),
4141
trigger: async function(visibleElement) {assert_false(this.isTopLayer());await blessTopLayer(visibleElement);await this.element.requestFullscreen();},
42-
close: function() {document.exitFullscreen();},
43-
isTopLayer: function() {return this.element.matches(':fullscreen');},
42+
close: async function() {await document.exitFullscreen();},
43+
isTopLayer: function() {return this.element.matches(':fullscreen')},
4444
},
4545
];
4646

@@ -53,10 +53,10 @@
5353
assert_false(ex.isTopLayer(),'Element should start out not in the top layer');
5454
return element;
5555
}
56-
function doneWithExample(ex) {
56+
async function doneWithExample(ex) {
5757
assert_true(!!ex.element);
5858
if (ex.isTopLayer())
59-
ex.close();
59+
await ex.close();
6060
ex.element.remove();
6161
ex.element = null;
6262
}
@@ -66,11 +66,16 @@
6666
const example1 = Object.assign([],examples[i]);
6767
const example2 = Object.assign([],examples[j]);
6868
const shouldClose = example2.closes.includes(example1.type);
69-
const desc = `A ${example2.type.description} should ${shouldClose ? "" : "*not*"} close a ${example1.type.description}`;
69+
const desc = `A ${example2.type.description} should${shouldClose ? "" : " *not*"} close a ${example1.type.description}.`;
7070
promise_test(async t => {
7171
const element1 = createElement(example1);
7272
const element2 = createElement(example2);
73-
t.add_cleanup(() => {doneWithExample(example1);doneWithExample(example2);});
73+
t.add_cleanup(() => {
74+
return Promise.all([
75+
doneWithExample(example1),
76+
doneWithExample(example2),
77+
]);
78+
});
7479
await example1.trigger(document.body); // Open the 1st top layer element
7580
assert_true(example1.isTopLayer()); // Make sure it is top layer
7681
await example2.trigger(element1); // Open the 2nd top layer element

‎LayoutTests/platform/gtk/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
This is a popover
2+
3+
autofocus button second autofocus button
4+
5+
FAIL Popover focus test: default behavior - popover is not focused assert_true: expected true got false
6+
FAIL Popover button click focus test: default behavior - popover is not focused assert_equals: focus should return to the prior focus expected Element node <button id="priorFocus"></button> but got Element node <button popovertarget="popover-id">Click me</button>
7+
PASS Popover corner cases test: default behavior - popover is not focused
8+
FAIL Popover focus test: autofocus popover assert_equals: autofocus popover activated by popover.showPopover() expected Element node <div popover="" data-test="autofocus popover" autofocus="... but got Element node <button id="priorFocus"></button>
9+
FAIL Popover button click focus test: autofocus popover assert_false: popover should start out hidden expected false got true
10+
FAIL Popover corner cases test: autofocus popover assert_false: popover should start out hidden expected false got true
11+
FAIL Popover focus test: autofocus empty popover assert_equals: autofocus empty popover activated by popover.showPopover() expected Element node <div popover="" data-test="autofocus empty popover" autof... but got Element node <button id="priorFocus"></button>
12+
FAIL Popover button click focus test: autofocus empty popover assert_false: popover should start out hidden expected false got true
13+
FAIL Popover corner cases test: autofocus empty popover assert_false: popover should start out hidden expected false got true
14+
FAIL Popover focus test: autofocus popover with button assert_equals: autofocus popover with button activated by popover.showPopover() expected Element node <div popover="" data-test="autofocus popover with button"... but got Element node <button id="priorFocus"></button>
15+
FAIL Popover button click focus test: autofocus popover with button assert_false: popover should start out hidden expected false got true
16+
FAIL Popover corner cases test: autofocus popover with button assert_false: popover should start out hidden expected false got true
17+
FAIL Popover focus test: autofocus child assert_equals: autofocus child activated by popover.showPopover() expected Element node <button autofocus="" class="should-be-focused">autofocus ... but got Element node <button id="priorFocus"></button>
18+
FAIL Popover button click focus test: autofocus child assert_false: popover should start out hidden expected false got true
19+
FAIL Popover corner cases test: autofocus child assert_false: popover should start out hidden expected false got true
20+
FAIL Popover focus test: autofocus on tabindex=0 element assert_equals: autofocus on tabindex=0 element activated by popover.showPopover() expected Element node <p autofocus="" tabindex="0" class="should-be-focused">Th... but got Element node <button id="priorFocus"></button>
21+
FAIL Popover button click focus test: autofocus on tabindex=0 element assert_false: popover should start out hidden expected false got true
22+
FAIL Popover corner cases test: autofocus on tabindex=0 element assert_false: popover should start out hidden expected false got true
23+
FAIL Popover focus test: autofocus multiple children assert_equals: autofocus multiple children activated by popover.showPopover() expected Element node <button autofocus="" class="should-be-focused">autofocus ... but got Element node <button id="priorFocus"></button>
24+
FAIL Popover button click focus test: autofocus multiple children assert_false: popover should start out hidden expected false got true
25+
FAIL Popover corner cases test: autofocus multiple children assert_false: popover should start out hidden expected false got true
26+
FAIL Popover focus test: autofocus popover and multiple autofocus children assert_equals: autofocus popover and multiple autofocus children activated by popover.showPopover() expected Element node <div popover="" autofocus="" tabindex="-1" data-test="aut... but got Element node <button id="priorFocus"></button>
27+
FAIL Popover button click focus test: autofocus popover and multiple autofocus children assert_false: popover should start out hidden expected false got true
28+
FAIL Popover corner cases test: autofocus popover and multiple autofocus children assert_false: popover should start out hidden expected false got true
29+

‎LayoutTests/platform/ios/TestExpectations‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ http/tests/fullscreen
8989
imported/w3c/web-platform-tests/fullscreen
9090
compositing/no-compositing-when-full-screen-is-present.html
9191
webkit.org/b/200308 fast/events/touch/ios/content-observation/non-visible-content-change-in-fullscreen-mode.html [ Skip ]
92-
92+
imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions.html [ Skip ]
9393
media/video-supports-fullscreen.html [ Skip ]
9494

9595
# Encrypted Media Extensions are not enabled

0 commit comments

Comments
 (0)