Fix window.maximized reverting to unmaximized on macOS when set with page.title#6712
Open
davidlawson wants to merge 1 commit into
Open
Fix window.maximized reverting to unmaximized on macOS when set with page.title#6712davidlawson wants to merge 1 commit into
window.maximized reverting to unmaximized on macOS when set with page.title#6712davidlawson wants to merge 1 commit into
Conversation
…page.title WindowService.update() and _onPageChanged() both call _scheduleWindowUpdate(), so a patch that changes both the window and the page title in one go schedules two sequential _updateWindow() runs. The second run is pure redundant re-application (_updateWindow always re-reads live property values), but if the first run already started an animated native operation - macOS NSWindow.zoom for maximizeWindow()/unmaximizeWindow() - the second run's isMaximized() guard can still read pre-animation state and reissue the same call while the window is mid-zoom. zoom: toggles an already-zoomed window, so page.window.maximized = True can silently animate itself right back to unmaximized moments after being applied. Coalesce calls to _scheduleWindowUpdate() that land before the previously-scheduled run has actually started, via an _updateScheduled flag reset when the run begins. This collapses any same-tick burst of calls into the single run _updateWindow was already designed to handle, without changing event-driven state refreshes for legitimate cases (user moves/focuses the window, etc).
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
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.
Description
Setting
page.window.maximized = Trueduring app startup on macOS intermittently causes the window to maximize and then immediately animate back to its unmaximized frame. In a real production app this happened on ~58% of launches (7 of 12 in a controlled loop); the failure rate scales with how much work is in the first patch, so trivial apps rarely show it while apps with a non-trivial first render show it often.Root cause
A typical startup sets
page.titleandpage.window.maximized = Truein the same synchronous block, so they land in a singlePATCH_CONTROLmessage. Applying that patch fires two independent listeners onWindowService:update()(window.dart:96) — the window control's own patch changed._onPageChanged()(window.dart:103) — a different control (the parent page) changed, specifically itstitle.Both call
_scheduleWindowUpdate(), so one patch enqueues two_updateWindow()runs chained on_pendingWindowUpdate.window.dart:318):maximized (true) != _maximized→await maximizeWindow()→ on macOS this iswindowManager.maximize()→NSWindow.zoom(_:), an animated resize (~300-400ms; seeWindowManager.swift:532windowShouldZoom/:540windowDidResizein thewindow_managerplugin, v0.5.2)._updateWindowalways re-reads every property live fromcontrol/parent— so run 2 is inherently redundant, replaying the exact same already-applied values. But it still executes, and its ownmaximizeWindow()/unmaximizeWindow()guards (isMaximized()) can read state from mid-animation, when the OS hasn't finished zooming yet.NSWindow.zoom(_:)is a toggle — calling it a second time on a window that's still (or already) zoomed animates it back to its pre-zoom frame. Whether the timing lines up for this to bite is what makes the bug intermittent.The two-runs-per-patch behavior is the actual bug:
_updateWindowwas clearly designed to be called once per settle-point (it reconciles every property from current live state in one pass), and nothing about a page-level title change needs its own separate pass when a window-level update for the same patch is already queued.Fix
_scheduleWindowUpdate()now coalesces calls that land before the previously-scheduled run has actually started (as opposed to completed), via an_updateScheduledflag reset at the top of the run:Because
update()and_onPageChanged()both fire synchronously while one patch is being applied, the second call reliably observes_updateScheduled == trueand no-ops — collapsing the pair into the single run_updateWindowwas already built to handle. A_scheduleWindowUpdate()call that arrives later, after the current run has already started, is unaffected: it's still chained via.then()onto the in-flight run's future, so it can't execute concurrently with (or interrupt) a native operation that's already in progress. Event-driven state refreshes for legitimate cases (the user drags/moves the window, focus changes, etc.) are untouched — this only removes the duplicate scheduling, not any event handling.Testing
No existing Dart test harness covers
WindowService(packages/flet/test/only hastransport/utilssuites, and this class wraps nativewindow_managerplatform-channel calls), so this was verified at the application level:maximized: trueis sent to the client exactly once per launch, yet the window visibly reverted on 7 of 12 launches — proving the bug is entirely client-side.maximized = True+ a moderately sized first render, to widen the race window) failed 2 of 8 runs and showed the tell-tale doublemaximizewindow event for a singlemaximized = Trueassignment on every run, pass or fail:dart analyze/dart format --output=nonepass on the changed file.Type of change
Checklist
website/sidebars.ymlfor breaking changes, removals, and deprecations, if applicable.Summary by Sourcery
Coalesce window update scheduling to prevent duplicate window state reconciliations for a single patch, avoiding intermittent maximize/unmaximize races on macOS when title and window properties change together.
Bug Fixes:
page.window.maximized = Trueon macOS from intermittently reverting to the previous unmaximized state when applied in the same patch aspage.titleby ensuring only one window update run is scheduled per patch.Enhancements:
_updateScheduledguard inWindowServiceto avoid redundant_updateWindowexecutions when multiple callers schedule updates from the same patch.Documentation:
page.titleandpage.window.maximizedare set together.