fix: a few regressions from previous PRs#23
Conversation
Code Review by Qodo
1. window_features not rustfmt-formatted
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1316
to
1322
| if let Some(size) = features.size() { | ||
| self.window_builder = self.window_builder.inner_size(size.width, size.height); | ||
| } | ||
|
|
||
| if let Some(size) = features.size() { | ||
| self.window_builder = self.window_builder.inner_size(size.width, size.height); | ||
| if let Some(position) = features.position() { | ||
| self.window_builder = self.window_builder.position(position.x,position.y); | ||
| } |
There was a problem hiding this comment.
1. window_features not rustfmt-formatted 📘 Rule violation ✓ Correctness
• The new window_features implementation contains irregular spacing (multiple spaces after commas and missing spaces after commas), which indicates cargo fmt would change the file. • This violates the requirement that Rust code must already match the project’s rustfmt configuration and pass cargo fmt --check without modifications.
Agent prompt
## Issue description
`window_features` contains spacing that likely fails the repository's rustfmt check.
## Issue Context
The code includes extra spaces after a comma and missing spaces after commas; rustfmt would reformat these lines.
## Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1315-1322]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+22
to
+23
| // Remove this re-export in v3 | ||
| pub use tauri_runtime::Cookie; |
There was a problem hiding this comment.
2. cookie re-export undocumented 📘 Rule violation ✧ Quality
• The PR introduces a new public re-export pub use tauri_runtime::Cookie; without an accompanying /// documentation comment. • This makes the public API harder to understand and violates the requirement that public API elements include documentation comments.
Agent prompt
## Issue description
A public re-export (`pub use tauri_runtime::Cookie;`) was added without Rust doc comments.
## Issue Context
Public API items should be self-documented for downstream users and docs generation.
## Fix Focus Areas
- crates/tauri/src/webview/mod.rs[21-23]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+92
to
+101
| tauri::WebviewUrl::External("about:blank".parse().unwrap()), | ||
| ) | ||
| .window_features(features) | ||
| .on_document_title_changed(|window, title| { | ||
| window.set_title(&title).unwrap(); | ||
| }) | ||
| .title(url.as_str()); | ||
|
|
||
| let window = builder.build().unwrap(); | ||
| tauri::webview::NewWindowResponse::Create { window } |
There was a problem hiding this comment.
3. New code uses unwrap() 📘 Rule violation ⛯ Reliability
• The added example code uses unwrap() on fallible operations (parse(), set_title, and build()), which can panic at runtime and hides error context. • The compliance requirement calls for returning/propagating Result for fallible operations rather than panicking, especially where failures should be handled gracefully.
Agent prompt
## Issue description
The new code panics via `unwrap()` on operations that can fail.
## Issue Context
Panics reduce robustness and remove useful context. Prefer `Result` propagation and explicit handling.
## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[89-101]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+84
to
+86
| .on_new_window(move |url, features| { | ||
| println!("new window requested: {url:?} {features:?}"); | ||
|
|
There was a problem hiding this comment.
4. println! logs url/features 📘 Rule violation ⛨ Security
• The new example logs url and features via println!, which is unstructured and may inadvertently expose sensitive data contained in URLs (e.g., query parameters or tokens). • Secure logging practices require logs to avoid sensitive data and be structured for auditing/monitoring.
Agent prompt
## Issue description
The new handler logs potentially sensitive URL data using unstructured `println!` output.
## Issue Context
URLs can contain secrets in query params/fragments; logs should be structured and avoid sensitive content.
## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[84-86]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+76
to
+77
| let app_ = app.handle().clone(); | ||
| let mut created_window_count = std::sync::atomic::AtomicUsize::new(0); |
There was a problem hiding this comment.
5. app_ variable name unclear 📘 Rule violation ✓ Correctness
• The newly introduced identifier app_ is ambiguous and does not clearly communicate what it represents (an app handle clone). • The compliance naming requirement expects identifiers to be self-documenting, especially for values captured into closures.
Agent prompt
## Issue description
`app_` is not a self-documenting name for a captured app handle.
## Issue Context
Clear names reduce cognitive overhead when reading closure captures and builder setup.
## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[76-77]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
1715
to
1721
| Message::Webview( | ||
| *self.window_id.lock().unwrap(), | ||
| self.webview_id, | ||
| WebviewMessage::DeleteCookie(cookie.clone().into_owned()), | ||
| WebviewMessage::SetCookie(cookie.into_owned()), | ||
| ), | ||
| )?; | ||
| Ok(()) |
There was a problem hiding this comment.
6. Delete_cookie sets cookie 🐞 Bug ✓ Correctness
• WryWebviewDispatcher::delete_cookie dispatches WebviewMessage::SetCookie instead of WebviewMessage::DeleteCookie, so callers cannot delete cookies anymore. • This is a behavior regression: the runtime event loop still has a dedicated DeleteCookie branch that calls webview.delete_cookie, but it is never reached from delete_cookie. • Impact: any code using Webview::delete_cookie will silently behave incorrectly (cookie persists/gets overwritten instead of removed).
Agent prompt
### Issue description
`WryWebviewDispatcher::delete_cookie` currently dispatches `WebviewMessage::SetCookie`, making `delete_cookie` behave like `set_cookie` and preventing cookie deletion.
### Issue Context
`WebviewMessage` includes a `DeleteCookie` variant and the message loop handles it by calling `webview.delete_cookie(&cookie)`, but the dispatcher never sends that variant.
### Fix Focus Areas
- crates/tauri-runtime-wry/src/lib.rs[1712-1721]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#170