Skip to content

Commit 57d6088

Browse files
mfreed7chromium-wpt-export-bot
authored andcommitted
Improve testing dialog closedby and requestClose
See this request: whatwg/html#10983 (comment) plus this comment: https://chromium-review.googlesource.com/c/chromium/src/+/6245064/comments/e932d31a_c996d18e This just adds more testing of dialog closedby and requestClose, to make sure close watchers are still set up, the `:open` pseudo class works properly, events get fired correctly, etc., when the dialog `open` attribute is manually added and removed. Bug: 376516550 Change-Id: Ibdee09c5f722db2ce97c64c38c39726c1503e1d2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6253798 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: Joey Arhar <[email protected]> Reviewed-by: Luke <[email protected]> Cr-Commit-Position: refs/heads/main@{#1422074}
1 parent c474a20 commit 57d6088

File tree

3 files changed

+51
-16
lines changed

3 files changed

+51
-16
lines changed

html/semantics/interactive-elements/the-dialog-element/dialog-closedby-start-open.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,16 @@
1616
promise_test(async (t) => {
1717
const dialog = document.querySelector('dialog#test1');
1818
assert_true(dialog.open);
19+
assert_true(dialog.matches(':open'));
1920
await new test_driver.send_keys(document.documentElement,ESC);
2021
assert_false(dialog.open);
22+
assert_false(dialog.matches(':open'));
2123
dialog.showModal();
2224
assert_true(dialog.open);
25+
assert_true(dialog.matches(':open'));
2326
await new test_driver.send_keys(document.documentElement,ESC);
2427
assert_false(dialog.open);
28+
assert_false(dialog.matches(':open'));
2529
}, `Dialogs that start open and have closedby should still function`);
2630
</script>
2731

@@ -48,7 +52,9 @@
4852
});
4953
});
5054
assert_true(dialog.open);
55+
assert_true(dialog.matches(':open'));
5156
await new test_driver.send_keys(document.documentElement,ESC);
5257
assert_false(dialog.open,'ESC should still work');
58+
assert_false(dialog.matches(':open'));
5359
}, `Opening and closing a dialog during the dialog focus fixup should still leave closedby functional`);
5460
</script>

html/semantics/interactive-elements/the-dialog-element/dialog-closedby.html

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,46 +27,57 @@
2727
<script>
2828
function openDialog(dialog,modal) {
2929
assert_false(dialog.open);
30+
assert_false(dialog.matches(':open'));
3031
if (modal) {
3132
dialog.showModal();
3233
} else {
3334
dialog.show();
3435
}
3536
assert_true(dialog.open);
37+
assert_true(dialog.matches(':open'));
3638
assert_equals(dialog.matches(':modal'),modal);
3739
}
3840
function runTest(dialog) {
3941
for(modal of [false,true]) {
4042
promise_test(async (t) => {
4143
assert_false(dialog.open);
44+
assert_false(dialog.matches(':open'));
4245
t.add_cleanup(() => dialog.close());
4346
// Try hitting ESC
4447
openDialog(dialog,modal);
4548
const closedByReflectionWhileOpen = dialog.closedBy;
4649
const ESC = '\uE00C';
4750
await new test_driver.send_keys(document.documentElement,ESC);
4851
const respondsToEsc = !dialog.open;
52+
const respondsToEsc2 = !dialog.matches(':open');
4953
dialog.close();
5054
// Try clicking outside
5155
openDialog(dialog,modal);
5256
await clickOn(outside);
5357
const respondsToLightDismiss = !dialog.open;
58+
const respondsToLightDismiss2 = !dialog.matches(':open');
5459
dialog.close();
5560
// See if expectations match
5661
let expectedReflectionWhileOpen = dialog.dataset.behavior;
5762
let expectedReflectionWhileClosed = dialog.dataset.behavior;
5863
switch (dialog.dataset.behavior) {
5964
case 'any':
6065
assert_true(respondsToEsc,'Dialog should respond to ESC');
66+
assert_true(respondsToEsc2,'Dialog should respond to ESC (:open)');
6167
assert_true(respondsToLightDismiss,'Dialog should respond to light dismiss');
68+
assert_true(respondsToLightDismiss2,'Dialog should respond to light dismiss (:open)');
6269
break;
6370
case 'closerequest':
6471
assert_true(respondsToEsc,'Dialog should respond to ESC');
72+
assert_true(respondsToEsc2,'Dialog should respond to ESC (:open)');
6573
assert_false(respondsToLightDismiss,'Dialog should NOT respond to light dismiss');
74+
assert_false(respondsToLightDismiss2,'Dialog should NOT respond to light dismiss (:open)');
6675
break;
6776
case 'none':
6877
assert_false(respondsToEsc,'Dialog should NOT respond to ESC');
78+
assert_false(respondsToEsc2,'Dialog should NOT respond to ESC (:open)');
6979
assert_false(respondsToLightDismiss,'Dialog should NOT respond to light dismiss');
80+
assert_false(respondsToLightDismiss2,'Dialog should NOT respond to light dismiss (:open)');
7081
break;
7182
case 'auto':
7283
if (modal) {

html/semantics/interactive-elements/the-dialog-element/dialog-requestclose.html

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,25 @@
1111

1212
<script>
1313
const dialog = document.querySelector('dialog');
14-
function openDialog(modal) {
14+
function openDialog(openMethod) {
1515
assert_false(dialog.open);
16-
if (modal) {
17-
dialog.showModal();
18-
} else {
19-
dialog.show();
16+
assert_false(dialog.matches(':open'));
17+
switch (openMethod) {
18+
case 'modeless':
19+
dialog.show();
20+
break;
21+
case 'modal':
22+
dialog.showModal();
23+
break;
24+
case 'open':
25+
dialog.open = true;
26+
break;
27+
default:
28+
assert_unreached('Unknown open method');
2029
}
2130
assert_true(dialog.open);
22-
assert_equals(dialog.matches(':modal'),modal);
31+
assert_true(dialog.matches(':open'));
32+
assert_equals(dialog.matches(':modal'),openMethod === 'modal');
2333
}
2434
function getSignal(t) {
2535
const controller = new AbortController();
@@ -43,25 +53,27 @@
4353
return getSignal(t);
4454
}
4555

46-
[false,true].forEach(modal => {
56+
['modeless','modal','open'].forEach(openMethod => {
4757
[null,'any','closedrequest','none'].forEach(closedby => {
48-
const testDescription = `for ${modal ? "modal" : "modeless"} dialog with closedby=${closedby}`;
58+
const testDescription = `for ${openMethod} dialog with closedby=${closedby}`;
4959
promise_test(async (t) => {
5060
await setup(t,closedby);
51-
openDialog(modal);
61+
openDialog(openMethod);
5262
dialog.requestClose();
5363
assert_false(dialog.open);
64+
assert_false(dialog.matches(':open'));
5465
},`requestClose basic behavior ${testDescription}`);
5566

5667
promise_test(async (t) => {
5768
const signal = await setup(t,closedby);
5869
let events = [];
5970
dialog.addEventListener('cancel',() => events.push('cancel'),{signal});
6071
dialog.addEventListener('close',() => events.push('close'),{signal});
61-
openDialog(modal);
72+
openDialog(openMethod);
6273
assert_array_equals(events,[]);
6374
dialog.requestClose();
6475
assert_false(dialog.open);
76+
assert_false(dialog.matches(':open'));
6577
assert_array_equals(events,['cancel'],'close is scheduled');
6678
await new Promise(resolve => requestAnimationFrame(resolve));
6779
assert_array_equals(events,['cancel','close']);
@@ -72,14 +84,15 @@
7284
let events = [];
7385
dialog.addEventListener('cancel',() => events.push('cancel'),{signal});
7486
dialog.addEventListener('close',() => events.push('close'),{signal});
75-
openDialog(modal);
87+
openDialog(openMethod);
7688
dialog.setAttribute('closedby',closedby);
7789
assert_array_equals(events,[]);
7890
dialog.requestClose();
7991
assert_false(dialog.open,'Adding closedby after dialog is open');
92+
assert_false(dialog.matches(':open'));
8093
assert_array_equals(events,['cancel']);
8194
events=[];
82-
openDialog(modal);
95+
openDialog(openMethod);
8396
dialog.removeAttribute('closedby');
8497
assert_array_equals(events,[]);
8598
dialog.requestClose();
@@ -96,32 +109,36 @@
96109
e.preventDefault();
97110
}
98111
},{signal});
99-
openDialog(modal);
112+
openDialog(openMethod);
100113
dialog.requestClose();
101114
assert_true(dialog.open,'cancel event was cancelled - dialog shouldn\'t close');
115+
assert_true(dialog.matches(':open'));
102116
shouldPreventDefault = false;
103117
dialog.requestClose();
104118
assert_false(dialog.open,'cancel event was not cancelled - dialog should now close');
119+
assert_false(dialog.matches(':open'));
105120
},`requestClose can be cancelled ${testDescription}`);
106121

107122
promise_test(async (t) => {
108123
const signal = await setup(t,closedby);
109124
dialog.addEventListener('cancel',(e) => e.preventDefault(),{signal});
110-
openDialog(modal);
125+
openDialog(openMethod);
111126
// No user activation here.
112127
dialog.requestClose();
113128
dialog.requestClose();
114129
dialog.requestClose();
115130
assert_true(dialog.open,'cancel event was cancelled - dialog shouldn\'t close');
131+
assert_true(dialog.matches(':open'));
116132
},`requestClose avoids abuse prevention logic ${testDescription}`);
117133

118134
promise_test(async (t) => {
119135
await setup(t,closedby);
120-
openDialog(modal);
136+
openDialog(openMethod);
121137
assert_equals(dialog.returnValue,'','Return value starts out empty');
122138
const returnValue = 'The return value';
123139
dialog.requestClose(returnValue);
124140
assert_false(dialog.open);
141+
assert_false(dialog.matches(':open'));
125142
assert_equals(dialog.returnValue,returnValue,'Return value should be set');
126143
dialog.show();
127144
dialog.close();
@@ -134,11 +151,12 @@
134151
promise_test(async (t) => {
135152
await setup(t,closedby);
136153
dialog.addEventListener('cancel',(e) => e.preventDefault(),{once:true});
137-
openDialog(modal);
154+
openDialog(openMethod);
138155
dialog.returnValue = 'foo';
139156
assert_equals(dialog.returnValue,'foo');
140157
dialog.requestClose('This should not get saved');
141158
assert_true(dialog.open,'cancelled');
159+
assert_true(dialog.matches(':open'));
142160
assert_equals(dialog.returnValue,'foo','Return value should not be changed');
143161
},`requestClose(returnValue) doesn't change returnvalue when cancelled ${testDescription}`);
144162
}

0 commit comments

Comments
 (0)