Skip to content

Commit

Permalink
Bug 1828216 [wpt PR 39560] - Only use the used invoker to establish p…
Browse files Browse the repository at this point in the history
…opover 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:
whatwg/html#9171

[1] whatwg/html#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 <[email protected]>
Code-Coverage: Findit <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1131606}

--

wpt-commits: 5acfa513ebd07b44adc8be2fc2c491e1b90236da
wpt-pr: 39560
  • Loading branch information
mfreed7 authored and moz-wptsync-bot committed Apr 19, 2023
1 parent cfe4d55 commit 0f526b7
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand Down

0 comments on commit 0f526b7

Please sign in to comment.