Skip to content

Commit

Permalink
Make dialog light dismiss and dialog.requestClose not need UA
Browse files Browse the repository at this point in the history
See the conversation here:

  openui/open-ui#1128 (comment)

Previously, both dialog light dismiss and dialog.requestClose() would be
subject to CloseWatcher's abuse prevention logic, which requires user
activation. But that doesn't make sense, as pointed out in the thread
above, since neither of those are "abusive" things - they're not
keeping a user from navigating back. So this CL adds a parameter
to CloseWatcher.requestClose() that allows callers to proceed with
requesting close even if there isn't sufficient user activation.
Note that this CL does not change the existing behavior for other
CloseWatchers - those will still require user activation for now.
See crbug.com/383593252 for that.

Bug: 376516550,383593252
Change-Id: Ibf5ccc835794aba2aa627ddec7caafa6c9c26e55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6085724
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1395143}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Dec 12, 2024
1 parent 92db821 commit 1b5c481
Showing 1 changed file with 17 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="../../popovers/resources/popover-utils.js"></script>

<dialog>Dialog</dialog>

Expand All @@ -29,18 +28,6 @@
return signal;
}

// Run this test first, since the user activation from other tests will persist.
promise_test(async (t) => {
t.add_cleanup(() => dialog.close());
const signal = getSignal(t);
dialog.addEventListener('cancel',(e) => {
e.preventDefault();
},{signal});
openDialog(/*modal*/true);
dialog.requestClose();
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();
Expand Down Expand Up @@ -89,20 +76,16 @@
dialog.setAttribute('closedby',closedby);
assert_array_equals(events,[]);
dialog.requestClose();
assert_false(dialog.open);
},`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});
assert_false(dialog.open,'Adding closedby after dialog is open');
assert_array_equals(events,['cancel']);
events=[];
openDialog(modal);
dialog.removeAttribute('closedby');
assert_array_equals(events,[]);
dialog.requestClose();
assert_false(dialog.open);
},`Removing closedby when closedby='${closedby}' for ${modal ? "modal" : "modeless"} dialog`);
assert_false(dialog.open,'Removing closedby after dialog is open');
assert_array_equals(events,['cancel']);
},`closedby has no effect on dialog.requestClose() ${testDescription}`);

if (dialog.closedBy != "none") {
promise_test(async (t) => {
Expand All @@ -114,15 +97,24 @@
}
},{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) => {
const signal = await setup(t,closedby);
dialog.addEventListener('cancel',(e) => e.preventDefault(),{signal});
openDialog(modal);
// No user activation here.
dialog.requestClose();
dialog.requestClose();
dialog.requestClose();
assert_true(dialog.open,'cancel event was cancelled - dialog shouldn\'t close');
},`requestClose avoids abuse prevention logic ${testDescription}`);

promise_test(async (t) => {
await setup(t,closedby);
openDialog(modal);
Expand Down

0 comments on commit 1b5c481

Please sign in to comment.