-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat: add WebView::set_cookie and WebView::delete_cookie
#13661
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
Conversation
Package Changes Through 1511888There are 10 changes which include tauri with minor, tauri-cli with minor, @tauri-apps/cli with minor, tauri-utils with minor, tauri-bundler with minor, tauri-macos-sign with minor, tauri-runtime-wry with minor, tauri-runtime with minor, @tauri-apps/api with minor, tauri-plugin 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 |
WSH032
left a comment
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.
This PR is ready, but it needs to wait for the Wry PR to be merged and for Tauri to upgrade the Wry version.
| WebviewMessage::SetCookie(cookie) => { | ||
| if let Err(e) = webview.set_cookie(&cookie) { | ||
| log::error!("failed to set webview cookie: {e}"); | ||
| } | ||
| } | ||
|
|
||
| WebviewMessage::DeleteCookie(cookie) => { | ||
| if let Err(e) = webview.delete_cookie(&cookie) { | ||
| log::error!("failed to delete webview cookie: {e}"); | ||
| } | ||
| } | ||
|
|
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 have some questions. Why do some methods use a channel to wait for a Result<()> to return, while others simply ignore (log) errors?
|
Oops, something went wrong on Linux. Should we enable |
|
Waiting for this to be merged. Cant wait to test it out ❤️ |
WSH032
left a comment
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.
Also, I think we should re-export cookie crate at mod webview, otherwise users won't be able to construct Cookie instances directly.
lucasfernog
left a comment
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.
nice one!
| Message::Webview( | ||
| *self.window_id.lock().unwrap(), | ||
| self.webview_id, | ||
| WebviewMessage::DeleteCookie(cookie.clone().into_owned()), |
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 cookie.clone() here was forgotten to be removed.
| use serde::Serialize; | ||
| use tauri_macros::default_runtime; | ||
| pub use tauri_runtime::webview::{NewWindowFeatures, PageLoadEvent}; | ||
| pub use tauri_runtime::Cookie; |
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.
We used to expose this as a public API. Wouldn't removing it cause some issues?
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.
Yeah, we should probably bring it back
Edit: done in #14020
ref #12665
wry pr tauri-apps/wry#1569