fix(macOS): closing a popup created from window.open can break IPC in the opener#1718
fix(macOS): closing a popup created from window.open can break IPC in the opener#1718CyberVy wants to merge 4 commits into
window.open can break IPC in the opener#1718Conversation
|
Can some please review this PR, thank you. :) |
Package Changes Through 76bb64eThere are 1 changes which include wry with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
@Legend-Master Hi. I'm sorry for bothering, but could you please review this PR? This is just an easy fix but it helps a lot. Thank you very much! |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Hey, sorry if I mislabeled this one. I/we have very limited time working on/reviewing things and are getting lots of automated PRs from LLM agents lately that I simply don't have the time to judge each one closely. And the description of this PR very much looks like it's generated from an LLM especially the 'Fix' section stating each code modifications |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
|
||
| let proto_delegate = ProtocolObject::from_ref(&*delegate); | ||
| unsafe { | ||
| let did_register_ipc_handler = unsafe { |
There was a problem hiding this comment.
I wonder what if you close the first window but kept the one from window.open? (when the first window drops, it will still remove the handler for everyone on the same content world?)
There was a problem hiding this comment.
This is a very good point. According to my knowledge, it will, because the message handler is registered successfully by the first window(Who registered it, who removes it). But I didn't make a test about it. I'll make a test tomorrow when I have my computer and give you response.
There was a problem hiding this comment.
The answer is yes according to the test. I got this exception in the other windows after the first window is closed.
TypeError: undefined is not an object (evaluating 'window.webkit.messageHandlers.ipc.postMessage')
|
I also discovered some bugs related to IPC message handlers. I think we should split fixes into several PRs to solve all of them. However, they may have implications for how we fix the current bug. So I'll describe them here. "Capabilities" only respect the windows that registered shared message handlers. For the other windows, they are treated as the windows which registered shared message handlers, and they are able to trigger Rust invocation, but the results can't be sent to frontend. Because message handlers don't know what they belong to. They are bound to the windows that registered message handlers, so the messages can be sent to the wrong windows. These issues make IPC almost unavailable when the windows share an IPC message handler. This PR can fix lifecycle of shared message handlers and make the windows registered them work fine, but cannot make IPC in other windows work perfectly. So, is the mechanism of shared message handlers still a good choice? Still patch or refactor? |
|
That's tricky... I wonder if that also applies to other things like custom protocol handlers? cc @lucasfernog I'm not the most familiar with the macOS part, maybe you can take over this? |
Yes, as far as I know, it also applies to other things. |
Bug
On macOS, a Tauri v2 app can lose the WebKit message handler used by Tauri IPC after a popup opened via
window.openis handled withon_new_windowand then closed.After this happens, the opener/main window still has Tauri's JavaScript initialization objects, but later
invoke()calls fail becausewindow.webkit.messageHandlersbecomesundefinedwhich means it is no longer available.Trigger
This only happens when the current/opener window is loaded from a remote HTTPS URL such as
https://example.com/. I could not reproduce it when the current window uses the local app/dev URL ("/").https://example.com/.window.open(...).NewWindowResponse::Create { window }.window.webkit.messageHandlers === undefined.Analysis
The suspected problem is that
WryWebViewDelegate::new(...)catches exceptions fromaddScriptMessageHandler_name(...), but ignores whether registration succeeded.Later,
InnerWebView::drop(...)unconditionally callsremoveScriptMessageHandlerForName("ipc").However, a popup should not remove the shared IPC message handler unless it successfully registered/owns it.