Fix Minimal Mode reverting on macOS when entered while maximized (#2365)#2367
Merged
Conversation
On macOS, AppKit delivers Qt::WindowStateChange asynchronously, so the Maximized/FullScreen bit could survive setFixedWidth(260) inside toggleMinimalMode(true). The deferred WindowStateChange then arrived with m_minimalMode==true and the Maximized bit still set, causing changeEvent to schedule a spurious toggleMinimalMode(false) — the window collapsed for a frame and snapped back to the full-size geometry restored from FullModeGeometry. - toggleMinimalMode(true): drop maximized/fullscreen state via showNormal() before forcing the applet width. FullModeGeometry is saved first, so exit can still restore the user's pre-minimal window. - Add m_enteringMinimalMode guard symmetric to m_exitingMinimalMode and honor it in changeEvent. Cleared via singleShot(0) so any AppKit- deferred WindowStateChange queued during the enter path drains through the early-return before the guard drops. Both behaviors are no-ops on Windows/Linux where setFixedWidth on a maximized top-level synchronously clears the state bit. Regressed in #2299, which collapsed minimal-exit paths into the single changeEvent branch and exposed the missing entering-side guard. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
There was a problem hiding this comment.
Looks good — thanks for the careful diagnosis and writeup.
The fix is well-targeted and the asymmetry analysis matches what I see in the code:
MainWindow.cpp:4201— addingm_enteringMinimalModetochangeEvent's early-return is the natural symmetric counterpart to the existingm_exitingMinimalModeguard atMainWindow.cpp:4213. The ordering is right too:!m_minimalModeshort-circuits ahead of the new flag, so the exit path doesn't have to clear it.MainWindow.cpp:9863—showNormal()runs aftersaveGeometry()is captured intoFullModeGeometry, so the exit path still restores the user's pre-minimal (maximized) window. Confirmed by the existing exit-side handling atMainWindow.cpp:9903-9908which independently checks the maximized bit on exit.- The deferred
QTimer::singleShot(0, ...)to clear the guard mirrors the existingm_exitingMinimalModeclear pattern and gives any AppKit-deferredWindowStateChangea chance to drain throughchangeEvent's early-return.
On Windows/Linux the new windowState() & (Qt::WindowMaximized | Qt::WindowFullScreen) check is effectively dead code on entry (synchronous state clearing), so the regression check on those platforms should be straightforward.
Minor non-blocking observation: there's a theoretical race if the user toggles minimal mode rapidly enough that a second enter happens before the first deferred clear fires — the first timer would clear the flag while the second enter is still in flight. This is unlikely in practice (it requires sub-event-loop human input timing) and the showNormal() defense-in-depth covers most of the surface anyway, so I don't think it needs addressing here.
Nice fix.
Collaborator
|
Claude here on Jeremy's behalf — merged via admin squash. Stale-code audit clean, root-cause precise (correctly identifies #2299's regression source), defense-in-depth fix is the right shape. Jeremy will smoke-test on macOS and follow up here if anything surfaces. 73, Jeremy KK7GWY & Claude (AI dev partner) |
Closed
2 tasks
aethersdr-agent Bot
added a commit
that referenced
this pull request
May 26, 2026
…. Principle XIV. Two changes in src/gui/MainWindow.cpp: 1. closeEvent now refreshes MinimalModeGeometry when quitting from Minimal Mode (guarded against maximized/fullscreen abnormal state to match toggleMinimalMode(false)). Previously the per-mode key was only written by toggleMinimalMode(false), so users who launched in Minimal Mode and quit without ever toggling out never updated it. 2. The constructor's auto-enter-minimal-mode and AetherialStrip restore blocks have moved from L1641-1648 to immediately after the geometry restore at L4847. The earlier call site caused toggleMinimalMode(true) to snapshot the default startup rect into FullModeGeometry (corrupting the previous session's saved rect) and to restoreGeometry(MinimalModeGeometry) against a window not yet placed on the correct screen. User report: drift only visible with FramelessWindowHint on Windows because native chrome lets the WM cache pos() and partially compensate for the missing per-mode save; Frameless removes that safety net so the bug surfaces. Fix addresses the root cause (no MinimalModeGeometry save on close + wrong-order restore on open), making position stable in both chrome modes. Risk note: the reorder preserves the m_enteringMinimalMode guard inside toggleMinimalMode (L13528/L13576) that hardens the macOS AppKit-deferred WindowStateChange path from #2367. Persisting MinimalModeGeometry inline alongside MainWindowGeometry keeps the per-mode key fresh atomically with the rest of the close- event state (Principle XIV: persist atomically — the AppSettings save() at end of closeEvent commits the full updated state in one shot, never partially). Blast radius: risk_score=0.358, 12 high-risk affected (top: AppSettings::instance, ThemeManager::instance, ThemeManager::applyStyleSheet). The downstream surface is the existing AppSettings/RadioModel calls closeEvent already makes; this commit adds one more setValue and a reordering, no new structural edges. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
2 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.
Fixes #2365.
Summary
On macOS, activating Minimal Mode from a maximized/fullscreen window made the window briefly collapse to applet width and immediately snap back to its full-size geometry. The
View → Minimal Modemenu item,Ctrl+M, and the title-bar minimal-mode button all exhibited the flash-then-revert behavior.Root cause
MainWindow::toggleMinimalMode(true)callssetFixedWidth(260)on the top-level window. On Windows/Linux this synchronously clearsQt::WindowMaximized/Qt::WindowFullScreenbefore the function returns. On macOS, AppKit delivers theWindowStateChangenotification asynchronously, so the Maximized bit survives into the deferred event.When
MainWindow::changeEventthen fires withm_minimalMode == trueand the Maximized bit still set, it schedules atoggleMinimalMode(false)because there is no entering-side re-entry guard symmetric tom_exitingMinimalMode. That handler restoresFullModeGeometry(which encoded the original maximized rect), so the window snaps back.This regressed in #2299 (commit
0270b78), which collapsed the multiple minimal-exit paths into the singlechangeEventbranch and exposed the missing entering-side guard.Fix
Two small, localized changes (
src/gui/MainWindow.cpp,src/gui/MainWindow.h):toggleMinimalMode(true), aftersaveGeometry()is captured intoFullModeGeometry, callshowNormal()if the window is currently maximized or fullscreen.FullModeGeometrypreserves the maximized rect, so exit-minimal still restores the user's pre-minimal window.m_enteringMinimalModeguard alongsidem_exitingMinimalMode. Set at the start oftoggleMinimalMode(true), cleared viaQTimer::singleShot(0)so any AppKit-deferredWindowStateChangequeued during the enter path drains throughchangeEvent's early-return before the guard drops.changeEventhonors the new flag at its early-return.Defense-in-depth — either change alone would resolve the symptom; the combination guards against any further AppKit-deferred notifications between
setFixedWidth(260)and the function returning.Risk
Low — additive change (one extra
showNormal()+ one boolean flag). On Windows/Linux thewindowState() & (Maximized|FullScreen)check is false at entry (synchronous state clear), so the new code path is a no-op.Test plan
Ctrl+M→ window stays in narrow minimal layout (does not revert).View → Minimal Modemenu item.Ctrl+M/ double-click title bar / maximize button → returns to the user's pre-minimal window.Generated by AetherClaude (automated agent for AetherSDR)