Skip to content

Commit

Permalink
Fix dialog.requestClose() in two ways
Browse files Browse the repository at this point in the history
1. The prior implementation ignored the argument to requestClose.
   With this CL, requestClose(string) stores away string and then
   returns it from the CloseWatcher close event.
2. If the dialog's closedBy state is None (either explicitly or
   through the "Auto" state), requestClose throws an exception.

I also added a more significant set of tests for requestClose() in
all states.

This goes along with recent discussion about throwing exceptions if
requestClose() is called when the dialog isn't in the right state,
relative to closedBy:

  whatwg/html#10164 (comment)

The spec PR has been updated accordingly:

  whatwg/html#10737

Bug: 376516550
Change-Id: I023845844e6afb4da9a71637d517ac78d2861329
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026242
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: David Baron <[email protected]>
Commit-Queue: David Baron <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1384345}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Nov 18, 2024
1 parent 6572d5b commit 6f4f09f
Showing 1 changed file with 124 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,31 +41,132 @@
assert_false(dialog.open,'Without user activation, requestClose can\'t be cancelled');
},`requestClose requires user activation in order to be cancelable`);

async function setup(t,closedby) {
t.add_cleanup(() => {
dialog.close();
dialog.removeAttribute('closedby');
dialog.returnValue = '';
});
assert_false(dialog.hasAttribute('closedby'));
if (closedby) {
dialog.setAttribute('closedby',closedby);
}
// Be sure any pending close events get fired.
await new Promise(resolve => requestAnimationFrame(resolve));
return getSignal(t);
}

[false,true].forEach(modal => {
promise_test(async (t) => {
t.add_cleanup(() => dialog.close());
openDialog(modal);
dialog.requestClose();
assert_false(dialog.open);
},`${modal ? "Modal:" : "Non-modal:"} requestClose closes the dialog`);
[null,'any','closedrequest','none'].forEach(closedby => {
const testDescription = `for ${modal ? "modal" : "modeless"} dialog with closedby=${closedby}`;
promise_test(async (t) => {
await setup(t,closedby);
openDialog(modal);
if (dialog.closedBy != "none") {
dialog.requestClose();
assert_false(dialog.open);
} else {
assert_throws_dom('InvalidStateError',() => dialog.requestClose());
assert_true(dialog.open);
}
},`requestClose basic behavior ${testDescription}`);

promise_test(async (t) => {
const signal = await setup(t,closedby);
let events = [];
dialog.addEventListener('cancel',() => events.push('cancel'),{signal});
dialog.addEventListener('close',() => events.push('close'),{signal});
openDialog(modal);
assert_array_equals(events,[]);
if (dialog.closedBy != "none") {
dialog.requestClose();
assert_false(dialog.open);
assert_array_equals(events,['cancel'],'close is scheduled');
await new Promise(resolve => requestAnimationFrame(resolve));
assert_array_equals(events,['cancel','close']);
} else {
assert_throws_dom('InvalidStateError',() => dialog.requestClose());
}
},`requestClose fires both cancel and close ${testDescription}`);

promise_test(async (t) => {
t.add_cleanup(() => dialog.close());
const signal = getSignal(t);
let shouldPreventDefault = true;
dialog.addEventListener('cancel',(e) => {
if (shouldPreventDefault) {
e.preventDefault();
promise_test(async (t) => {
const signal = await setup(t,'none');
let events = [];
dialog.addEventListener('cancel',() => events.push('cancel'),{signal});
dialog.addEventListener('close',() => events.push('close'),{signal});
openDialog(modal);
dialog.setAttribute('closedby',closedby);
assert_array_equals(events,[]);
if (dialog.closedBy != "none") {
dialog.requestClose();
assert_false(dialog.open);
} else {
assert_throws_dom('InvalidStateError',() => dialog.requestClose());
}
},{signal});
openDialog(modal);
await clickOn(dialog); // User activation
dialog.requestClose();
assert_true(dialog.open,'cancel event was cancelled - dialog shouldn\'t close');
shouldPreventDefault = false;
await clickOn(dialog); // User activation
dialog.requestClose();
assert_false(dialog.open,'cancel event was not cancelled - dialog should now close');
},`${modal ? "Modal:" : "Non-modal:"} requestClose can be cancelled`);
},`changing closedby from 'none' to '${closedby}' for ${modal ? "modal" : "modeless"} dialog`);

promise_test(async (t) => {
const signal = await setup(t,closedby);
let events = [];
dialog.addEventListener('cancel',() => events.push('cancel'),{signal});
dialog.addEventListener('close',() => events.push('close'),{signal});
openDialog(modal);
dialog.removeAttribute('closedby');
assert_array_equals(events,[]);
if (dialog.closedBy != "none") {
dialog.requestClose();
assert_false(dialog.open);
} else {
assert_throws_dom('InvalidStateError',() => dialog.requestClose());
}
},`Removing closedby when closedby='${closedby}' for ${modal ? "modal" : "modeless"} dialog`);

if (dialog.closedBy != "none") {
promise_test(async (t) => {
const signal = await setup(t,closedby);
let shouldPreventDefault = true;
dialog.addEventListener('cancel',(e) => {
if (shouldPreventDefault) {
e.preventDefault();
}
},{signal});
openDialog(modal);
await clickOn(dialog); // User activation
dialog.requestClose();
assert_true(dialog.open,'cancel event was cancelled - dialog shouldn\'t close');
shouldPreventDefault = false;
await clickOn(dialog); // User activation
dialog.requestClose();
assert_false(dialog.open,'cancel event was not cancelled - dialog should now close');
},`requestClose can be cancelled ${testDescription}`);

promise_test(async (t) => {
await setup(t,closedby);
openDialog(modal);
assert_equals(dialog.returnValue,'','Return value starts out empty');
const returnValue = 'The return value';
dialog.requestClose(returnValue);
assert_false(dialog.open);
assert_equals(dialog.returnValue,returnValue,'Return value should be set');
dialog.show();
dialog.close();
assert_equals(dialog.returnValue,returnValue,'Return value should not be changed by close()');
dialog.show();
dialog.close('another');
assert_equals(dialog.returnValue,'another','Return value changes via close(value)');
},`requestClose(returnValue) passes along the return value ${testDescription}`);

promise_test(async (t) => {
await setup(t,closedby);
dialog.addEventListener('cancel',(e) => e.preventDefault(),{once:true});
openDialog(modal);
dialog.returnValue = 'foo';
assert_equals(dialog.returnValue,'foo');
dialog.requestClose('This should not get saved');
assert_true(dialog.open,'cancelled');
assert_equals(dialog.returnValue,'foo','Return value should not be changed');
},`requestClose(returnValue) doesn't change returnvalue when cancelled ${testDescription}`);
}
});
});
</script>

0 comments on commit 6f4f09f

Please sign in to comment.