Allow top layer elements to be nested within popovers

See the issue raised at whatwg/html#9998 which was discussed at whatwg/html#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 <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1254541}
mbrodesser-Igalia · Feb 19, 2024 · a1b4975 · a1b4975
1 parent a6b9d82
commit a1b4975
Show file tree

Hide file tree

Showing 5 changed files with 245 additions and 0 deletions.
diff --git a/html/semantics/popovers/popover-attribute-basic.html b/html/semantics/popovers/popover-attribute-basic.html
@@ -1,6 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel="author" href="mailto:masonf@chromium.org">
+<link rel=help href="https://html.spec.whatwg.org/multipage/popover.html">
 <link rel=help href="https://open-ui.org/components/popover.research.explainer">
 <meta name="timeout" content="long">
 <script src="/resources/testharness.js"></script>

diff --git a/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html b/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" href="mailto:masonf@chromium.org">
+<link rel=help href="https://html.spec.whatwg.org/multipage/popover.html">
+<link rel=help href="https://open-ui.org/components/popover.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="/common/top-layer.js"></script>
+<script src="resources/popover-utils.js"></script>
+<script src="resources/popover-top-layer-nesting.js"></script>
+
+<div id=tests>
+  <div> Single popover=auto ancestor
+    <div popover=auto class=target data-stay-open=true></div>
+  </div>
+
+  <div> Single popover=manual ancestor
+    <div popover=auto class=target data-stay-open=true></div>
+  </div>
+
+  <div> Nested popover=auto ancestors
+    <div popover=auto data-stay-open=true>
+      <div popover=auto class=target data-stay-open=true></div>
+    </div>
+  </div>
+
+  <div> Nested popover=auto ancestors, target is outer
+    <div popover=auto class=target data-stay-open=true>
+      <div popover=auto data-stay-open=false></div>
+    </div>
+  </div>
+
+  <div> Top layer inside of nested element
+    <div popover=auto data-stay-open=true>
+      <button class=target></button>
+    </div>
+  </div>
+</div>
+
+<script>
+  const tests = Array.from(document.querySelectorAll('#tests>div'));
+  runTopLayerTests(tests,/*testAnchorAttribute*/true);
+</script>
diff --git a/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html b/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" href="mailto:masonf@chromium.org">
+<link rel=help href="https://html.spec.whatwg.org/multipage/popover.html">
+<link rel=help href="https://open-ui.org/components/popover.research.explainer">
+<link rel=help href="https://open-ui.org/components/popover-hint.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="/common/top-layer.js"></script>
+<script src="resources/popover-utils.js"></script>
+<script src="resources/popover-top-layer-nesting.js"></script>
+
+<div id=tests>
+  <div> Single popover=hint ancestor
+    <div popover=hint class=target data-stay-open=true></div>
+  </div>
+
+  <div> Nested auto/hint ancestors
+    <div popover=auto data-stay-open=true>
+      <div popover=hint class=target data-stay-open=true></div>
+    </div>
+  </div>
+
+  <div> Nested auto/hint ancestors, target is auto
+    <div popover=auto class=target data-stay-open=true>
+      <div popover=hint data-stay-open=false></div>
+    </div>
+  </div>
+
+  <div> Unrelated hint, target=hint
+    <div popover=auto data-stay-open=true></div>
+    <div popover=hint class=target data-stay-open=true></div>
+  </div>
+
+  <div> Unrelated hint, target=auto
+    <div popover=auto class=target data-stay-open=true></div>
+    <div popover=hint data-stay-open=false></div>
+  </div>
+</div>
+
+<script>
+  const tests = Array.from(document.querySelectorAll('#tests>div'));
+  runTopLayerTests(tests);
+</script>
diff --git a/html/semantics/popovers/popover-top-layer-nesting.tentative.html b/html/semantics/popovers/popover-top-layer-nesting.tentative.html
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" href="mailto:masonf@chromium.org">
+<link rel=help href="https://html.spec.whatwg.org/multipage/popover.html">
+<link rel=help href="https://open-ui.org/components/popover.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="/common/top-layer.js"></script>
+<script src="resources/popover-utils.js"></script>
+<script src="resources/popover-top-layer-nesting.js"></script>
+
+<div id=tests>
+  <div> Single popover=auto ancestor
+    <div popover=auto class=target data-stay-open=true></div>
+  </div>
+
+  <div> Single popover=manual ancestor
+    <div popover=auto class=target data-stay-open=true></div>
+  </div>
+
+  <div> Nested popover=auto ancestors
+    <div popover=auto data-stay-open=true>
+      <div popover=auto class=target data-stay-open=true></div>
+    </div>
+  </div>
+
+  <div> Nested popover=auto ancestors, target is outer
+    <div popover=auto class=target data-stay-open=true>
+      <div popover=auto data-stay-open=false></div>
+    </div>
+  </div>
+
+  <div> Top layer inside of nested element
+    <div popover=auto data-stay-open=true>
+      <button class=target></button>
+    </div>
+  </div>
+</div>
+
+<script>
+  const tests = Array.from(document.querySelectorAll('#tests>div'));
+  runTopLayerTests(tests);
+</script>
diff --git a/html/semantics/popovers/resources/popover-top-layer-nesting.js 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`);
+      }
+    });
+  });
+}