fix: eliminate window flickering on Windows 11 launch (#1584)#1588
fix: eliminate window flickering on Windows 11 launch (#1584)#1588mrv0for0vandeta wants to merge 4 commits into
Conversation
) Replace minimize/unminimize cycle with direct focus calls to eliminate visible flickering while maintaining keyboard focus functionality. Changes: - Remove minimize/unminimize from Windows focus fix - Add atomic guard to prevent concurrent focus cycles - Increase CEF initialization delay to 500ms - Implement three-attempt focus strategy with exponential backoff - Add OPENHUMAN_DISABLE_FOCUS_FIX environment variable - Remove duplicate WindowEvent::CloseRequested handler - Update documentation with issue context and trade-offs The previous approach used window.minimize() + window.unminimize() to trigger WM_KILLFOCUS + WM_SETFOCUS events for CEF focus initialization. This worked but caused visible flickering that users reported as rapid flashing making the app unusable. The new approach uses direct set_focus() calls without window state changes. This eliminates flickering but may require users to click once in the text field on some systems. This trade-off is acceptable as flickering made the app completely unusable. Fixes tinyhumansai#1584
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReworks Windows cold-launch CEF focus handling to a guarded deferred set_focus task and widens the Tauri main-window close-to-hide handler to compile for macOS and Windows. Adds a poison-recovering test env-lock helper and updates many tests to use it. ChangesWindows Focus Fix and Main Window Event Handling
Test env-lock helper and test updates
Sequence Diagram(s)sequenceDiagram
participant AppInit as App Init
participant FocusTask as Deferred Focus Task
participant Window as Tauri Main Window
participant Webview as CEF Webview
AppInit->>AppInit: check FOCUS_FIX_RUNNING & OPENHUMAN_DISABLE_FOCUS_FIX
alt schedule focus fix
AppInit->>FocusTask: spawn after ~500ms
FocusTask->>Window: set_focus() attempt 1
FocusTask->>Webview: set_focus() attempt 1
FocusTask->>Window: set_focus() attempt 2
FocusTask->>Webview: set_focus() attempt 2
FocusTask->>Window: set_focus() attempt 3
FocusTask->>Webview: set_focus() attempt 3
else skip scheduling
AppInit->>AppInit: do not schedule focus fix
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
senamakel
left a comment
There was a problem hiding this comment.
some unit tests, failng do have a look
All test modules that use ENV_LOCK for serializing environment variable mutations now recover gracefully from mutex poisoning. When a test panics while holding the lock, subsequent tests can continue instead of cascading failures. Changes: - Add test_env_lock() helper in config module that recovers from poison - Add env_lock() helpers in local test modules (logging, lib, file_logging) - Replace all .lock().unwrap() calls with poison-recovering helpers - Prevents PoisonError cascades across 38 failing tests This fixes the test suite failures in: - openhuman::config::ops::tests - openhuman::local_ai::schemas::tests - openhuman::subconscious::executor::tests - openhuman::threads::ops::tests - openhuman::update::ops::tests - openhuman::credentials::cli::tests - openhuman::composio::periodic::tests - core::jsonrpc::tests
Summary
OPENHUMAN_DISABLE_FOCUS_FIXenvironment variable for user controlWindowEvent::CloseRequestedhandlerProblem
Users on Windows 11 reported that the app window flickers rapidly on launch (#1584), making the UI completely unusable. The flickering was caused by an aggressive minimize/unminimize cycle in the Windows CEF focus fix (lines 1686-1720 in
app/src-tauri/src/lib.rs). While this cycle successfully initialized keyboard focus, it created a visible flickering effect that users found unacceptable.Solution
Replaced the minimize/unminimize cycle with direct
set_focus()calls that don't change window state. This eliminates flickering while still attempting to initialize keyboard focus. The implementation includes:AtomicBoolOPENHUMAN_DISABLE_FOCUS_FIX=1Trade-off: Keyboard input may require one click on some systems, but this is acceptable as flickering made the app completely unusable.
Submission Checklist
Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy - N/A: This is a Windows-specific focus fix that requires manual testing on Windows 11 hardware. The change is isolated to startup behavior and doesn't affect testable business logic.
Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge. - Pending: Awaiting CI resultsCoverage matrix updated — added/removed/renamed feature rows in
docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change) - N/A: behaviour-only change - no new features added, only modified existing Windows focus fix behaviorAll affected feature IDs from the matrix are listed in the PR description under
## Related- N/A: No feature IDs affected, this is a bug fixNo new external network dependencies introduced (mock backend used per Testing Strategy) - N/A: No network dependencies added
Manual smoke checklist updated if this touches release-cut surfaces (
docs/RELEASE-MANUAL-SMOKE.md) - N/A: Does not touch release-cut surfacesLinked issue closed via
Closes #NNNin the## Relatedsection - Done: See Related section belowpnpm --filter openhuman-app format:check- Pending: Will run locally if neededpnpm typecheck- Pending: Will run locally if neededFocused tests: - N/A: Windows-specific behavior requires manual testing
Rust fmt/check (if changed): - Pending: Awaiting CI results
Tauri fmt/check (if changed): - Pending: Awaiting CI results
Impact
Runtime/platform impact: Windows 11 desktop only. No impact on macOS, Linux, mobile, web, or CLI.
Performance: Slightly longer startup delay (~580ms additional) but eliminates visible flickering. User experience significantly improved.
Security: No security implications. Atomic guard ensures thread safety.
Migration: No migration needed. Users can opt-out via environment variable if needed.
Compatibility: Maintains backward compatibility. The
show_main_windowfunction still uses unminimize for legitimate cases (showing from tray).Related
Closes: #1584
Follow-up PR(s)/TODOs: None. This is a complete fix for the reported issue.
AI Authored PR Metadata
Linear Issue
Commit & Branch
fix/windows-flickering-158434f20b60Validation Run
pnpm --filter openhuman-app format:check: Pending CIpnpm typecheck: Pending CIValidation Blocked
Behavior Changes
Parity Contract
show_main_windowfunction still uses unminimize for showing from trayDuplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Tests