Skip to content

Commit d5abb3a

Browse files
josepharharchromium-wpt-export-bot
josepharhar
authored and
chromium-wpt-export-bot
committed
Don't throw when popovers and dialogs are in requested state
This is being changed in the HTML spec here: whatwg/html#9142 Change-Id: Ib8aaaf314c2a1de5d082494e5172e029d531c8e8
1 parent 7053aa2 commit d5abb3a

File tree

5 files changed

+71
-18
lines changed

5 files changed

+71
-18
lines changed

‎html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!DOCTYPE html>
2+
<link rel=author href="mailto:[email protected]">
3+
<link rel=help href="https://github.com/whatwg/html/pull/9142">
4+
<script src="/resources/testharness.js"></script>
5+
<script src="/resources/testharnessreport.js"></script>
6+
7+
<dialog>hello</dialog>
8+
9+
<script>
10+
test(() => {
11+
const dialog = document.querySelector('dialog');
12+
13+
// calling close() on a dialog that is already closed should not throw.
14+
dialog.close();
15+
16+
dialog.show();
17+
// calling show() on a dialog that is already showing non-modal should not throw.
18+
dialog.show();
19+
assert_throws_dom('InvalidStateError', () => dialog.showModal(),
20+
'Calling showModal() on a dialog that is already showing non-modal should throw.');
21+
dialog.close();
22+
23+
dialog.showModal();
24+
assert_throws_dom('InvalidStateError', () => dialog.show(),
25+
'Calling show() on a dialog that is already showing modal should throw.');
26+
// calling showModal() on a dialog that is already showing modal should not throw.
27+
dialog.showModal();
28+
});
29+
</script>

‎html/semantics/popovers/popover-attribute-basic.html

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,19 @@
254254
},{once: true});
255255
assert_true(popover.matches(':popover-open'));
256256
assert_true(other_popover.matches(':popover-open'));
257-
assert_throws_dom('InvalidStateError', () => popover.hidePopover());
257+
popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
258258
assert_false(other_popover.matches(':popover-open'),'unrelated popover is hidden');
259259
assert_false(popover.matches(':popover-open'),'popover is still hidden if its type changed during hide event');
260-
assert_throws_dom("InvalidStateError",() => other_popover.hidePopover(),'Nested popover should already be hidden');
261-
},`Changing the popover type in a "beforetoggle" event handler should throw an exception (during hidePopover())`);
260+
other_popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
261+
},`Changing the popover type in a "beforetoggle" event handler during hidePopover() should not throw an exception`);
262+
263+
test(t => {
264+
const popover = document.createElement('div');
265+
assert_throws_dom('NotSupportedError', () => popover.hidePopover(),
266+
'Calling hidePopover on an element without a popover attribute should throw.');
267+
popover.setAttribute('popover', 'auto');
268+
popover.hidePopover(); // Calling hidePopover on a disconnected popover should not throw.
269+
},'Calling hidePopover on a disconnected popover should not throw.');
262270

263271
function interpretedType(typeString,method) {
264272
if (validTypes.includes(typeString))

‎html/semantics/popovers/popover-light-dismiss.html

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@
532532
p14.hidePopover();
533533
},{once:true});
534534
assert_true(p13.matches(':popover-open') && p14.matches(':popover-open') && p15.matches(':popover-open'),'all three should be open');
535-
assert_throws_dom('InvalidStateError',() => p14.hidePopover(),'should throw because the event listener has already hidden the popover');
535+
p14.hidePopover();
536536
assert_true(p13.matches(':popover-open'),'p13 should still be open');
537537
assert_false(p14.matches(':popover-open'));
538538
assert_false(p15.matches(':popover-open'));
@@ -579,10 +579,7 @@
579579
p20.showPopover();
580580
});
581581
p20.addEventListener('beforetoggle', logEvents);
582-
// Because the `beforetoggle` handler shows a different popover,
583-
// and that action closes the p19 popover, the call to hidePopover()
584-
// will result in an exception.
585-
assert_throws_dom('InvalidStateError',() => p19.hidePopover());
582+
p19.hidePopover();
586583
assert_array_equals(events,['hide p19','show p20'],'There should not be a second hide event for 19');
587584
assert_false(p19.matches(':popover-open'));
588585
assert_true(p20.matches(':popover-open'));

‎html/semantics/popovers/popover-move-documents.html

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,30 @@
44
<script src="/resources/testharness.js"></script>
55
<script src="/resources/testharnessreport.js"></script>
66

7+
<script>
8+
async function iframeLoaded(iframe) {
9+
return new Promise(resolve => {
10+
if (iframe.contentWindow.document.readyState == 'complete') {
11+
resolve();
12+
} else {
13+
iframe.onload = resolve;
14+
}
15+
});
16+
}
17+
</script>
18+
719
<iframe id=myframe srcdoc="<p>iframe</p>"></iframe>
820
<div id=p1 popover=auto>p1</div>
921
<script>
10-
test(() => {
22+
promise_test(async () => {
23+
await iframeLoaded(myframe);
24+
await new Promise(resolve => {
25+
if (myframe.contentWindow.document.readyState == 'complete') {
26+
resolve();
27+
} else {
28+
29+
}
30+
});
1131
p1.addEventListener('beforetoggle', () => {
1232
myframe.contentWindow.document.body.appendChild(p1);
1333
});
@@ -18,7 +38,8 @@
1838
<iframe id=myframe2 srcdoc="<p>iframe</p>"></iframe>
1939
<div id=p2 popover=auto>p2</div>
2040
<script>
21-
test(() => {
41+
promise_test(async () => {
42+
await iframeLoaded(myframe2);
2243
const p2 = document.getElementById('p2');
2344
p2.showPopover();
2445
p2.addEventListener('beforetoggle', () => {
@@ -27,10 +48,7 @@
2748
assert_true(p2.matches(':popover-open'),
2849
'The popover should be open after calling showPopover()');
2950

30-
// Because the `beforetoggle` handler changes the document,
31-
// and that action closes the popover, the call to hidePopover()
32-
// will result in an exception.
33-
assert_throws_dom('InvalidStateError',() => p2.hidePopover());
51+
p2.hidePopover();
3452
assert_false(p2.matches(':popover-open'),
3553
'The popover should be closed after moving it between documents.');
3654
}, 'Moving popovers between documents while hiding should not throw an exception.');
@@ -43,7 +61,8 @@
4361
<div id=p5 popover=auto>p5</div>
4462
</div>
4563
<script>
46-
test(() => {
64+
promise_test(async () => {
65+
await iframeLoaded(myframe3);
4766
p3.showPopover();
4867
p4.showPopover();
4968
p4.addEventListener('beforetoggle', event => {

‎html/semantics/popovers/resources/popover-utils.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,10 @@ function assertIsFunctionalPopover(popover, checkVisibility) {
153153
assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/false, 'A popover should start out hidden');
154154
popover.showPopover();
155155
if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/true, 'After showPopover(), a popover should be visible');
156-
assert_throws_dom("InvalidStateError",() => popover.showPopover(),'Calling showPopover on a showing popover should throw InvalidStateError');
156+
popover.showPopover(); // Calling showPopover on a showing popover should not throw.
157157
popover.hidePopover();
158158
if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/false, 'After hidePopover(), a popover should be hidden');
159-
assert_throws_dom("InvalidStateError",() => popover.hidePopover(),'Calling hidePopover on a hidden popover should throw InvalidStateError');
159+
popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
160160
popover.togglePopover();
161161
if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/true, 'After togglePopover() on hidden popover, it should be visible');
162162
popover.togglePopover();
@@ -172,7 +172,7 @@ function assertIsFunctionalPopover(popover, checkVisibility) {
172172
const parent = popover.parentElement;
173173
popover.remove();
174174
assert_throws_dom("InvalidStateError",() => popover.showPopover(),'Calling showPopover on a disconnected popover should throw InvalidStateError');
175-
assert_throws_dom("InvalidStateError",() => popover.hidePopover(),'Calling hidePopover on a disconnected popover should throw InvalidStateError');
175+
popover.hidePopover(); // Calling hidePopover on a disconnected popover should not throw.
176176
assert_throws_dom("InvalidStateError",() => popover.togglePopover(),'Calling hidePopover on a disconnected popover should throw InvalidStateError');
177177
parent.appendChild(popover);
178178
}

0 commit comments

Comments
 (0)