Skip to content

Fix focusedWaveWindow (also fixes bug w/ save session as....)#2987

Merged
sawka merged 3 commits intomainfrom
sawka/fix-savesession
Mar 5, 2026
Merged

Fix focusedWaveWindow (also fixes bug w/ save session as....)#2987
sawka merged 3 commits intomainfrom
sawka/fix-savesession

Conversation

@sawka
Copy link
Member

@sawka sawka commented Mar 5, 2026

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 32c60c5d-de16-47f4-a6cf-58fc30773cfe

📥 Commits

Reviewing files that changed from the base of the PR and between 185bf62 and 160c37a.

📒 Files selected for processing (1)
  • emain/emain-ipc.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • emain/emain-ipc.ts

Walkthrough

IPC handlers for saving images and text were changed to resolve the parent BrowserWindow dynamically using BrowserWindow.fromWebContents(event.sender) (or event.sender.hostWebContents) instead of relying on a previously imported focused window. The function saveImageFileWithNativeDialog now accepts a sender parameter and resolves the target window from it with null-checks. All IPC call sites were updated to pass the appropriate sender. The webview-image-contextmenu IPC path and save-text-file handler were updated accordingly. Separately, the WaveBrowserWindow focus handler now assigns the global focusedWaveWindow = this when a window gains focus.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author, so this check cannot assess the relevance of the description to the changeset. Add a pull request description explaining the changes, their motivation, and any related bugs being fixed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title refers to fixing focusedWaveWindow and mentions fixing a bug with save session, which aligns with the changeset's focus on replacing direct focusedWindow references with dynamic window resolution.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/fix-savesession

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 and usage tips.

@sawka sawka changed the title fix bug w/ save session as.... Fix focusedWaveWindow (also fixes bug w/ save session as....) Mar 5, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@emain/emain-ipc.ts`:
- Around line 133-145: In saveImageFileWithNativeDialog, the code returns when
electron.BrowserWindow.fromWebContents(sender) yields null without disposing
readStream; call readStream.destroy() (or readStream.close() if supported)
before returning to free buffered data and listeners. Update the ww == null
branch in saveImageFileWithNativeDialog to destroy the Readable
(readStream.destroy()) and then return, ensuring no leaked resources.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 080a430d-eeff-48e8-a48f-95b0cb95326c

📥 Commits

Reviewing files that changed from the base of the PR and between 61d8445 and 185bf62.

📒 Files selected for processing (2)
  • emain/emain-ipc.ts
  • emain/emain-window.ts

@kilo-code-bot
Copy link
Contributor

kilo-code-bot bot commented Mar 5, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

Notes

The PR correctly fixes the focusedWaveWindow issue by dynamically resolving the BrowserWindow from the IPC sender using electron.BrowserWindow.fromWebContents() instead of relying on a global variable. This resolves the save session issue where the wrong window could be targeted.

The existing CodeRabbit comment (line 146) regarding readStream resource cleanup has been addressed - the code now properly calls readStream.destroy() when the window is null (line 144) and when the dialog is canceled (line 173).

Files Reviewed (2 files)
  • emain/emain-ipc.ts - Issue resolved
  • emain/emain-window.ts - no issues

@sawka sawka merged commit f59fdb5 into main Mar 5, 2026
6 checks passed
@sawka sawka deleted the sawka/fix-savesession branch March 5, 2026 19:33
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