Skip to content

[popover] Avoid conflicting interactions in the top layer#11155

Merged
webkit-early-warning-system merged 1 commit into
WebKit:mainfrom
nt1m:eng/popover-Avoid-conflicting-interactions-in-the-top-layer
Mar 7, 2023
Merged

[popover] Avoid conflicting interactions in the top layer#11155
webkit-early-warning-system merged 1 commit into
WebKit:mainfrom
nt1m:eng/popover-Avoid-conflicting-interactions-in-the-top-layer

Conversation

@nt1m

@nt1m nt1m commented Mar 7, 2023 •
edited by webkit-early-warning-system
Loading

Copy link
Copy Markdown
Member

ffc70a7

[popover] Avoid conflicting interactions in the top layer
https://bugs.webkit.org/show_bug.cgi?id=252297
rdar://105763866

Reviewed by Simon Fraser.

- Run "hide all popovers" algorithm every time something is appended to the top layer (since the auto popovers will be covered by the new element)
- Don't allow something in the popover showing visibility state to be appended in the top layer, since it is already there (this is similar to how dialogs can't go fullscreen).
- Fix test to avoid race conditions by properly waiting for async operations

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-combinations-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions.html:
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/gtk/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt: Added.
* Source/WebCore/dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::requestFullscreenForElement):
(WebCore::FullscreenManager::willEnterFullscreen):
(WebCore::isInPopoverShowingState):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::showModal):
(WebCore::HTMLDialogElement::runFocusingSteps):

Canonical link: https://commits.webkit.org/261317@main

4490465

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios   🛠 mac ✅ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
  🧪 api-ios   🧪 mac-wk1   🧪 gtk-wk2
✅ 🛠 tv   🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch   🧪 mac-wk2-stress
  🛠 🧪 merge ✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@nt1m nt1m requested review from cdumez and rniwa as code owners March 7, 2023 06:22
@nt1m nt1m self-assigned this Mar 7, 2023
@webkit-early-warning-system

webkit-early-warning-system commented Mar 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 2b2921a)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios   🧪 mac-wk1   🧪 gtk-wk2
✅ 🛠 tv   🧪 mac-wk2 ⏳ 🧪 api-gtk
✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch ⏳ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@nt1m nt1m changed the title ??? [popover] Avoid conflicting interactions in the top layer Mar 7, 2023
@nt1m nt1m added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Mar 7, 2023
smfr
smfr approved these changes Mar 7, 2023
View reviewed changes
Comment thread Source/WebCore/dom/FullscreenManager.cpp Outdated

@smfr smfr Mar 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should move this code into a function and share it.

@nt1m nt1m force-pushed the eng/popover-Avoid-conflicting-interactions-in-the-top-layer branch from 2b2921a to 9cf6adb Compare March 7, 2023 07:00
@nt1m nt1m added the merge-queue Applied to send a pull request to merge-queue label Mar 7, 2023
@webkit-early-warning-system

webkit-early-warning-system commented Mar 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 9cf6adb)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios   🧪 mac-wk1 ❌ 🧪 gtk-wk2
✅ 🛠 tv   🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch   🧪 mac-wk2-stress
❌ 🛠 🧪 merge ✅ 🛠 watch-sim

@nt1m nt1m removed the merge-queue Applied to send a pull request to merge-queue label Mar 7, 2023
@nt1m nt1m force-pushed the eng/popover-Avoid-conflicting-interactions-in-the-top-layer branch from 9cf6adb to 4490465 Compare March 7, 2023 08:41
@nt1m nt1m added the merge-queue Applied to send a pull request to merge-queue label Mar 7, 2023
@webkit-early-warning-system

webkit-early-warning-system commented Mar 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on current version of this PR (hash 4490465)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios   🛠 mac ✅ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
  🧪 api-ios   🧪 mac-wk1   🧪 gtk-wk2
✅ 🛠 tv   🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch   🧪 mac-wk2-stress
  🛠 🧪 merge ✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@nt1m nt1m added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merge-queue Applied to send a pull request to merge-queue labels Mar 7, 2023
@nt1m
[popover] Avoid conflicting interactions in the top layer
ffc70a7 
https://bugs.webkit.org/show_bug.cgi?id=252297
rdar://105763866

Reviewed by Simon Fraser.

- Run "hide all popovers" algorithm every time something is appended to the top layer (since the auto popovers will be covered by the new element)
- Don't allow something in the popover showing visibility state to be appended in the top layer, since it is already there (this is similar to how dialogs can't go fullscreen).
- Fix test to avoid race conditions by properly waiting for async operations

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-combinations-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-interactions.html:
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/gtk/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt: Added.
* Source/WebCore/dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::requestFullscreenForElement):
(WebCore::FullscreenManager::willEnterFullscreen):
(WebCore::isInPopoverShowingState):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::showModal):
(WebCore::HTMLDialogElement::runFocusingSteps):

Canonical link: https://commits.webkit.org/261317@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/popover-Avoid-conflicting-interactions-in-the-top-layer branch from 4490465 to ffc70a7 Compare March 7, 2023 08:54
@webkit-commit-queue

webkit-commit-queue commented Mar 7, 2023

Copy link
Copy Markdown
Collaborator

Committed 261317@main (ffc70a7): https://commits.webkit.org/261317@main

Reviewed commits have been landed. Closing PR #11155 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit ffc70a7 into WebKit:main Mar 7, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 7, 2023
@nt1m nt1m deleted the eng/popover-Avoid-conflicting-interactions-in-the-top-layer branch March 7, 2023 08:58
@foolip foolip mentioned this pull request Apr 25, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smfr smfr smfr approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@pxlcoder pxlcoder Awaiting requested review from pxlcoder

@darinadler darinadler Awaiting requested review from darinadler

@jernoble jernoble Awaiting requested review from jernoble

Assignees

@nt1m nt1m

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nt1m @webkit-early-warning-system @webkit-commit-queue @smfr