Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't throw when popovers and dialogs are in requested state #39781

Merged
merged 1 commit into from
May 16, 2023
Merged

Don't throw when popovers and dialogs are in requested state #39781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<link rel=author href="mailto:[email protected]">
<link rel=help href="https://github.com/whatwg/html/pull/9142">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<dialog>hello</dialog>

<script>
test(() => {
const dialog = document.querySelector('dialog');

// calling close() on a dialog that is already closed should not throw.
dialog.close();

dialog.show();
// calling show() on a dialog that is already showing non-modal should not throw.
dialog.show();
assert_throws_dom('InvalidStateError', () => dialog.showModal(),
'Calling showModal() on a dialog that is already showing non-modal should throw.');
dialog.close();

dialog.showModal();
assert_throws_dom('InvalidStateError', () => dialog.show(),
'Calling show() on a dialog that is already showing modal should throw.');
// calling showModal() on a dialog that is already showing modal should not throw.
dialog.showModal();
});
</script>
16 changes: 13 additions & 3 deletions html/semantics/popovers/popover-attribute-basic.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,21 @@
},{once: true});
assert_true(popover.matches(':popover-open'));
assert_true(other_popover.matches(':popover-open'));
assert_throws_dom('InvalidStateError', () => popover.hidePopover());
popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
assert_false(other_popover.matches(':popover-open'),'unrelated popover is hidden');
assert_false(popover.matches(':popover-open'),'popover is still hidden if its type changed during hide event');
assert_throws_dom("InvalidStateError",() => other_popover.hidePopover(),'Nested popover should already be hidden');
},`Changing the popover type in a "beforetoggle" event handler should throw an exception (during hidePopover())`);
other_popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
},`Changing the popover type in a "beforetoggle" event handler during hidePopover() should not throw an exception`);

test(t => {
const popover = document.createElement('div');
assert_throws_dom('NotSupportedError', () => popover.hidePopover(),
'Calling hidePopover on an element without a popover attribute should throw.');
popover.setAttribute('popover', 'auto');
popover.hidePopover(); // Calling hidePopover on a disconnected popover should not throw.
assert_throws_dom('InvalidStateError', () => popover.showPopover(),
'Calling showPopover on a disconnected popover should throw.');
},'Calling hidePopover on a disconnected popover should not throw.');

function interpretedType(typeString,method) {
if (validTypes.includes(typeString))
Expand Down
7 changes: 2 additions & 5 deletions html/semantics/popovers/popover-light-dismiss.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@
p14.hidePopover();
},{once:true});
assert_true(p13.matches(':popover-open') && p14.matches(':popover-open') && p15.matches(':popover-open'),'all three should be open');
assert_throws_dom('InvalidStateError',() => p14.hidePopover(),'should throw because the event listener has already hidden the popover');
p14.hidePopover();
assert_true(p13.matches(':popover-open'),'p13 should still be open');
assert_false(p14.matches(':popover-open'));
assert_false(p15.matches(':popover-open'));
Expand Down Expand Up @@ -579,10 +579,7 @@
p20.showPopover();
});
p20.addEventListener('beforetoggle', logEvents);
// Because the `beforetoggle` handler shows a different popover,
// and that action closes the p19 popover, the call to hidePopover()
// will result in an exception.
assert_throws_dom('InvalidStateError',() => p19.hidePopover());
p19.hidePopover();
assert_array_equals(events,['hide p19','show p20'],'There should not be a second hide event for 19');
assert_false(p19.matches(':popover-open'));
assert_true(p20.matches(':popover-open'));
Expand Down
33 changes: 26 additions & 7 deletions html/semantics/popovers/popover-move-documents.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,30 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<script>
async function iframeLoaded(iframe) {
return new Promise(resolve => {
if (iframe.contentWindow.document.readyState == 'complete') {
resolve();
} else {
iframe.onload = resolve;
}
});
}
</script>

<iframe id=myframe srcdoc="<p>iframe</p>"></iframe>
<div id=p1 popover=auto>p1</div>
<script>
test(() => {
promise_test(async () => {
await iframeLoaded(myframe);
await new Promise(resolve => {
if (myframe.contentWindow.document.readyState == 'complete') {
resolve();
} else {

}
});
p1.addEventListener('beforetoggle', () => {
myframe.contentWindow.document.body.appendChild(p1);
});
Expand All @@ -18,7 +38,8 @@
<iframe id=myframe2 srcdoc="<p>iframe</p>"></iframe>
<div id=p2 popover=auto>p2</div>
<script>
test(() => {
promise_test(async () => {
await iframeLoaded(myframe2);
const p2 = document.getElementById('p2');
p2.showPopover();
p2.addEventListener('beforetoggle', () => {
Expand All @@ -27,10 +48,7 @@
assert_true(p2.matches(':popover-open'),
'The popover should be open after calling showPopover()');

// Because the `beforetoggle` handler changes the document,
// and that action closes the popover, the call to hidePopover()
// will result in an exception.
assert_throws_dom('InvalidStateError',() => p2.hidePopover());
p2.hidePopover();
assert_false(p2.matches(':popover-open'),
'The popover should be closed after moving it between documents.');
}, 'Moving popovers between documents while hiding should not throw an exception.');
Expand All @@ -43,7 +61,8 @@
<div id=p5 popover=auto>p5</div>
</div>
<script>
test(() => {
promise_test(async () => {
await iframeLoaded(myframe3);
p3.showPopover();
p4.showPopover();
p4.addEventListener('beforetoggle', event => {
Expand Down
6 changes: 3 additions & 3 deletions html/semantics/popovers/resources/popover-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,10 @@ function assertIsFunctionalPopover(popover, checkVisibility) {
assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/false, 'A popover should start out hidden');
popover.showPopover();
if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/true, 'After showPopover(), a popover should be visible');
assert_throws_dom("InvalidStateError",() => popover.showPopover(),'Calling showPopover on a showing popover should throw InvalidStateError');
popover.showPopover(); // Calling showPopover on a showing popover should not throw.
popover.hidePopover();
if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/false, 'After hidePopover(), a popover should be hidden');
assert_throws_dom("InvalidStateError",() => popover.hidePopover(),'Calling hidePopover on a hidden popover should throw InvalidStateError');
popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
popover.togglePopover();
if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/true, 'After togglePopover() on hidden popover, it should be visible');
popover.togglePopover();
Expand All @@ -172,7 +172,7 @@ function assertIsFunctionalPopover(popover, checkVisibility) {
const parent = popover.parentElement;
popover.remove();
assert_throws_dom("InvalidStateError",() => popover.showPopover(),'Calling showPopover on a disconnected popover should throw InvalidStateError');
assert_throws_dom("InvalidStateError",() => popover.hidePopover(),'Calling hidePopover on a disconnected popover should throw InvalidStateError');
popover.hidePopover(); // Calling hidePopover on a disconnected popover should not throw.
assert_throws_dom("InvalidStateError",() => popover.togglePopover(),'Calling hidePopover on a disconnected popover should throw InvalidStateError');
parent.appendChild(popover);
}
Expand Down