Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

REGRESSION (iOS 16): AR QuickLook banner tap event no longer propagated to anchor if not attached to DOM #6226

Merged
merged 1 commit into from Nov 8, 2022
Merged

REGRESSION (iOS 16): AR QuickLook banner tap event no longer propagated to anchor if not attached to DOM #6226

merged 1 commit into from Nov 8, 2022

Conversation

rr-codes
Copy link
Contributor

@rr-codes rr-codes commented Nov 7, 2022 •
edited by webkit-early-warning-system

a203e21

REGRESSION (iOS 16): AR QuickLook banner tap event no longer propagated to anchor if not attached to DOM
https://bugs.webkit.org/show_bug.cgi?id=245959
rdar://100952298

Reviewed by Wenson Hsieh.

The commit https://commits.webkit.org/251042@main added a check `!element->isConnected()`
in `Document::dispatchSystemPreviewActionEvent` to ensure the element is part
of the DOM before sending the `_apple_ar_quicklook_button_tapped` event.

However, this broke behavior in the case of a detached element, and so this PR
reverts that check to maintain the existing behavior.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::dispatchSystemPreviewActionEvent):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SystemPreview.mm:
(-[TestSystemPreviewTriggeredHandler userContentController:didReceiveScriptMessage:]):
(-[TestSystemPreviewTriggeredOnDetachedElementHandler userContentController:didReceiveScriptMessage:]):
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/system-preview-trigger.html:

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

e2fc55f

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

@rr-codes rr-codes self-assigned this Nov 7, 2022
@rr-codes rr-codes added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Nov 7, 2022
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash f8c3397)

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

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 715f69a)

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

@webkit-early-warning-system
Copy link
Collaborator

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

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

@rr-codes rr-codes added the merge-queue Applied to send a pull request to merge-queue label Nov 8, 2022
@rr-codes
REGRESSION (iOS 16): AR QuickLook banner tap event no longer propagat…
a203e21 
…ed to anchor if not attached to DOM

https://bugs.webkit.org/show_bug.cgi?id=245959
rdar://100952298

Reviewed by Wenson Hsieh.

The commit https://commits.webkit.org/251042@main added a check `!element->isConnected()`
in `Document::dispatchSystemPreviewActionEvent` to ensure the element is part
of the DOM before sending the `_apple_ar_quicklook_button_tapped` event.

However, this broke behavior in the case of a detached element, and so this PR
reverts that check to maintain the existing behavior.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::dispatchSystemPreviewActionEvent):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SystemPreview.mm:
(-[TestSystemPreviewTriggeredHandler userContentController:didReceiveScriptMessage:]):
(-[TestSystemPreviewTriggeredOnDetachedElementHandler userContentController:didReceiveScriptMessage:]):
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/system-preview-trigger.html:

Canonical link: https://commits.webkit.org/256462@main
@webkit-early-warning-system webkit-early-warning-system merged commit a203e21 into WebKit:main Nov 8, 2022
@webkit-commit-queue
Copy link
Collaborator

Committed 256462@main (a203e21): https://commits.webkit.org/256462@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whsieh whsieh whsieh approved these changes

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

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

@pxlcoder pxlcoder Awaiting requested review from pxlcoder

Assignees

@rr-codes rr-codes

Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Milestone
No milestone
4 participants
@rr-codes @webkit-early-warning-system @webkit-commit-queue @whsieh