Skip to content

fix(display/webkit): WindowExists unwraps the query's typed nil - #43

Merged
Snider merged 1 commit into
mainfrom
fix/window-exists-typed-nil
Aug 6, 2026
Merged

fix(display/webkit): WindowExists unwraps the query's typed nil#43
Snider merged 1 commit into
mainfrom
fix/window-exists-typed-nil

Conversation

@Snider

@Snider Snider commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

queryWindowByName wraps a miss as a typed-nil *WindowInfo in Result.Value; the interface nil-check in WindowExists therefore returned true for every name. Downstream, lthn/desktop's tear-off always took the warm-window path against windows that were never created and failed at window.set_url.

Fix unwraps the value; the new TestWindowHelpersBehaviour_WindowExists_Ugly registers the production miss shape and fails against the old code.

Gate: GOWORK=off go test ./display/webkit/... all green, vet clean, gofmt clean.

🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io

Summary by CodeRabbit

  • Bug Fixes

    • Corrected window detection so uncreated windows are no longer reported as existing.
    • Improved handling of unsuccessful or invalid window-query results.
  • Tests

    • Added regression coverage for cases where a window query returns an empty result.

queryWindowByName answers a miss with a typed-nil *WindowInfo inside the
Result, and a typed nil passes an interface nil-check — so WindowExists
said yes for every name that was ever asked about. Callers then took
warm-window paths (set_url, resize, restore) against windows that were
never created; lthn/desktop's tear-off failed on exactly this.

The new Ugly case registers a query handler answering the production
miss shape and pins the unwrap.

Co-Authored-By: Virgil <virgil@lethean.io>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 09721ba7-161a-4815-861e-b3605ffd3a76

📥 Commits

Reviewing files that changed from the base of the PR and between 86908f6 and 6c10906.

📒 Files selected for processing (2)
  • go/display/webkit/window_helpers.go
  • go/display/webkit/window_helpers_behaviour_test.go

📝 Walkthrough

Walkthrough

WindowExists now validates query results before returning true. It rejects failed queries, invalid result types, and typed-nil *window.WindowInfo values. A regression test verifies that a typed-nil result reports the window as absent.

Changes

Window existence validation

Layer / File(s) Summary
Validate window query results
go/display/webkit/window_helpers.go, go/display/webkit/window_helpers_behaviour_test.go
WindowExists checks query success, asserts *window.WindowInfo, and rejects nil pointers. The regression test covers a successful query containing a typed-nil result.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Snider
Snider merged commit 1027de0 into main Aug 6, 2026
1 of 2 checks passed
@Snider
Snider deleted the fix/window-exists-typed-nil branch August 6, 2026 11:29
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.

1 participant