Fix corner case invoker issue with popover nested inside invoker #10770
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In this situation: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This patch fixes that case.
annevk
reviewed
Nov 18, 2024
mfreed7
commented
Nov 18, 2024
Collaborator
Author
mfreed7
left a comment
mfreed7
left a comment
There was a problem hiding this comment.
Thanks for the review!
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Nov 18, 2024
In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Nov 18, 2024
In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Nov 18, 2024
In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498}
mfreed7
added a commit
to mfreed7/wpt
that referenced
this pull request
Nov 18, 2024
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Nov 20, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232
annevk
reviewed
Nov 21, 2024
Collaborator
Author
Thanks! I filled out the rest of the template. |
i3roly
pushed a commit
to i3roly/firefox-dynasty
that referenced
this pull request
Nov 21, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232
annevk
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Nov 22, 2024
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Nov 26, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Reviewed-by: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232 UltraBlame original commit: 1bc5ed8ce26d83713b264aae65068e3c8de79546
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Nov 27, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Reviewed-by: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232 UltraBlame original commit: 1bc5ed8ce26d83713b264aae65068e3c8de79546
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Nov 27, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Reviewed-by: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232 UltraBlame original commit: 1bc5ed8ce26d83713b264aae65068e3c8de79546
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Nov 27, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Nov 28, 2024
…-button test, a=testonly Automatic update from web-platform-tests Remove .tentative from popover-nested-in-button test Spec PR: whatwg/html#10770 -- wpt-commits: d8bd5c4ede4a4548950435d135e85ed52468778a wpt-pr: 49241
i3roly
pushed a commit
to i3roly/firefox-dynasty
that referenced
this pull request
Nov 30, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232
i3roly
pushed a commit
to i3roly/firefox-dynasty
that referenced
this pull request
Nov 30, 2024
…-button test, a=testonly Automatic update from web-platform-tests Remove .tentative from popover-nested-in-button test Spec PR: whatwg/html#10770 -- wpt-commits: d8bd5c4ede4a4548950435d135e85ed52468778a wpt-pr: 49241
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Dec 1, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Reviewed-by: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232 UltraBlame original commit: 9ef5f6532921cb390f6bb6a414f9d7a989e859d3
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Dec 1, 2024
…-button test, a=testonly Automatic update from web-platform-tests Remove .tentative from popover-nested-in-button test Spec PR: whatwg/html#10770 -- wpt-commits: d8bd5c4ede4a4548950435d135e85ed52468778a wpt-pr: 49241 UltraBlame original commit: 8882e5746d1dc531c6d8054452b50f9ff4e5ca9a
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Dec 1, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Reviewed-by: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232 UltraBlame original commit: 9ef5f6532921cb390f6bb6a414f9d7a989e859d3
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Dec 1, 2024
…-button test, a=testonly Automatic update from web-platform-tests Remove .tentative from popover-nested-in-button test Spec PR: whatwg/html#10770 -- wpt-commits: d8bd5c4ede4a4548950435d135e85ed52468778a wpt-pr: 49241 UltraBlame original commit: 8882e5746d1dc531c6d8054452b50f9ff4e5ca9a
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Dec 1, 2024
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Reviewed-by: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232 UltraBlame original commit: 9ef5f6532921cb390f6bb6a414f9d7a989e859d3
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Dec 1, 2024
…-button test, a=testonly Automatic update from web-platform-tests Remove .tentative from popover-nested-in-button test Spec PR: whatwg/html#10770 -- wpt-commits: d8bd5c4ede4a4548950435d135e85ed52468778a wpt-pr: 49241 UltraBlame original commit: 8882e5746d1dc531c6d8054452b50f9ff4e5ca9a
webkit-commit-queue
pushed a commit
to lukewarlow/WebKit
that referenced
this pull request
Dec 8, 2024
https://bugs.webkit.org/show_bug.cgi?id=283494 Reviewed by Tim Nguyen. This patch matches the new spec behaviour for when a popover is inside its invoker. See whatwg/html#10770 * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-nested-in-button-expected.txt: * Source/WebCore/html/HTMLButtonElement.cpp: (WebCore::HTMLButtonElement::defaultEventHandler): * Source/WebCore/html/HTMLFormControlElement.cpp: (WebCore::HTMLFormControlElement::handlePopoverTargetAction const): * Source/WebCore/html/HTMLFormControlElement.h: * Source/WebCore/html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::defaultEventHandler): Canonical link: https://commits.webkit.org/287522@main
mnutt
pushed a commit
to movableink/webkit
that referenced
this pull request
Dec 8, 2024
https://bugs.webkit.org/show_bug.cgi?id=283494 Reviewed by Tim Nguyen. This patch matches the new spec behaviour for when a popover is inside its invoker. See whatwg/html#10770 * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-nested-in-button-expected.txt: * Source/WebCore/html/HTMLButtonElement.cpp: (WebCore::HTMLButtonElement::defaultEventHandler): * Source/WebCore/html/HTMLFormControlElement.cpp: (WebCore::HTMLFormControlElement::handlePopoverTargetAction const): * Source/WebCore/html/HTMLFormControlElement.h: * Source/WebCore/html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::defaultEventHandler): Canonical link: https://commits.webkit.org/287522@main
jwidar
pushed a commit
to jwidar/LatencyZeroGithub
that referenced
this pull request
Sep 16, 2025
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232
jwidar
pushed a commit
to jwidar/LatencyZeroGithub
that referenced
this pull request
Sep 16, 2025
…ained within invokers, a=testonly Automatic update from web-platform-tests Implement new behavior for popovers contained within invokers In this case: ``` <button popovertarget=foo>Activate <div popover id=foo>Clicking me shouldn't close me</div> </button> ``` clicking the button properly activates the popover, however, clicking on the popover itself after that should **not** close the popover. It currently does because the popover click bubbles to the `<button>` and activates the invoker, which toggles the popover closed. This CL changes that behavior so that clicks on the popover in the case above no longer re-invoke the popover. Spec PR: whatwg/html#10770 Bug: 379241451 Change-Id: Iab67127c46a97a081a7818bfd917864729bf8b5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026982 Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1384498} -- wpt-commits: 50f7c0548260cdc2f11bface1816a283f9314de8 wpt-pr: 49232
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this situation:
clicking the button properly activates the popover, however, clicking on the popover itself after that should not close the popover. It currently does, because the popover click bubbles to the
<button>and activates the invoker, which toggles the popover closed.This patch fixes that case by checking that the invoke event wasn't on the popover itself.
(See WHATWG Working Mode: Changes for more details.)
/form-elements.html ( diff )
/input.html ( diff )
/popover.html ( diff )