From 0f526b711f40115dfa4bba8ebf10526b067e052e Mon Sep 17 00:00:00 2001
From: Mason Freed <masonf@chromium.org>
Date: Tue, 18 Apr 2023 22:10:42 +0000
Subject: [PATCH] Bug 1828216 [wpt PR 39560] - Only use the used invoker to
 establish popover hierarchy, a=testonly

Automatic update from web-platform-tests
Only use the used invoker to establish popover hierarchy

See [1] for more context, but the idea is that instead of using just
the DOM structure to establish the popover hierarchy, the user's
behavior should matter. For example, if one popover contains a popover
invoker pointing to another popover, it should matter whether that
invoker is *actually used* to open the second popover.

An example:
 - Component 1 is a third party widget, which uses popover
 - Component 2 is another third party widget, also using popover
 - A page wants to use both components separately, from separate
   invoking buttons.
 - Component 1 also wants to be able to use Component 2, via a button
   within Component 1.

In this example, the page should be able to still independently use
these components. So a user clicking the page's button for Component 2
is expected to close Component 1 if it's open, because that's a direct
invocation of Component 2. However, if the user clicks the button
within Component 1 to get Component 2, it is natural to leave Component
1 open because this is a nested call.

Important note: this often happens to be the behavior even before this
CL, since the user clicking on the page-level Component 2 invoking
button represents a light dismiss signal for Component 1, so it closes
either way. But this CL simplifies the implementation considerably,
removing the need to track all invokers on the page, and also removing
the need to continuously check whether invoker relationships have
changed.

Spec PR:
https://github.com/whatwg/html/pull/9171

[1] https://github.com/whatwg/html/issues/9160

Bug: 1307772
Change-Id: I60ccb133a79084db8c251218fdbd10684fea947b
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4429412
Commit-Queue: Mason Freed <masonf@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1131606}

--

wpt-commits: 5acfa513ebd07b44adc8be2fc2c491e1b90236da
wpt-pr: 39560
---
 .../popovers/popover-light-dismiss.html       |  7 +-
 .../popovers/popover-shadow-dom.html          |  2 +-
 .../popover-target-element-disabled.html      | 85 ++++++++-----------
 3 files changed, 38 insertions(+), 56 deletions(-)

diff --git a/testing/web-platform/tests/html/semantics/popovers/popover-light-dismiss.html b/testing/web-platform/tests/html/semantics/popovers/popover-light-dismiss.html
index 0cb13979407e4..cc28df3be3bbd 100644
--- a/testing/web-platform/tests/html/semantics/popovers/popover-light-dismiss.html
+++ b/testing/web-platform/tests/html/semantics/popovers/popover-light-dismiss.html
@@ -298,11 +298,10 @@
     assert_false(popover3.matches(':popover-open'));
     popover3.showPopover();
     assert_true(popover3.matches(':popover-open'));
-    assert_true(popover5.matches(':popover-open'));
-    popover5.hidePopover();
+    assert_false(popover5.matches(':popover-open'),'Popover 5 was not invoked from popover3\'s invoker');
+    popover3.hidePopover();
     assert_false(popover3.matches(':popover-open'));
-    assert_false(popover5.matches(':popover-open'));
-  },'An invoking element that was not used to invoke the popover can still be part of the ancestor chain');
+  },'An invoking element that was not used to invoke the popover is not part of the ancestor chain');
 </script>
 
 <div popover id=p6>Inside popover 6
diff --git a/testing/web-platform/tests/html/semantics/popovers/popover-shadow-dom.html b/testing/web-platform/tests/html/semantics/popovers/popover-shadow-dom.html
index 9279a121bea73..87293f1e3d56c 100644
--- a/testing/web-platform/tests/html/semantics/popovers/popover-shadow-dom.html
+++ b/testing/web-platform/tests/html/semantics/popovers/popover-shadow-dom.html
@@ -187,7 +187,7 @@
     polyfill_declarative_shadow_dom(test5);
     const [popover1,popover2] = getPopoverReferences('test5');
     popover1.showPopover();
-    popover2.showPopover();
+    popover1.querySelector('button').click(); // Use invoker to keep 2 visible
     // Both 1 and 2 should be open at this point.
     assert_true(popover1.matches(':popover-open'), 'popover1 not open');
     assert_true(isElementVisible(popover1));
diff --git a/testing/web-platform/tests/html/semantics/popovers/popover-target-element-disabled.html b/testing/web-platform/tests/html/semantics/popovers/popover-target-element-disabled.html
index bf1f297148442..d5c951768c083 100644
--- a/testing/web-platform/tests/html/semantics/popovers/popover-target-element-disabled.html
+++ b/testing/web-platform/tests/html/semantics/popovers/popover-target-element-disabled.html
@@ -11,6 +11,10 @@
 <script>
 test(() => {
   outerpopover.showPopover();
+  outerpopover.querySelector('button').click(); // Invoke innerpopover
+  assert_false(innerpopover.matches(':popover-open'),
+    'disabled button shouldn\'t open the target popover');
+  assert_true(outerpopover.matches(':popover-open'));
   innerpopover.showPopover();
   assert_true(innerpopover.matches(':popover-open'),
     'The inner popover should be able to open successfully.');
@@ -26,39 +30,18 @@
 <script>
 test(() => {
   outerpopover2.showPopover();
-  innerpopover2.showPopover();
+  outerpopover2.querySelector('button').click(); // Invoke innerpopover
   assert_true(innerpopover2.matches(':popover-open'),
     'The inner popover should be able to open successfully.');
   assert_true(outerpopover2.matches(':popover-open'),
     'The outer popover should stay open when opening the inner one.');
 
   togglebutton2.disabled = true;
-  assert_false(innerpopover2.matches(':popover-open'),
-    'The inner popover should be closed when the hierarchy is broken.');
-  assert_false(outerpopover2.matches(':popover-open'),
-    'The outer popover should be closed when the hierarchy is broken.');
-}, 'Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly outer popover is closed.');
-</script>
-
-<div id=outerpopover3 popover=auto>
-  <button id=togglebutton3 popovertarget=innerpopover3>toggle popover</button>
-</div>
-<div id=innerpopover3 popover=auto>popover</div>
-<script>
-test(() => {
-  outerpopover3.showPopover();
-  innerpopover3.showPopover();
-  assert_true(innerpopover3.matches(':popover-open'),
-    'The inner popover should be able to open successfully.');
-  assert_true(outerpopover3.matches(':popover-open'),
-    'The outer popover should stay open when opening the inner one.');
-
-  togglebutton3.disabled = true;
-  assert_false(innerpopover3.matches(':popover-open'),
-    'The inner popover be should be closed when the hierarchy is broken.');
-  assert_false(outerpopover3.matches(':popover-open'),
-    'The outer popover be should be closed when the hierarchy is broken.');
-}, 'Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly inner popover is closed.');
+  assert_true(innerpopover2.matches(':popover-open'),
+    'Changing disabled states after popovers are open shouldn\'t close anything');
+  assert_true(outerpopover2.matches(':popover-open'),
+  'Changing disabled states after popovers are open shouldn\'t close anything');
+}, 'Disabling popover*target buttons when popovers are open should not cause popovers to be closed.');
 </script>
 
 <div id=outerpopover4 popover=auto>
@@ -69,18 +52,18 @@
 <script>
 test(() => {
   outerpopover4.showPopover();
-  innerpopover4.showPopover();
+  outerpopover4.querySelector('button').click(); // Invoke innerpopover
   assert_true(innerpopover4.matches(':popover-open'),
     'The inner popover should be able to open successfully.');
   assert_true(outerpopover4.matches(':popover-open'),
     'The outer popover should stay open when opening the inner one.');
 
   togglebutton4.setAttribute('form', 'submitform');
-  assert_false(innerpopover4.matches(':popover-open'),
-    'The inner popover be should be closed when the hierarchy is broken.');
-  assert_false(outerpopover4.matches(':popover-open'),
-    'The outer popover be should be closed when the hierarchy is broken.');
-}, 'Setting the form attribute on popover*target buttons when popovers are open should close all popovers.');
+  assert_true(innerpopover4.matches(':popover-open'),
+    'The inner popover be should be not closed when invoking buttons cease to be invokers.');
+  assert_true(outerpopover4.matches(':popover-open'),
+    'The outer popover be should be not closed when invoking buttons cease to be invokers.');
+}, 'Setting the form attribute on popover*target buttons when popovers are open should not close them.');
 </script>
 
 <div id=outerpopover5 popover=auto>
@@ -90,18 +73,18 @@
 <script>
 test(() => {
   outerpopover5.showPopover();
-  innerpopover5.showPopover();
+  outerpopover5.querySelector('input').click(); // Invoke innerpopover
   assert_true(innerpopover5.matches(':popover-open'),
     'The inner popover should be able to open successfully.');
   assert_true(outerpopover5.matches(':popover-open'),
     'The outer popover should stay open when opening the inner one.');
 
   togglebutton5.setAttribute('type', 'text');
-  assert_false(innerpopover5.matches(':popover-open'),
-    'The inner popover be should be closed when the hierarchy is broken.');
-  assert_false(outerpopover5.matches(':popover-open'),
-    'The outer popover be should be closed when the hierarchy is broken.');
-}, 'Changing the input type on a popover*target button when popovers are open should close all popovers.');
+  assert_true(innerpopover5.matches(':popover-open'),
+    'The inner popover be should be not closed when invoking buttons cease to be invokers.');
+  assert_true(outerpopover5.matches(':popover-open'),
+    'The outer popover be should be not closed when invoking buttons cease to be invokers.');
+}, 'Changing the input type on a popover*target button when popovers are open should not close anything.');
 </script>
 
 <div id=outerpopover6 popover=auto>
@@ -111,18 +94,18 @@
 <script>
 test(() => {
   outerpopover6.showPopover();
-  innerpopover6.showPopover();
+  outerpopover6.querySelector('button').click(); // Invoke innerpopover
   assert_true(innerpopover6.matches(':popover-open'),
     'The inner popover should be able to open successfully.');
   assert_true(outerpopover6.matches(':popover-open'),
     'The outer popover should stay open when opening the inner one.');
 
   togglebutton6.remove();
-  assert_false(innerpopover6.matches(':popover-open'),
-    'The inner popover be should be closed when the hierarchy is broken.');
-  assert_false(outerpopover6.matches(':popover-open'),
-    'The outer popover be should be closed when the hierarchy is broken.');
-}, 'Disconnecting popover*target buttons when popovers are open should close all popovers.');
+  assert_true(innerpopover6.matches(':popover-open'),
+    'The inner popover be should be not closed when invoking buttons are removed.');
+  assert_true(outerpopover6.matches(':popover-open'),
+    'The outer popover be should be not closed when invoking buttons are removed.');
+}, 'Disconnecting popover*target buttons when popovers are open should not close anything.');
 </script>
 
 <div id=outerpopover7 popover=auto>
@@ -132,18 +115,18 @@
 <script>
 test(() => {
   outerpopover7.showPopover();
-  innerpopover7.showPopover();
+  outerpopover7.querySelector('button').click(); // Invoke innerpopover
   assert_true(innerpopover7.matches(':popover-open'),
     'The inner popover should be able to open successfully.');
   assert_true(outerpopover7.matches(':popover-open'),
     'The outer popover should stay open when opening the inner one.');
 
   togglebutton7.setAttribute('popovertarget', 'otherpopover7');
-  assert_false(innerpopover7.matches(':popover-open'),
-    'The inner popover be should be closed when the hierarchy is broken.');
-  assert_false(outerpopover7.matches(':popover-open'),
-    'The outer popover be should be closed when the hierarchy is broken.');
-}, 'Changing the popovertarget attribute to break the chain should close all popovers.');
+  assert_true(innerpopover7.matches(':popover-open'),
+    'The inner popover be should be not closed when invoking buttons are retargeted.');
+    assert_true(outerpopover7.matches(':popover-open'),
+    'The outer popover be should be not closed when invoking buttons are retargeted.');
+}, 'Changing the popovertarget attribute to break the chain should not close anything.');
 </script>
 
 <div id=outerpopover8 popover=auto>