-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(tauri-runtime-wry): ensure webview2 environment options matches #14109
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.
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
base: dev
Are you sure you want to change the base?
Conversation
on Windows the Webview2 environment options must match when created webviews sharing the same data directory (unless specified, all webviews share the same data directory). This PR validates the options, warning if they do not match, and forces the webview to be created with the same previous environment to prevent a crash. discovered as part of #14089
Package Changes Through 059bafaThere are 5 changes which include @tauri-apps/cli with patch, tauri-cli with patch, tauri with patch, tauri-build with patch, tauri-runtime-wry with patch 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with this, we should update the documentation webview_attributes.environment
and related function, then mark NewWindowOpener.environment
as deprecated since we now pass them automatically now and you can't use a different one within the same data directory anyways
|
||
#[cfg(windows)] | ||
#[derive(Debug, PartialEq, Eq)] | ||
struct WebView2EnvironmentOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, this feels very error prune to me but I don't know if there's a better way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well this is just for the warning - we ALWAYS force the same environment for the same data directory
#[cfg(windows)] | ||
environment_options: WebView2EnvironmentOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe?
#[cfg(windows)] | |
environment_options: WebView2EnvironmentOptions, | |
#[cfg(all(debug_assertions, windows))] | |
environment_options: WebView2EnvironmentOptions, |
|
||
#[cfg(windows)] | ||
if let Some(environment) = &web_context.environment { | ||
webview_builder = webview_builder.with_environment(environment.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole environment thing is a bit confusing now, that we have it in on_new_window
and tell people to pass it in with with_environment
/window_features
, but we also have it done automatically here
Some documentation around the related attributes would be nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well i'm not sure if we should be explicit or not. like only validate the environment options matches? though in this case we get to that "error prune" validation issue..
since the on_new_window environment is all handled by window_features i think it's ok to keep it as is, so it's explicitly matched instead of relying on internal details
but i agree documentation should be improved anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well i'm not sure if we should be explicit or not. like only validate the environment options matches? though in this case we get to that "error prune" validation issue..
I think forcing the same environment makes sense here, no matter the options, it's too error prune the other way around
since the on_new_window environment is all handled by window_features i think it's ok to keep it as is, so it's explicitly matched instead of relying on internal details
If we want to force the environment, keeping it feels like some extra confusions, anyways, we can't remove it in v2 anyways, let's just update documentation
on Windows the Webview2 environment options must match when created webviews sharing the same data directory (unless specified, all webviews share the same data directory). This PR validates the options, warning if they do not match, and forces the webview to be created with the same previous environment to prevent a crash.
discovered as part of #14089