Skip to content

Commit

Permalink
[popover] Only use the used invoker to establish popover hierarchy
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=255878

Reviewed by Tim Nguyen.

Implement this behaviour after the discussion here:
whatwg/html#9160

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-shadow-dom.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled.html:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* Source/WebCore/html/HTMLElement.cpp:
(WebCore::topmostPopoverAncestor):
(WebCore::HTMLElement::checkAndPossiblyClosePopoverStackInternal): Deleted.
* Source/WebCore/html/HTMLElement.h:
(WebCore::HTMLElement::checkAndPossiblyClosePopoverStack): Deleted.
* Source/WebCore/html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::removedFromAncestor):
(WebCore::HTMLFormControlElement::attributeChanged):
(WebCore::HTMLFormControlElement::disabledStateChanged):
(WebCore::HTMLFormControlElement::didChangeForm): Deleted.
* Source/WebCore/html/HTMLFormControlElement.h:
* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::updateType):

Canonical link: https://commits.webkit.org/264002@main
  • Loading branch information
rwlbuis committed May 12, 2023
1 parent e4cb3bd commit 3bb25ea
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ PASS Clicking inside a popover does not close that popover
PASS Popovers close on pointerup, not pointerdown
PASS Synthetic events can't close popovers
PASS Moving focus outside the popover should not dismiss the popover
PASS Clicking inside a child popover shouldn't close either popover
PASS Clicking inside a parent popover should close child popover
FAIL Clicking inside a child popover shouldn't close either popover assert_true: popover1 should be open expected true got false
FAIL Clicking inside a parent popover should close child popover assert_true: expected true got false
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case)
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation)
FAIL Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation) assert_true: expected true got false
PASS Clicking on invoking element, even if it wasn't used for activation, shouldn't close its popover
PASS Clicking on popovertarget element, even if it wasn't used for activation, should hide it exactly once
PASS Clicking on anchor element (that isn't an invoking element) shouldn't prevent its popover from being closed
PASS Dragging from an open popover outside an open popover should leave the popover open
FAIL Dragging from an open popover outside an open popover should leave the popover open assert_true: expected true got false
PASS A popover inside an invoking element doesn't participate in that invoker's ancestor chain
PASS An invoking element that was not used to invoke the popover can still be part of the ancestor chain
PASS An invoking element that was not used to invoke the popover is not part of the ancestor chain
FAIL Scrolling within a popover should not close the popover promise_test: Unhandled rejection with value: object "Error: Unknown source type "wheel"."
PASS Clicking inside a shadow DOM popover does not close that popover
PASS Clicking outside a shadow DOM popover should close that popover
Expand Down
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 @@ -2,11 +2,10 @@ form
helloother button

PASS Disabled popover*target buttons should not affect the popover heirarchy.
PASS Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly outer popover is closed.
PASS Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly inner popover is closed.
PASS Setting the form attribute on popover*target buttons when popovers are open should close all popovers.
PASS Changing the input type on a popover*target button when popovers are open should close all popovers.
PASS Disconnecting popover*target buttons when popovers are open should close all popovers.
PASS Changing the popovertarget attribute to break the chain should close all popovers.
PASS Disabling popover*target buttons when popovers are open should not cause popovers to be closed.
PASS Setting the form attribute on popover*target buttons when popovers are open should not close them.
PASS Changing the input type on a popover*target button when popovers are open should not close anything.
PASS Disconnecting popover*target buttons when popovers are open should not close anything.
PASS Changing the popovertarget attribute to break the chain should not close anything.
PASS Modifying popovertarget on a button which doesn't break the chain shouldn't close any popovers.

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

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ PASS Clicking inside a popover does not close that popover
PASS Popovers close on pointerup, not pointerdown
PASS Synthetic events can't close popovers
PASS Moving focus outside the popover should not dismiss the popover
PASS Clicking inside a child popover shouldn't close either popover
PASS Clicking inside a parent popover should close child popover
FAIL Clicking inside a child popover shouldn't close either popover assert_true: popover1 should be open expected true got false
FAIL Clicking inside a parent popover should close child popover assert_true: expected true got false
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case)
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation)
FAIL Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation) assert_true: expected true got false
PASS Clicking on invoking element, even if it wasn't used for activation, shouldn't close its popover
PASS Clicking on popovertarget element, even if it wasn't used for activation, should hide it exactly once
PASS Clicking on anchor element (that isn't an invoking element) shouldn't prevent its popover from being closed
PASS Dragging from an open popover outside an open popover should leave the popover open
FAIL Dragging from an open popover outside an open popover should leave the popover open assert_true: expected true got false
PASS A popover inside an invoking element doesn't participate in that invoker's ancestor chain
PASS An invoking element that was not used to invoke the popover can still be part of the ancestor chain
PASS An invoking element that was not used to invoke the popover is not part of the ancestor chain
FAIL Scrolling within a popover should not close the popover promise_test: Unhandled rejection with value: object "Error: Unknown source type "wheel"."
PASS Clicking inside a shadow DOM popover does not close that popover
PASS Clicking outside a shadow DOM popover should close that popover
Expand Down
Loading

0 comments on commit 3bb25ea

Please sign in to comment.