From a8a4f241414cdabb1854db1aef3493446bbddddf Mon Sep 17 00:00:00 2001 From: Mason Freed Date: Wed, 31 Jan 2024 08:16:53 -0800 Subject: [PATCH] Allow top layer elements to be nested within popovers See the issue raised at https://github.com/whatwg/html/issues/9998 which was discussed at https://github.com/whatwg/html/issues/9993 This CL makes the following changes: 1. Change `FindTopmostPopoverAncestor()` so that the provided element does not have to be a popover. The logic does not materially change - all of the same mechanisms can be used to connect a non-popover top layer element (dialog or fullscreen) to an ancestor popover. 2. Add a utility `TopLayerElementPopoverAncestor()` which finds the popover ancestor for a provided top layer element by calling `FindTopmostPopoverAncestor()` with the proper arguments. 3. In dialog and fullscreen code, where it previously called `HideAllPopoversUntil(nullptr,...)` to hide **all** open popovers, it now (with flag enabled) hides only up to the nearest popover ancestor. Tests are added, which are marked `.tentative`. Bug: 1520938 Change-Id: I8d2c4000ed3959ac4e8bf521e22e9dfd532c62d5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5229300 Auto-Submit: Mason Freed Commit-Queue: Joey Arhar Reviewed-by: Joey Arhar Cr-Commit-Position: refs/heads/main@{#1254541} --- .../popovers/popover-attribute-basic.html | 1 + ...er-top-layer-nesting-anchor.tentative.html | 45 ++++++++ ...ver-top-layer-nesting-hints.tentative.html | 46 ++++++++ .../popover-top-layer-nesting.tentative.html | 45 ++++++++ .../resources/popover-top-layer-nesting.js | 108 ++++++++++++++++++ 5 files changed, 245 insertions(+) create mode 100644 html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html create mode 100644 html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html create mode 100644 html/semantics/popovers/popover-top-layer-nesting.tentative.html create mode 100644 html/semantics/popovers/resources/popover-top-layer-nesting.js diff --git a/html/semantics/popovers/popover-attribute-basic.html b/html/semantics/popovers/popover-attribute-basic.html index 13108949cb3c5e..2af3bbc137f1a0 100644 --- a/html/semantics/popovers/popover-attribute-basic.html +++ b/html/semantics/popovers/popover-attribute-basic.html @@ -1,6 +1,7 @@ + diff --git a/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html b/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html new file mode 100644 index 00000000000000..4520ab05770404 --- /dev/null +++ b/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html @@ -0,0 +1,45 @@ + + + + + + + + + + + + + +
+
Single popover=auto ancestor +
+
+ +
Single popover=manual ancestor +
+
+ +
Nested popover=auto ancestors +
+
+
+
+ +
Nested popover=auto ancestors, target is outer +
+
+
+
+ +
Top layer inside of nested element +
+ +
+
+
+ + diff --git a/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html b/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html new file mode 100644 index 00000000000000..4ec1f49bda9fbd --- /dev/null +++ b/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html @@ -0,0 +1,46 @@ + + + + + + + + + + + + + + +
+
Single popover=hint ancestor +
+
+ +
Nested auto/hint ancestors +
+
+
+
+ +
Nested auto/hint ancestors, target is auto +
+
+
+
+ +
Unrelated hint, target=hint +
+
+
+ +
Unrelated hint, target=auto +
+
+
+
+ + diff --git a/html/semantics/popovers/popover-top-layer-nesting.tentative.html b/html/semantics/popovers/popover-top-layer-nesting.tentative.html new file mode 100644 index 00000000000000..a0b3b60b72b554 --- /dev/null +++ b/html/semantics/popovers/popover-top-layer-nesting.tentative.html @@ -0,0 +1,45 @@ + + + + + + + + + + + + + +
+
Single popover=auto ancestor +
+
+ +
Single popover=manual ancestor +
+
+ +
Nested popover=auto ancestors +
+
+
+
+ +
Nested popover=auto ancestors, target is outer +
+
+
+
+ +
Top layer inside of nested element +
+ +
+
+
+ + diff --git a/html/semantics/popovers/resources/popover-top-layer-nesting.js b/html/semantics/popovers/resources/popover-top-layer-nesting.js new file mode 100644 index 00000000000000..ace10b3f7bc47f --- /dev/null +++ b/html/semantics/popovers/resources/popover-top-layer-nesting.js @@ -0,0 +1,108 @@ +function createTopLayerElement(t,topLayerType) { + let element, show, showing; + switch (topLayerType) { + case 'dialog': + element = document.createElement('dialog'); + show = () => element.showModal(); + showing = () => element.matches(':modal'); + break; + case 'fullscreen': + element = document.createElement('div'); + show = async (topmostElement) => { + // Be sure to add user activation to the topmost visible target: + await blessTopLayer(topmostElement); + await element.requestFullscreen(); + }; + showing = () => document.fullscreenElement === element; + break; + default: + assert_unreached('Invalid top layer type'); + } + t.add_cleanup(() => element.remove()); + return {element,show,showing}; +} +function runTopLayerTests(testCases, testAnchorAttribute) { + testAnchorAttribute = testAnchorAttribute || false; + testCases.forEach(test => { + const description = test.firstChild.data.trim(); + assert_equals(test.querySelectorAll('.target').length,1,'There should be exactly one target'); + const target = test.querySelector('.target'); + assert_true(!!target,'Invalid test case'); + const popovers = Array.from(test.querySelectorAll('[popover]')); + assert_true(popovers.length > 0,'No popovers found'); + ['dialog','fullscreen'].forEach(topLayerType => { + promise_test(async t => { + const {element,show,showing} = createTopLayerElement(t,topLayerType); + target.appendChild(element); + + // Show the popovers. + t.add_cleanup(() => popovers.forEach(popover => popover.hidePopover())); + popovers.forEach(popover => popover.showPopover()); + popovers.forEach(popover => assert_true(popover.matches(':popover-open'),'All popovers should be open')); + + // Activate the top layer element. + await show(popovers[popovers.length-1]); + assert_true(showing()); + popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Incorrect behavior')); + + // Add another popover within the top layer element and make sure entire stack stays open. + const newPopover = document.createElement('div'); + t.add_cleanup(() => newPopover.remove()); + newPopover.popover = popoverHintSupported() ? 'hint' : 'auto'; + element.appendChild(newPopover); + popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Adding another popover shouldn\'t change anything')); + assert_true(showing(),'top layer element should still be top layer'); + newPopover.showPopover(); + assert_true(newPopover.matches(':popover-open')); + popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Showing the popover shouldn\'t change anything')); + assert_true(showing(),'top layer element should still be top layer'); + },`${description} with ${topLayerType}`); + + promise_test(async t => { + const {element,show,showing} = createTopLayerElement(t,topLayerType); + element.popover = popoverHintSupported() ? 'hint' : 'auto'; + target.appendChild(element); + + // Show the popovers. + t.add_cleanup(() => popovers.forEach(popover => popover.hidePopover())); + popovers.forEach(popover => popover.showPopover()); + popovers.forEach(popover => assert_true(popover.matches(':popover-open'),'All popovers should be open')); + const targetWasOpenPopover = target.matches(':popover-open'); + + // Show the top layer element as a popover first. + element.showPopover(); + assert_true(element.matches(':popover-open'),'element should be open as a popover'); + assert_equals(target.matches(':popover-open'),targetWasOpenPopover,'target shouldn\'t change popover state'); + + try { + await show(element); + assert_unreached('It is an error to activate a top layer element that is already a showing popover'); + } catch (e) { + // We expect an InvalidStateError for dialogs, and a TypeError for fullscreens. + // Anything else should fall through to the test harness. + if (e.name !== 'InvalidStateError' && e.name !== 'TypeError') { + throw e; + } + } + },`${description} with ${topLayerType}, top layer element *is* a popover`); + + if (testAnchorAttribute) { + promise_test(async t => { + const {element,show,showing} = createTopLayerElement(t,topLayerType); + element.anchorElement = target; + document.body.appendChild(element); + + // Show the popovers. + t.add_cleanup(() => popovers.forEach(popover => popover.hidePopover())); + popovers.forEach(popover => popover.showPopover()); + popovers.forEach(popover => assert_true(popover.matches(':popover-open'),'All popovers should be open')); + + // Activate the top layer element. + await show(popovers[popovers.length-1]); + assert_true(showing()); + popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Incorrect behavior')); + },`${description} with ${topLayerType}, anchor attribute`); + } + }); + }); +}