-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat: add Webview::cookies and Webview::cookies_for_url()
#12665
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
e2ca406 to
bccd107
Compare
Package Changes Through b53ebb2There are 8 changes which include tauri with minor, tauri-runtime with minor, tauri-runtime-wry with minor, tauri-utils with patch, tauri-cli with minor, @tauri-apps/cli with minor, @tauri-apps/api with minor, tauri-bundler 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 |
FabianLars
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.
thank you!!
I chose not to expose wry::webview::cookies() as it seemed to have an implementation note that it didn't work on Android at the moment.
It would be fine to do the same in tauri as well (having this function return an empty vec with a platform-specific note)
crates/tauri/src/webview/mod.rs
Outdated
| Ok(()) | ||
| } | ||
|
|
||
| /// Returns all cookies for a specified URL including HTTP-only and secure cookies. |
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.
Can you add a note about the scheme requirements here? imo quite important
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've added a note. This actually seems more of a bug/runtime limitation than anything. I would imagine this is because the tauri:// protocol behaves like the the file:// protocol in browsers and doesn't actually expose a functional cookie store.
|
I've added the relevant note and the I've also gone in and fixed up my fustfmt and clippy issues. |
|
As I'm testing this more deeply, I'm finding some concerning going. Fetching cookies is occasionally hanging the entire process. I will try to dig in some more. |
|
So after testing a bit more I've narrowed down the source of the hangs. It seems to happen in the MacOS implemenation specifically getting caught up in wry's I've gone and made a PR over on wry to resolve it tauri-apps/wry#1486 |
|
Where are you meant to use this? I've tried using the pr in a |
I get the same hunging when I do it on windows.... |
tauri::Webview::cookies_by_url() and tauri::CookieWebview::cookies and Webview::cookies_for_url()
|
the issue on Windows is similar to window creation, which must be done on a separate thread because we need to wait for an async operation on the webview side. see tauri-apps/wry#583 |
Got it. Thank you for the docs and for the example file. |
|
the only gotcha here is that we're re-exporting the cookie crate which is on 0.18 meaning we can't update it to 0.19 for instance without it being flagged as a breaking change, i'll at least update our toml for this |
|
alternatively we could do the same we do in with_webview and document that this may change between minor versions. Seeing the crate's activity we probably don't have to worry about new 0.x versions in the near future. |
|
Finally we will have a way to get the cookies for the websites. If wry allows maybe in the future it would be nice to also have a set cookie method. I know for a fact that I am gonna use it... Thank you so much for adding support for this @lucasfernog . I know it doesnt seem much but for my use case I am heavly dependant on cookie access and I was actually deciding if it would be better to switch to electron and you added this nice PR. |
|
thanks to @charrondev for bringing the bindings here and @amrbashir for working on the wry implementations |
The issue was introduced in cedb24d (tauri-apps#12665). As the `.cookies()` docs state, the command should be async.
Remove the cookies code from it. Firtly, they are not necessary: "Hello, world!" should be simple. Secondly, it's broken on Windows: `.cookies()` hangs. The cookies stuff was introduced recently, in cedb24d (tauri-apps#12665). This supersedes tauri-apps#12992.
Remove the cookies code from it. Firtly, they are not necessary: "Hello, world!" should be simple. Secondly, it's broken on Windows: `.cookies()` hangs. The cookies stuff was introduced recently, in cedb24d (tauri-apps#12665). This commit reverts the changes to the example. This supersedes tauri-apps#12992.
Remove the cookies code from it. Firtly, they are not necessary: "Hello, world!" should be simple. Secondly, it's broken on Windows: `.cookies()` hangs. The cookies stuff was introduced recently, in cedb24d (tauri-apps#12665). This commit reverts the changes to the example. This supersedes tauri-apps#12992.
Closes
Related
WebView::cookiesandWebView::cookies_for_urlwry#1394Related issues not fully handled by this PR:
Changes
Webview::cookies_for_url()andtauri::CookieImplementation notes
I'm open to any requested changes here, but here's how I've gone about this:
cookie::Cookieas a re-export fromtauri_runtime::Cookieandtauri::Cookie. Notably I did not re-export thewry::Cookiewhich is a re-export ofcookie::Cookiesincetauri_runtimedoes not have a dependency onwry.cookies_for_url()on the webview dispatcher and add an implementation for wry. I usedWebviewDispatcher::reparentas my template for a dispatch the takes a parameter and returns a result.wry::webview::cookies()as it seemed to have an implementation note that it didn't work on Android at the moment.Notes from my testing
Notably the wry support seems to only work when a site served over http (which is what I need it for). In my manual testing there doesn't seem to be a clear story for cookies on the
tauri://protocol (and there probably shouldn't be?). In any case by linking this PR into my own project I am able to extract the cookies from content served over http/https.