Remove "resize" as a <popup> light dismiss trigger [chromium/src : main]
Mason Freed (Gerrit)
Attention is currently required from: Dan Clark.
Mason Freed would like Dan Clark to review this change.
Remove "resize" as a <popup> light dismiss trigger
This ended up being a confusing part of the implementation of
<popup>: it was easy to accidentally resize the popup, causing
it to light dismiss. For example, adding a :hover style that
adds a border to the popup or an element might cause a light
dismiss. And the rationale for this being a trigger in the
first place was an implementation concern - avoiding
expensive relayout in the case that the popup is anchor-
positioned to something on the page.
Bug:1168738
Change-Id: Ic5a3e616941ace8b47b4abc02dbf786aa78521c4
Fixed:1252176
---
M third_party/blink/renderer/core/html/html_popup_element.cc
M third_party/blink/renderer/core/html/html_popup_element.h
D third_party/blink/web_tests/external/wpt/html/semantics/interactive-elements/the-popup-element/popup-light-dismiss-resize.tentative.html
3 files changed, 0 insertions(+), 94 deletions(-)
To view, visit change 3180152. To unsubscribe, or for help writing mail filters, visit settings.
Mason Freed (Gerrit)
Attention is currently required from: Dan Clark.
Patch set 1:Auto-Submit +1
1 comment:
Patchset:
PTAL, thanks!
To view, visit change 3180152. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Attention is currently required from: Dan Clark.
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/30948.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Dan Clark (Gerrit)
Attention is currently required from: Mason Freed.
Patch set 1:Code-Review +1
2 comments:
Patchset:
LGTM, thanks!
File third_party/blink/web_tests/external/wpt/html/semantics/interactive-elements/the-popup-element/popup-light-dismiss-resize.tentative.html:
Patch Set #1, Line 35:
assert_false(popup.open,'popup should close when resized');Just a thought, would it be worthwhile to keep the test and have it validate that resize doesn't close the popup? Although checking that an async operation never happens is kind of a strange test...so maybe not worth it.
To view, visit change 3180152. To unsubscribe, or for help writing mail filters, visit settings.
Mason Freed (Gerrit)
Patch set 2:Auto-Submit +1Commit-Queue +2
1 comment:
File third_party/blink/web_tests/external/wpt/html/semantics/interactive-elements/the-popup-element/popup-light-dismiss-resize.tentative.html:
Patch Set #1, Line 35:
assert_false(popup.open,'popup should close when resized');
Just a thought, would it be worthwhile to keep the test and have it validate that resize doesn't clo […]
Thanks! I did think about doing that, but as you mentioned, it's hard to write a test to check for something that (asynchronously) "never happens". You almost have to write a setTimeout() and that's discouraged. So I just went with no test here.
I'm going to land this, but if you really think we need a test, I'm happy to land one as a followup!
To view, visit change 3180152. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Remove "resize" as a <popup> light dismiss trigger
This ended up being a confusing part of the implementation of
<popup>: it was easy to accidentally resize the popup, causing
it to light dismiss. For example, adding a :hover style that
adds a border to the popup or an element might cause a light
dismiss. And the rationale for this being a trigger in the
first place was an implementation concern - avoiding
expensive relayout in the case that the popup is anchor-
positioned to something on the page.
Bug: 1168738
Change-Id: Ic5a3e616941ace8b47b4abc02dbf786aa78521c4
Fixed: 1252176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3180152
Commit-Queue: Mason Freed <mas...@chromium.org>
Auto-Submit: Mason Freed <mas...@chromium.org>
Reviewed-by: Dan Clark <dan...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#924911}
---
M third_party/blink/renderer/core/html/html_popup_element.cc
M third_party/blink/renderer/core/html/html_popup_element.h
D third_party/blink/web_tests/external/wpt/html/semantics/interactive-elements/the-popup-element/popup-light-dismiss-resize.tentative.html
3 files changed, 0 insertions(+), 94 deletions(-)
Blink WPT Bot (Gerrit)
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/30948