Skip to content

Commit

Permalink
Disallow "combined" top layer APIs
Browse files Browse the repository at this point in the history
As resolved in [1], the top layer APIs, when used on the *same element*,
should disallow the second API usage. For example, with an element like
`<dialog popover>`, this can get to the top layer two ways:
  1) `dialog.showModal()`
  2) `dialog.showPopover()`
With this CL, the first such call will succeed, and the second will
throw an exception.

[1] openui/open-ui#520 (comment)

Bug: 1307772
Change-Id: If968211a3c69e72b9e734cdff93b8cf96e01860c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4017895
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1070452}
  • Loading branch information
Mason Freed authored and chromium-wpt-export-bot committed Nov 11, 2022
1 parent c82e314 commit dbb2c32
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 14 deletions.
124 changes: 124 additions & 0 deletions html/semantics/popovers/popover-top-layer-combinations.tentative.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>Popover combined with dialog/fullscreen behavior</title>
<link rel=author href="mailto:[email protected]">
<link rel=help href="https://open-ui.org/components/popup.research.explainer">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/popover-utils.js"></script>

<div id=examples>
<dialog popover>Popover Dialog</dialog>
<dialog popover open style="top:50px;">Open Non-modal Popover Dialog</dialog>
<dialog popover=manual defaultopen style="top:100px;">Defaultopen Popover Dialog</dialog>
<dialog popover=manual defaultopen open style="top:150px;">Defaultopen and Open Popover Dialog</dialog>
<div popover class=fullscreen>Fullscreen Popover</div>
<dialog popover class=fullscreen>Fullscreen Popover Dialog</dialog>
<dialog popover open class=fullscreen style="top:200px;">Fullscreen Open Non-modal Popover Dialog</dialog>
</div>
<button id=visible>Visible button</button>

<style>
[popover] {
inset:auto;
top:0;
left:0;
}
</style>

<script>
const isDialog = (ex) => ex instanceof HTMLDialogElement;
const isFullscreen = (ex) => ex.classList.contains('fullscreen');
function ensureIsOpenPopover(ex,message) {
// Because :open will eventually support <dialog>, this does extra work to
// verify we're dealing with an :open Popover.
message = message || 'Error';
assert_true(ex.matches(':open'),`${message}: Popover doesn\'t match :open`);
assert_false(ex.matches(':closed'),`${message}: Popover matches :closed`);
ex.hidePopover(); // Shouldn't throw if this is a showing popover
ex.showPopover(); // Show it again to avoid state change
assert_true(ex.matches(':open') && !ex.matches(':closed'),`${message}: Sanity`);
}
window.onload = () => requestAnimationFrame(() => requestAnimationFrame(() => {
const examples = Array.from(document.querySelectorAll('#examples>*'));
examples.forEach(ex => {
promise_test(async (t) => {
t.add_cleanup(() => ex.remove());
if (ex.hasAttribute('open')) {
assert_true(isDialog(ex));
assert_true(isElementVisible(ex),'Open dialog should be visible by default');
} else if (ex.hasAttribute('defaultopen')) {
ensureIsOpenPopover(ex,'defaultopen should open the popover on load');
assert_true(isElementVisible(ex),'Popover with defaultopen should be visible by default');
}
if (isElementVisible(ex)) {
// ex is already open (defaultopen or open dialog)
assert_throws_dom("InvalidStateError",() => ex.showPopover(),'Calling showPopover on an already-showing element should throw InvalidStateError');
if (ex.hasAttribute('open')) {
assert_true(isDialog(ex));
ex.removeAttribute('open');
assert_false(isElementVisible(ex),'Removing the open attribute should hide the dialog');
} else {
ex.hidePopover(); // Should not throw
}
} else {
ex.showPopover(); // Should not throw
ensureIsOpenPopover(ex,'showPopover should work');
ex.hidePopover(); // Should not throw
assert_true(ex.matches(':closed'),'hidePopover should work');
}
assert_false(isElementVisible(ex));

// Start with popover, try the other API
ex.showPopover();
let tested_something=false;
if (isDialog(ex)) {
tested_something=true;
ensureIsOpenPopover(ex);
assert_throws_dom("InvalidStateError",() => ex.showModal(),'Calling showModal() on an already-showing Popover should throw InvalidStateError');
assert_throws_dom("InvalidStateError",() => ex.show(),'Calling show() on an already-showing Popover should throw InvalidStateError');
}
if (isFullscreen(ex)) {
tested_something=true;
let requestSucceeded = false;
await blessTopLayer(ex)
.then(() => ex.requestFullscreen())
.then(() => {requestSucceeded = true;}) // We should not hit this.
.catch((exception) => {
// This exception is expected.
assert_equals(exception.name,'TypeError',`Invalid exception from requestFullscreen() (${exception.message})`);
});
assert_false(requestSucceeded,'requestFullscreen() should not succeed when the element is an already-showing Popover');
}
assert_true(tested_something);
ensureIsOpenPopover(ex);
ex.hidePopover();

// Start with the other API, then try popover
if (isDialog(ex)) {
ex.show();
assert_true(ex.hasAttribute('open'));
assert_throws_dom("InvalidStateError",() => ex.showPopover(),'Calling showPopover() on an already-showing non-modal dialog should throw InvalidStateError');
ex.close();
assert_false(ex.hasAttribute('open'));
ex.showModal();
assert_true(ex.hasAttribute('open'));
assert_throws_dom("InvalidStateError",() => ex.showPopover(),'Calling showPopover() on an already-showing modal dialog should throw InvalidStateError');
ex.close();
assert_false(ex.hasAttribute('open'));
} else if (isFullscreen(ex)) {
let requestSucceeded = false;
await blessTopLayer(visible)
.then(() => ex.requestFullscreen())
.then(() => {
assert_throws_dom("InvalidStateError",() => ex.showPopover(),'Calling showPopover() on an already-fullscreen element should throw InvalidStateError');
});
await document.exitFullscreen()
.then(() => assert_true(true));
}
}, `Popover combination: ${ex.textContent}`);
});
}));
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/popover-utils.js"></script>

<body>
<script>
Expand Down Expand Up @@ -36,7 +37,7 @@
type: types.fullscreen,
closes: [types.popover, types.fullscreen],
createElement: () => document.createElement('div'),
trigger: async function(visibleElement) {assert_false(this.isTopLayer());await bless(visibleElement);await this.element.requestFullscreen();},
trigger: async function(visibleElement) {assert_false(this.isTopLayer());await blessTopLayer(visibleElement);await this.element.requestFullscreen();},
close: function() {assert_equals(this.element,document.fullscreenElement); document.exitFullscreen();},
isTopLayer: function() {return this.element.matches(':fullscreen');},
},
Expand All @@ -58,19 +59,6 @@
ex.element.remove();
ex.element = null;
}
async function bless(visibleElement) {
// The normal "bless" function doesn't work well when there are top layer
// elements blocking clicks. Additionally, since the normal test_driver.bless
// function just adds a button to the main document and clicks it, we can't
// call that in the presence of open popovers, since that click will close them.
const button = document.createElement('button');
button.innerHTML = "Click me to activate";
visibleElement.appendChild(button);
let wait_click = new Promise(resolve => button.addEventListener("click", resolve, {once: true}));
await test_driver.click(button);
await wait_click;
button.remove();
}
// Test interactions between top layer elements
for(let i=0;i<examples.length;++i) {
for(let j=0;j<examples.length;++j) {
Expand Down
13 changes: 13 additions & 0 deletions html/semantics/popovers/resources/popover-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,16 @@ async function waitForHoverTime(hoverWaitTimeMs) {
await new Promise(resolve => step_timeout(resolve,hoverWaitTimeMs));
await waitForRender();
};
async function blessTopLayer(visibleElement) {
// The normal "bless" function doesn't work well when there are top layer
// elements blocking clicks. Additionally, since the normal test_driver.bless
// function just adds a button to the main document and clicks it, we can't
// call that in the presence of open popovers, since that click will close them.
const button = document.createElement('button');
button.innerHTML = "Click me to activate";
visibleElement.appendChild(button);
let wait_click = new Promise(resolve => button.addEventListener("click", resolve, {once: true}));
await test_driver.click(button);
await wait_click;
button.remove();
}

0 comments on commit dbb2c32

Please sign in to comment.