Skip to content

fix: a few regressions from previous PRs - #10

Open
tomerqodo wants to merge 5 commits into
greptile_full_base_fix_a_few_regressions_from_previous_prs_pr10from
greptile_full_head_fix_a_few_regressions_from_previous_prs_pr10
Open

tomerqodo wants to merge 5 commits into
greptile_full_base_fix_a_few_regressions_from_previous_prs_pr10from
greptile_full_head_fix_a_few_regressions_from_previous_prs_pr10

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#10

@greptile-apps

greptile-apps Bot commented Jan 26, 2026

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

This PR attempts to fix regressions from previous changes by:

  • Renaming with_window_features to window_features across the codebase
  • Adding a re-export of Cookie type from tauri_runtime
  • Reorganizing code in the example app to move on_new_window callback into the desktop-only configuration block
  • Swapping the order of size and position checks in window_features method

However, a critical bug was introduced in crates/tauri-runtime-wry/src/lib.rs:1718 where the delete_cookie method incorrectly sends a SetCookie message instead of DeleteCookie, causing cookies to be set rather than deleted. Additionally, there are formatting inconsistencies with spacing after commas in the window_features method.

Confidence Score: 0/5

  • This PR introduces a critical functional bug that breaks cookie deletion
  • The delete_cookie method sends the wrong message type (SetCookie instead of DeleteCookie), which completely breaks the cookie deletion functionality. This is a critical regression that will cause cookies to be set when they should be deleted.
  • Critical attention required for crates/tauri-runtime-wry/src/lib.rs due to the cookie deletion bug. Minor formatting issues in crates/tauri/src/webview/webview_window.rs should also be addressed.

Important Files Changed

Filename Overview
crates/tauri-runtime-wry/src/lib.rs Critical bug: delete_cookie method incorrectly sends SetCookie message instead of DeleteCookie
crates/tauri/src/webview/mod.rs Updated import from local Cookie to tauri_runtime::Cookie re-export, updated method name in doc example
crates/tauri/src/webview/webview_window.rs Renamed with_window_features to window_features, swapped order of size/position checks, formatting issues with spacing
examples/api/src-tauri/src/lib.rs Moved on_new_window callback inside desktop-only config block, updated method name, improved window naming

Sequence Diagram

sequenceDiagram
    participant App as Application Code
    participant Builder as WebviewWindowBuilder
    participant Dispatcher as WryWebviewDispatcher
    participant Runtime as Webview Runtime
    
    Note over App,Runtime: Window Features Configuration
    App->>Builder: window_features(features)
    Builder->>Builder: Extract size from features
    Builder->>Builder: Extract position from features
    Builder->>Builder: Configure window_builder
    
    Note over App,Runtime: New Window Callback Flow
    App->>Builder: on_new_window(callback)
    Builder->>Builder: Store callback for new windows
    Runtime->>App: Trigger new window request
    App->>Builder: Create new WebviewWindowBuilder
    Builder->>Builder: Apply window_features(features)
    Builder->>Runtime: Build window
    
    Note over Dispatcher,Runtime: Cookie Operations (Bug Present)
    App->>Dispatcher: delete_cookie(cookie)
    Dispatcher->>Runtime: SetCookie message (WRONG!)
    Runtime->>Runtime: Sets cookie instead of deleting
Loading

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

*self.window_id.lock().unwrap(),
self.webview_id,
WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
WebviewMessage::SetCookie(cookie.into_owned()),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete_cookie is using SetCookie message instead of DeleteCookie - this will set the cookie instead of deleting it

Suggested change
WebviewMessage::SetCookie(cookie.into_owned()),
WebviewMessage::DeleteCookie(cookie.into_owned()),
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/tauri-runtime-wry/src/lib.rs
Line: 1718:1718

Comment:
`delete_cookie` is using `SetCookie` message instead of `DeleteCookie` - this will set the cookie instead of deleting it

```suggestion
        WebviewMessage::DeleteCookie(cookie.into_owned()),
```

How can I resolve this? If you propose a fix, please make it concise.

self.window_builder = self.window_builder.position(position.x, position.y);
pub fn window_features(mut self, features: NewWindowFeatures) -> Self {
if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inconsistent spacing after comma (extra spaces)

Suggested change
self.window_builder = self.window_builder.inner_size(size.width, size.height);
self.window_builder = self.window_builder.inner_size(size.width, size.height);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/tauri/src/webview/webview_window.rs
Line: 1317:1317

Comment:
inconsistent spacing after comma (extra spaces)

```suggestion
      self.window_builder = self.window_builder.inner_size(size.width, size.height);
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing space after comma

Suggested change
self.window_builder = self.window_builder.position(position.x,position.y);
self.window_builder = self.window_builder.position(position.x, position.y);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/tauri/src/webview/webview_window.rs
Line: 1321:1321

Comment:
missing space after comma

```suggestion
      self.window_builder = self.window_builder.position(position.x, position.y);
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants