fix(window): persist and restore window size and position across restarts#16
fix(window): persist and restore window size and position across restarts#16DysektAI wants to merge 1 commit into
Conversation
…arts Only the window position was saved (and only at exit); the constructor always reset to 1080x720, so every restart lost the user's size and shifted the window (issue #11). - Add LastWindowWidth/LastWindowHeight config keys, round-tripped with the existing position keys. - Restore saved size and position at startup, but only when the saved caption strip still intersects an attached display's working area; a changed monitor layout falls back to default placement instead of resurrecting the window off-screen. - Save normal-mode bounds at exit: from minimal UI use the pre-minimal restore bounds, from maximized use RestoreBounds, otherwise live geometry. First-run minimal closes keep any previously saved size. - Exiting minimal UI with no in-session restore bounds (started directly into minimal mode) now also restores persisted bounds. Regression tests cover config round-trip, missing-key defaults, and on/off-screen visibility validation. Fixes #11
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| Rect bounds = | ||
| _isMinimalUi ? _restoreBounds | ||
| : WindowState == WindowState.Maximized ? RestoreBounds | ||
| : new Rect(Left, Top, Width, Height); |
There was a problem hiding this comment.
🟡 Window closed from the taskbar/tray while minimized saves a wrong position and size
When the app is closed while minimized, its live geometry is saved (new Rect(Left, Top, Width, Height) at src/App/PageSwitcher.xaml.cs:588) instead of the restore geometry, so the remembered location and size no longer match where the user actually had the window.
Impact: After minimizing the window (e.g. minimize-to-tray) and then exiting, the next launch does not reopen the window at its previous on-screen size/position and instead falls back to a default placement.
Why the minimized state is mishandled in PersistWindowBounds
PersistWindowBounds (src/App/PageSwitcher.xaml.cs:583-604) branches on minimal UI and WindowState.Maximized, using RestoreBounds for the maximized case. However WindowState.Minimized is not handled and falls into the final new Rect(Left, Top, Width, Height) branch. A minimized WPF window's Left/Top/Width/Height do not reflect the normal (restore) placement — RestoreBounds exists precisely to report the size/location before a window was minimized or maximized. This path is reachable: OnTitleBarMinimize sets WindowState = WindowState.Minimized (src/App/PageSwitcher.xaml.cs:661); with MinimizeToTray the window is then hidden (src/App/PageSwitcher.xaml.cs:185-186) and the user exits via the tray menu (OnContextMenuExitApplication → ExitApplication, src/App/PageSwitcher.xaml.cs:352), all while WindowState is still Minimized. The persisted position is then off-screen/garbage and gets rejected by IsVisibleOnAnyScreen on the next start, discarding the user's arrangement.
| Rect bounds = | |
| _isMinimalUi ? _restoreBounds | |
| : WindowState == WindowState.Maximized ? RestoreBounds | |
| : new Rect(Left, Top, Width, Height); | |
| Rect bounds = | |
| _isMinimalUi ? _restoreBounds | |
| : WindowState != WindowState.Normal ? RestoreBounds | |
| : new Rect(Left, Top, Width, Height); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| foreach (System.Windows.Forms.Screen screen in System.Windows.Forms.Screen.AllScreens) | ||
| { | ||
| System.Drawing.Rectangle area = screen.WorkingArea; | ||
| bool intersects = | ||
| grabRight > area.Left && grabLeft < area.Right && grabBottom > area.Top && grabTop < area.Bottom; |
There was a problem hiding this comment.
🔍 Off-screen validation compares WPF DIP coordinates against physical-pixel screen bounds
IsVisibleOnAnyScreen (src/App/PageSwitcher.xaml.cs:142-160) reads screen.WorkingArea from System.Windows.Forms.Screen, which is expressed in physical device pixels, and compares it against left/top/width/height that originate from WPF Left/Top/Width/Height (device-independent units) via the persisted LastWindowPositionX/Y and restored Width/Height. On non-100% DPI setups these two coordinate spaces diverge. On single-monitor high-DPI this rarely fully excludes a valid window (screens are large), but in multi-monitor layouts where a secondary display sits at a large physical X/Y offset, a genuinely on-screen saved position can be scaled differently and wrongly rejected (falling back to default placement) or an off-screen one wrongly accepted. The previous code restored the saved position without this validation, so DIP→DIP restoration was self-consistent; the new physical-vs-DIP comparison introduces the mismatch. Worth confirming whether saved positions and screen bounds should be reconciled in one coordinate space (e.g. via PresentationSource/CompositionTarget transform or per-monitor DPI).
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (bounds.IsEmpty || bounds.Width < MinimumWidth || bounds.Height < MinimumHeight) | ||
| { | ||
| // No trustworthy normal-mode geometry (e.g. first run closed while | ||
| // still in minimal UI): keep the previous saved size, only track | ||
| // the position as before. | ||
| RatConfig.LastWindowPositionX = (int)Left; | ||
| RatConfig.LastWindowPositionY = (int)Top; | ||
| return; |
There was a problem hiding this comment.
📝 Info: Minimal-UI-at-exit fallback persists the shifted overlay position
When the app is closed while in minimal UI with no in-session restore bounds (_restoreBounds empty, e.g. started directly into minimal mode), PersistWindowBounds (src/App/PageSwitcher.xaml.cs:590-597) falls back to saving (int)Left/(int)Top. In minimal mode Left was shifted left by half the overlay width (src/App/PageSwitcher.xaml.cs:532-533), so the persisted position reflects the compact overlay, not a normal-window top-left. On the next normal launch the window would restore to that shifted coordinate. This matches the prior behavior (the old code always saved live Left/Top), so it is not a regression, but the saved position is not the user's normal-mode arrangement.
Was this helpful? React with 👍 or 👎 to provide feedback.
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| internal static bool IsVisibleOnAnyScreen(double left, double top, double width, double height) | ||
| { | ||
| // The title bar is the reliable grab strip; require most of it plus a | ||
| // minimal slice of the window body to land on some screen. | ||
| double grabLeft = left + Math.Min(40, width / 4); | ||
| double grabRight = left + width - Math.Min(40, width / 4); | ||
| double grabTop = top; | ||
| double grabBottom = top + Math.Min(48, height); | ||
|
|
||
| foreach (System.Windows.Forms.Screen screen in System.Windows.Forms.Screen.AllScreens) | ||
| { | ||
| System.Drawing.Rectangle area = screen.WorkingArea; | ||
| bool intersects = | ||
| grabRight > area.Left && grabLeft < area.Right && grabBottom > area.Top && grabTop < area.Bottom; | ||
| if (intersects) | ||
| return true; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Program.Main calls Application.SetHighDpiMode(HighDpiMode.PerMonitorV2), which means System.Windows.Forms.Screen.WorkingArea returns physical device pixels. WPF's Window.Left/Top/Width/Height — the values passed as left, top, width, height — are in WPF device-independent units (96 DPI base). At 150% DPI, 1 WPF unit = 1.5 physical pixels.
Concrete failure: a user's secondary 1920 px-wide monitor at 150% DPI is 1280 WPF units wide. They position the window at WPF Left = 1300 (barely off the right edge). Config saves 1300. They unplug the monitor. On next launch IsVisibleOnAnyScreen(1300, …) computes grabLeft ≈ 1340 (WPF units) and compares against area.Right = 1920 (physical pixels). 1340 < 1920 is true, so the window is accepted as on-screen and restored to a physically inaccessible position — the exact regression issue #11 was meant to cure.
One approach: obtain the device pixel ratio from the PresentationSource (available on this in the instance context) and scale the WPF coordinates before comparing, or convert each Screen.WorkingArea rectangle from physical pixels to WPF units by dividing by the appropriate DPI scale factor.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/App/PageSwitcher.xaml.cs
Line: 142-159
Comment:
**Coordinate-system mismatch breaks off-screen detection at non-100% DPI**
`Program.Main` calls `Application.SetHighDpiMode(HighDpiMode.PerMonitorV2)`, which means `System.Windows.Forms.Screen.WorkingArea` returns **physical device pixels**. WPF's `Window.Left`/`Top`/`Width`/`Height` — the values passed as `left`, `top`, `width`, `height` — are in **WPF device-independent units** (96 DPI base). At 150% DPI, 1 WPF unit = 1.5 physical pixels.
Concrete failure: a user's secondary 1920 px-wide monitor at 150% DPI is 1280 WPF units wide. They position the window at WPF `Left = 1300` (barely off the right edge). Config saves `1300`. They unplug the monitor. On next launch `IsVisibleOnAnyScreen(1300, …)` computes `grabLeft ≈ 1340` (WPF units) and compares against `area.Right = 1920` (physical pixels). `1340 < 1920` is true, so the window is accepted as on-screen and restored to a physically inaccessible position — the exact regression issue #11 was meant to cure.
One approach: obtain the device pixel ratio from the `PresentationSource` (available on `this` in the instance context) and scale the WPF coordinates before comparing, or convert each `Screen.WorkingArea` rectangle from physical pixels to WPF units by dividing by the appropriate DPI scale factor.
How can I resolve this? If you propose a fix, please make it concise.| public void On_screen_position_is_accepted() | ||
| { | ||
| // Center of the primary working area must be restorable on any machine | ||
| // that can run the test suite (Windows desktop session). | ||
| System.Drawing.Rectangle area = System.Windows.Forms.Screen.PrimaryScreen!.WorkingArea; | ||
| double left = area.Left + (area.Width - 1080) / 2.0; | ||
| double top = area.Top + (area.Height - 720) / 2.0; | ||
|
|
||
| Assert.True(PageSwitcher.IsVisibleOnAnyScreen(left, top, 1080, 720)); | ||
| } |
There was a problem hiding this comment.
Screen.PrimaryScreen.WorkingArea.Width returns physical device pixels under PerMonitorV2. The test feeds those values directly to IsVisibleOnAnyScreen as if they were WPF device-independent units. At 100% DPI (typical CI machine) the two scales are identical so the test passes, but on a 150% DPI developer machine area.Width could be 2880 px while the actual WPF-unit width of the same monitor is 1920. The computed left = (2880-1080)/2 = 900 looks plausible but represents a position physically at 1350 px — very different from where the WPF window would actually appear. The test exercises a path through the function but does not accurately model the coordinates that RestoreWindowBounds passes at runtime.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs
Line: 78-87
Comment:
**Test uses physical-pixel coordinates as WPF logical units**
`Screen.PrimaryScreen.WorkingArea.Width` returns physical device pixels under `PerMonitorV2`. The test feeds those values directly to `IsVisibleOnAnyScreen` as if they were WPF device-independent units. At 100% DPI (typical CI machine) the two scales are identical so the test passes, but on a 150% DPI developer machine `area.Width` could be 2880 px while the actual WPF-unit width of the same monitor is 1920. The computed `left = (2880-1080)/2 = 900` looks plausible but represents a position physically at 1350 px — very different from where the WPF window would actually appear. The test exercises a path through the function but does not accurately model the coordinates that `RestoreWindowBounds` passes at runtime.
How can I resolve this? If you propose a fix, please make it concise.| private void PersistWindowBounds() | ||
| { | ||
| Rect bounds = | ||
| _isMinimalUi ? _restoreBounds | ||
| : WindowState == WindowState.Maximized ? RestoreBounds | ||
| : new Rect(Left, Top, Width, Height); | ||
|
|
||
| if (bounds.IsEmpty || bounds.Width < MinimumWidth || bounds.Height < MinimumHeight) | ||
| { | ||
| // No trustworthy normal-mode geometry (e.g. first run closed while | ||
| // still in minimal UI): keep the previous saved size, only track | ||
| // the position as before. | ||
| RatConfig.LastWindowPositionX = (int)Left; | ||
| RatConfig.LastWindowPositionY = (int)Top; | ||
| return; | ||
| } |
There was a problem hiding this comment.
When bounds.IsEmpty (first-run closed while still in minimal UI, meaning _restoreBounds was never set to valid normal-mode geometry), the fallback saves Left/Top of the minimal UI overlay as LastWindowPositionX/Y. On the next launch, if LastWindowMode is Normal, RestoreWindowBounds will position the full-size window at the compact overlay's coordinates (which could be a corner of the screen). The position is then validated by IsVisibleOnAnyScreen, so it at least won't restore off-screen, but the window will appear at an ergonomically surprising location. Skipping the position update in this branch (leaving LastWindowPositionX/Y as int.MinValue) would fall back to OS default placement, which is likely preferable to anchoring the normal window at an arbitrary minimal-UI corner.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/App/PageSwitcher.xaml.cs
Line: 583-598
Comment:
**Fallback writes the minimal-UI position as the normal-window position**
When `bounds.IsEmpty` (first-run closed while still in minimal UI, meaning `_restoreBounds` was never set to valid normal-mode geometry), the fallback saves `Left`/`Top` of the *minimal UI* overlay as `LastWindowPositionX/Y`. On the next launch, if `LastWindowMode` is `Normal`, `RestoreWindowBounds` will position the full-size window at the compact overlay's coordinates (which could be a corner of the screen). The position is then validated by `IsVisibleOnAnyScreen`, so it at least won't restore off-screen, but the window will appear at an ergonomically surprising location. Skipping the position update in this branch (leaving `LastWindowPositionX/Y` as `int.MinValue`) would fall back to OS default placement, which is likely preferable to anchoring the normal window at an arbitrary minimal-UI corner.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/App/PageSwitcher.xaml.cs">
<violation number="1" location="src/App/PageSwitcher.xaml.cs:142">
P2: `IsVisibleOnAnyScreen` compares WPF device-independent pixel (DIU) coordinates against WinForms `Screen.WorkingArea` physical pixels without scaling, causing the off-screen guard to be ineffective on high-DPI displays.
WPF `Left`/`Top`/`Width`/`Height` are in DIU (1/96"), while `System.Windows.Forms.Screen.WorkingArea` returns physical pixels. At 200% scaling on a 3840×2160 monitor (1920×1080 DIU view), a window at Left=1900 DIU (physical pixel 3800) is off-screen, but `IsVisibleOnAnyScreen` compares `grabLeft=1940` DIU against `area.Right=3840` physical pixels (1940 < 3840 → true) and incorrectly reports the window as visible.
The app explicitly enables `HighDpiMode.PerMonitorV2` (in Program.cs) and already has `DisplayCoordinateConverter` / `GetDpiScale` for DPI-aware coordinate conversion elsewhere in the codebase, so the conversion infrastructure exists.
**Impact:** When a user unplugs a high-DPI external monitor, the off-screen rejection intended by this change may silently accept a position that is actually off-screen, causing the window to appear on a display that no longer exists or outside the working area.</violation>
<violation number="2" location="src/App/PageSwitcher.xaml.cs:572">
P3: `OnClosed` calls `ExitApplication()` which calls `Application.Current.Shutdown()`. When the application exits via the tray menu's Exit command, `OnContextMenuExitApplication` → `ExitApplication()` already calls `Shutdown()`, which triggers `OnClosed` again, leading to a redundant second call to `PersistWindowBounds()` and `RatConfig.SaveConfig()`.
At that point the window's `Left`/`Top`/`Width`/`Height` are still accessible, so the data is correct — but it's an unnecessary double-write that could be avoided by guarding `ExitApplication()` against re-entry or removing the call from `OnClosed` entirely (since `Shutdown()` in the current `ExitApplication` already handles window lifecycle). The redundancy was pre-existing but the new `PersistWindowBounds` / `SaveConfig` logic makes the redundant write more noticeable because it now writes four values per call instead of two.</violation>
<violation number="3" location="src/App/PageSwitcher.xaml.cs:587">
P2: Exiting from the taskbar or tray while minimized can persist minimized/iconic geometry instead of the user's normal window placement. Use `RestoreBounds` for every non-`Normal` `WindowState`, not only `Maximized`.</violation>
<violation number="4" location="src/App/PageSwitcher.xaml.cs:595">
P1: Closing after starting directly in minimal UI overwrites the saved normal position with the compact overlay's position. The next launch treats that coordinate as the main-window origin and re-anchors the overlay again, so retain both saved position and size when `_restoreBounds` is empty.</violation>
</file>
<file name="tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs">
<violation number="1" location="tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs:82">
P3: This test uses `Screen.PrimaryScreen.WorkingArea` which returns physical device pixels (under `PerMonitorV2` DPI mode), but feeds those values directly to `IsVisibleOnAnyScreen` as if they were WPF device-independent units. At 100% DPI (typical CI) the scales are identical so the test passes, but on a 150% DPI developer machine the computed position doesn't accurately represent where a WPF window would actually appear. The test exercises a path through the function but doesn't model the coordinate relationship that `RestoreWindowBounds` uses at runtime.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| RatConfig.LastWindowPositionX = (int)Left; | ||
| RatConfig.LastWindowPositionY = (int)Top; | ||
| return; |
There was a problem hiding this comment.
P1: Closing after starting directly in minimal UI overwrites the saved normal position with the compact overlay's position. The next launch treats that coordinate as the main-window origin and re-anchors the overlay again, so retain both saved position and size when _restoreBounds is empty.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/App/PageSwitcher.xaml.cs, line 595:
<comment>Closing after starting directly in minimal UI overwrites the saved normal position with the compact overlay's position. The next launch treats that coordinate as the main-window origin and re-anchors the overlay again, so retain both saved position and size when `_restoreBounds` is empty.</comment>
<file context>
@@ -513,12 +569,40 @@ private void AnchorNearTopRight(double rightEdge, double topEdge)
+ // No trustworthy normal-mode geometry (e.g. first run closed while
+ // still in minimal UI): keep the previous saved size, only track
+ // the position as before.
+ RatConfig.LastWindowPositionX = (int)Left;
+ RatConfig.LastWindowPositionY = (int)Top;
+ return;
</file context>
| RatConfig.LastWindowPositionX = (int)Left; | |
| RatConfig.LastWindowPositionY = (int)Top; | |
| return; | |
| return; |
| { | ||
| Rect bounds = | ||
| _isMinimalUi ? _restoreBounds | ||
| : WindowState == WindowState.Maximized ? RestoreBounds |
There was a problem hiding this comment.
P2: Exiting from the taskbar or tray while minimized can persist minimized/iconic geometry instead of the user's normal window placement. Use RestoreBounds for every non-Normal WindowState, not only Maximized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/App/PageSwitcher.xaml.cs, line 587:
<comment>Exiting from the taskbar or tray while minimized can persist minimized/iconic geometry instead of the user's normal window placement. Use `RestoreBounds` for every non-`Normal` `WindowState`, not only `Maximized`.</comment>
<file context>
@@ -513,12 +569,40 @@ private void AnchorNearTopRight(double rightEdge, double topEdge)
+ {
+ Rect bounds =
+ _isMinimalUi ? _restoreBounds
+ : WindowState == WindowState.Maximized ? RestoreBounds
+ : new Rect(Left, Top, Width, Height);
+
</file context>
| : WindowState == WindowState.Maximized ? RestoreBounds | |
| : WindowState != WindowState.Normal ? RestoreBounds |
| /// True when enough of the window (roughly the caption strip) intersects | ||
| /// the working area of at least one attached display for the user to grab it. | ||
| /// </summary> | ||
| internal static bool IsVisibleOnAnyScreen(double left, double top, double width, double height) |
There was a problem hiding this comment.
P2: IsVisibleOnAnyScreen compares WPF device-independent pixel (DIU) coordinates against WinForms Screen.WorkingArea physical pixels without scaling, causing the off-screen guard to be ineffective on high-DPI displays.
WPF Left/Top/Width/Height are in DIU (1/96"), while System.Windows.Forms.Screen.WorkingArea returns physical pixels. At 200% scaling on a 3840×2160 monitor (1920×1080 DIU view), a window at Left=1900 DIU (physical pixel 3800) is off-screen, but IsVisibleOnAnyScreen compares grabLeft=1940 DIU against area.Right=3840 physical pixels (1940 < 3840 → true) and incorrectly reports the window as visible.
The app explicitly enables HighDpiMode.PerMonitorV2 (in Program.cs) and already has DisplayCoordinateConverter / GetDpiScale for DPI-aware coordinate conversion elsewhere in the codebase, so the conversion infrastructure exists.
Impact: When a user unplugs a high-DPI external monitor, the off-screen rejection intended by this change may silently accept a position that is actually off-screen, causing the window to appear on a display that no longer exists or outside the working area.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/App/PageSwitcher.xaml.cs, line 142:
<comment>`IsVisibleOnAnyScreen` compares WPF device-independent pixel (DIU) coordinates against WinForms `Screen.WorkingArea` physical pixels without scaling, causing the off-screen guard to be ineffective on high-DPI displays.
WPF `Left`/`Top`/`Width`/`Height` are in DIU (1/96"), while `System.Windows.Forms.Screen.WorkingArea` returns physical pixels. At 200% scaling on a 3840×2160 monitor (1920×1080 DIU view), a window at Left=1900 DIU (physical pixel 3800) is off-screen, but `IsVisibleOnAnyScreen` compares `grabLeft=1940` DIU against `area.Right=3840` physical pixels (1940 < 3840 → true) and incorrectly reports the window as visible.
The app explicitly enables `HighDpiMode.PerMonitorV2` (in Program.cs) and already has `DisplayCoordinateConverter` / `GetDpiScale` for DPI-aware coordinate conversion elsewhere in the codebase, so the conversion infrastructure exists.
**Impact:** When a user unplugs a high-DPI external monitor, the off-screen rejection intended by this change may silently accept a position that is actually off-screen, causing the window to appear on a display that no longer exists or outside the working area.</comment>
<file context>
@@ -111,6 +107,58 @@ internal void ResetWindowSize()
+ /// True when enough of the window (roughly the caption strip) intersects
+ /// the working area of at least one attached display for the user to grab it.
+ /// </summary>
+ internal static bool IsVisibleOnAnyScreen(double left, double top, double width, double height)
+ {
+ // The title bar is the reliable grab strip; require most of it plus a
</file context>
| { | ||
| RatConfig.LastWindowPositionX = (int)Left; | ||
| RatConfig.LastWindowPositionY = (int)Top; | ||
| PersistWindowBounds(); |
There was a problem hiding this comment.
P3: OnClosed calls ExitApplication() which calls Application.Current.Shutdown(). When the application exits via the tray menu's Exit command, OnContextMenuExitApplication → ExitApplication() already calls Shutdown(), which triggers OnClosed again, leading to a redundant second call to PersistWindowBounds() and RatConfig.SaveConfig().
At that point the window's Left/Top/Width/Height are still accessible, so the data is correct — but it's an unnecessary double-write that could be avoided by guarding ExitApplication() against re-entry or removing the call from OnClosed entirely (since Shutdown() in the current ExitApplication already handles window lifecycle). The redundancy was pre-existing but the new PersistWindowBounds / SaveConfig logic makes the redundant write more noticeable because it now writes four values per call instead of two.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/App/PageSwitcher.xaml.cs, line 572:
<comment>`OnClosed` calls `ExitApplication()` which calls `Application.Current.Shutdown()`. When the application exits via the tray menu's Exit command, `OnContextMenuExitApplication` → `ExitApplication()` already calls `Shutdown()`, which triggers `OnClosed` again, leading to a redundant second call to `PersistWindowBounds()` and `RatConfig.SaveConfig()`.
At that point the window's `Left`/`Top`/`Width`/`Height` are still accessible, so the data is correct — but it's an unnecessary double-write that could be avoided by guarding `ExitApplication()` against re-entry or removing the call from `OnClosed` entirely (since `Shutdown()` in the current `ExitApplication` already handles window lifecycle). The redundancy was pre-existing but the new `PersistWindowBounds` / `SaveConfig` logic makes the redundant write more noticeable because it now writes four values per call instead of two.</comment>
<file context>
@@ -513,12 +569,40 @@ private void AnchorNearTopRight(double rightEdge, double topEdge)
{
- RatConfig.LastWindowPositionX = (int)Left;
- RatConfig.LastWindowPositionY = (int)Top;
+ PersistWindowBounds();
RatConfig.SaveConfig();
Application.Current.Shutdown();
</file context>
| { | ||
| // Center of the primary working area must be restorable on any machine | ||
| // that can run the test suite (Windows desktop session). | ||
| System.Drawing.Rectangle area = System.Windows.Forms.Screen.PrimaryScreen!.WorkingArea; |
There was a problem hiding this comment.
P3: This test uses Screen.PrimaryScreen.WorkingArea which returns physical device pixels (under PerMonitorV2 DPI mode), but feeds those values directly to IsVisibleOnAnyScreen as if they were WPF device-independent units. At 100% DPI (typical CI) the scales are identical so the test passes, but on a 150% DPI developer machine the computed position doesn't accurately represent where a WPF window would actually appear. The test exercises a path through the function but doesn't model the coordinate relationship that RestoreWindowBounds uses at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs, line 82:
<comment>This test uses `Screen.PrimaryScreen.WorkingArea` which returns physical device pixels (under `PerMonitorV2` DPI mode), but feeds those values directly to `IsVisibleOnAnyScreen` as if they were WPF device-independent units. At 100% DPI (typical CI) the scales are identical so the test passes, but on a 150% DPI developer machine the computed position doesn't accurately represent where a WPF window would actually appear. The test exercises a path through the function but doesn't model the coordinate relationship that `RestoreWindowBounds` uses at runtime.</comment>
<file context>
@@ -0,0 +1,95 @@
+ {
+ // Center of the primary working area must be restorable on any machine
+ // that can run the test suite (Windows desktop session).
+ System.Drawing.Rectangle area = System.Windows.Forms.Screen.PrimaryScreen!.WorkingArea;
+ double left = area.Left + (area.Width - 1080) / 2.0;
+ double top = area.Top + (area.Height - 720) / 2.0;
</file context>
1 participant
Root cause
Only
LastWindowPositionX/Ywere persisted, and only at exit.PageSwitcher's constructor always calledResetWindowSize()(1080×720), so every restart discarded the user's size and re-anchored the position to a stale coordinate pair — the window shifted exactly as described in #11.Fix
LastWindowWidth/LastWindowHeightconfig keys, round-tripped alongside the existing position keys (backwards compatible; missing keys stay unset).IsVisibleOnAnyScreen). If the monitor layout changed since last run, the window falls back to default placement instead of appearing off-screen.RestoreBoundsValidation
WindowBoundsPersistenceTests: config round-trip, missing-key defaults, off-screen rejection, on-screen acceptance (209 passed total).Fixes #11
Summary by cubic
Persist and restore window size and position across restarts to stop the window from resetting to 1080×720 and shifting. Also avoids resurrecting the window off-screen after monitor changes.
LastWindowWidthandLastWindowHeightconfig keys; backward compatible when keys are missing.RestoreBounds; otherwise use live geometry. First-run minimal closes keep any previously saved size.Written for commit b4c815a. Summary will update on new commits.
Greptile Summary
This PR persists and restores window size (new
LastWindowWidth/LastWindowHeightconfig keys) alongside the existing position keys, and adds monitor-layout validation so the window falls back to default placement when the saved position lands off all currently-attached displays.RestoreWindowBounds/PersistWindowBoundsreplace the previous inline save/restore, correctly handling the maximized, minimal-UI, and first-run-minimal-mode edge cases.IsVisibleOnAnyScreenvalidates the caption strip againstScreen.AllScreensworking areas before restoring position; however, the app runsPerMonitorV2DPI awareness (set inProgram.Main) soScreen.WorkingAreareturns physical pixels while WPF window coordinates are device-independent units — these diverge at non-100% DPI and can cause off-screen positions to pass the check.Confidence Score: 3/5
The core persistence logic and edge-case handling are sound, but the off-screen guard compares WPF logical coordinates against physical-pixel screen bounds in a PerMonitorV2 process, which means the guard can silently accept off-screen positions on high-DPI machines.
The coordinate mismatch in IsVisibleOnAnyScreen directly undermines the fix for issue #11: on any machine running at a non-100% DPI scale, a window that was on a now-disconnected monitor can still pass the visibility check and be restored off-screen. The config round-trip and the minimal-UI/maximized branching logic are well-structured and backwards-compatibility is clean, but the DPI bug affects the one guard added specifically to prevent the reported problem.
Files Needing Attention: src/App/PageSwitcher.xaml.cs — specifically IsVisibleOnAnyScreen and the fallback position-save path in PersistWindowBounds; tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs — the on-screen acceptance test.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Config as RatConfig participant PS as PageSwitcher participant Screen as Screen.AllScreens note over PS: Startup PS->>Config: LoadConfig() PS->>PS: ResetWindowSize() (1080x720 defaults) PS->>Config: Read LastWindowWidth/Height/X/Y PS->>PS: RestoreWindowBounds() PS->>Screen: IsVisibleOnAnyScreen(left, top, width, height) Screen-->>PS: true / false alt position on-screen PS->>PS: "Left=saved, Top=saved, Width=saved, Height=saved" else position off-screen PS->>PS: keep default placement end note over PS: Enter Minimal UI PS->>PS: "_restoreBounds = RestoreBounds" PS->>PS: ShowMinimalUI() note over PS: Exit Minimal UI (ShowUI) alt _restoreBounds non-empty PS->>PS: restore from _restoreBounds else started in minimal mode (no in-session bounds) PS->>PS: ResetWindowSize() + RestoreWindowBounds() end note over PS: Exit / Close PS->>PS: PersistWindowBounds() alt closing from minimal UI and _restoreBounds empty PS->>Config: "LastWindowPositionX/Y = minimal UI Left/Top" else normal geometry PS->>Config: "LastWindowPositionX/Y/Width/Height = bounds" end PS->>Config: SaveConfig()Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(window): persist and restore window ..." | Re-trigger Greptile