From 2ddcf3b921a674cfdf3fe92ce546dadba64a72c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Tue, 22 Sep 2026 13:23:13 +0200 Subject: [PATCH] [client] Hand covered windows over when a popup closes under another Closing the browser-login popup while the install-progress popup was still up restored the main window the login had hidden, even though the install popup was meant to own the screen until it finished. The owner tag on each hidden entry only stops a popup from restoring another's windows; it says nothing about what to do with its own when a second popup still covers them. Track which popups currently own the screen and, on restore, re-tag the entries another live popup covers to that popup instead of showing them. A popup is never handed its own window, so closing the popup on top still brings the one below back. --- client/ui/services/windowmanager.go | 29 ++++++++-- client/ui/services/windowmanager_test.go | 73 ++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go index 0ac745ae7..af6d726a3 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -236,6 +236,7 @@ type WindowManager struct { // hiddenWindows holds windows hidden while a popup owns the screen, each tagged with // the popup that hid it so closing one popup cannot restore what another still hides. hiddenWindows []hiddenWindow + hiding map[string]bool // allWindows and raiseMain are the seams the hide/restore tests replace; both are nil // in production, where the Wails app and the platform helper are used directly. allWindows func() []hideableWindow @@ -279,6 +280,7 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo pendingOps: map[string][]windowOp{}, pendingClose: map[string]windowCloser{}, restoreGen: map[string]uint64{}, + hiding: map[string]bool{}, painted: map[uint]bool{}, mounted: map[uint]bool{}, showPending: map[uint]bool{}, @@ -1244,6 +1246,7 @@ func (s *WindowManager) retitleAll() { // the record, in which case the windows are re-shown rather than stranded. func (s *WindowManager) hideOtherWindows(keepName string) { s.mu.Lock() + s.hiding[keepName] = true gen := s.restoreGen[keepName] s.mu.Unlock() @@ -1275,13 +1278,14 @@ func (s *WindowManager) hideOtherWindows(keepName string) { } } -// restoreHiddenWindows re-shows the windows owner hid, leaving those another popup still -// hides untouched. If the main window was among them, raiseToForeground lifts it above -// the SSO browser, which still owns the foreground — a plain Show/Focus would be demoted -// to a taskbar flash and leave it stranded behind. +// restoreHiddenWindows re-shows the windows owner hid, unless another popup still covers +// them, in which case they are handed to that popup. If the main window was among them, +// raiseToForeground lifts it above the SSO browser, which still owns the foreground — a +// plain Show/Focus would be demoted to a taskbar flash and leave it stranded behind. func (s *WindowManager) restoreHiddenWindows(owner string) { s.mu.Lock() mainWindow := s.mainWindow + delete(s.hiding, owner) var restore []hideableWindow kept := s.hiddenWindows[:0] for _, hidden := range s.hiddenWindows { @@ -1289,6 +1293,11 @@ func (s *WindowManager) restoreHiddenWindows(owner string) { kept = append(kept, hidden) continue } + if coverer, covered := s.coveringPopupLocked(hidden.win); covered { + hidden.owner = coverer + kept = append(kept, hidden) + continue + } if hidden.win != nil { restore = append(restore, hidden.win) } @@ -1309,6 +1318,18 @@ func (s *WindowManager) restoreHiddenWindows(owner string) { } } +func (s *WindowManager) coveringPopupLocked(w hideableWindow) (string, bool) { + if w == nil { + return "", false + } + for name := range s.hiding { + if name != w.Name() { + return name, true + } + } + return "", false +} + // getScreenBasedOnCursorPosition returns the cursor's display, falling back to the // main-window screen, then nil (OS-default placement). func (s *WindowManager) getScreenBasedOnCursorPosition() *application.Screen { diff --git a/client/ui/services/windowmanager_test.go b/client/ui/services/windowmanager_test.go index 489c7cc47..890fba24f 100644 --- a/client/ui/services/windowmanager_test.go +++ b/client/ui/services/windowmanager_test.go @@ -18,6 +18,7 @@ func newTestWindowManager() *WindowManager { pendingOps: map[string][]windowOp{}, pendingClose: map[string]windowCloser{}, restoreGen: map[string]uint64{}, + hiding: map[string]bool{}, generation: map[string]uint64{}, } } @@ -448,6 +449,78 @@ func TestInstallDuringLoginKeepsMainHiddenUntilLoginCloses(t *testing.T) { require.Empty(t, s.hiddenWindows) } +func TestLoginClosingUnderInstallHandsMainToInstall(t *testing.T) { + main := newFakeWindow(windowMain) + login := newFakeWindow(windowBrowserLogin) + install := newFakeWindow(windowInstallProgress) + install.visible = false + s := newTestWindowManager() + d := newFakeDesktop(s, main, login, install) + + s.hideOtherWindows(windowBrowserLogin) + install.visible = true + s.hideOtherWindows(windowInstallProgress) + require.False(t, main.visible) + require.False(t, login.visible, "the install popup hides the login popup") + + // The login popup closes while the install popup is still up: the main window it + // hid must not resurface under the install popup, it is handed over instead. + s.restoreHiddenWindows(windowBrowserLogin) + require.False(t, main.visible, "the install popup still covers the main window") + require.Equal(t, 0, d.raised) + require.Equal(t, []string{windowInstallProgress, windowInstallProgress}, ownersOf(s.hiddenWindows)) + + s.restoreHiddenWindows(windowInstallProgress) + require.True(t, main.visible, "the install popup restores the handed-over main window") + require.Equal(t, 1, d.raised) + require.Empty(t, s.hiddenWindows) +} + +func TestInstallClosingUnderLoginHandsMainToLogin(t *testing.T) { + main := newFakeWindow(windowMain) + install := newFakeWindow(windowInstallProgress) + login := newFakeWindow(windowBrowserLogin) + login.visible = false + s := newTestWindowManager() + d := newFakeDesktop(s, main, install, login) + + s.hideOtherWindows(windowInstallProgress) + login.visible = true + s.hideOtherWindows(windowBrowserLogin) + require.False(t, install.visible, "the login popup hides the install popup") + + s.restoreHiddenWindows(windowInstallProgress) + require.False(t, main.visible, "the login popup still covers the main window") + require.Equal(t, 0, d.raised) + require.Equal(t, []string{windowBrowserLogin, windowBrowserLogin}, ownersOf(s.hiddenWindows)) + + s.restoreHiddenWindows(windowBrowserLogin) + require.True(t, main.visible) + require.Equal(t, 1, d.raised) + require.Empty(t, s.hiddenWindows) +} + +func TestPopupClosingReshowsTheCoveringPopupItself(t *testing.T) { + main := newFakeWindow(windowMain) + install := newFakeWindow(windowInstallProgress) + login := newFakeWindow(windowBrowserLogin) + login.visible = false + s := newTestWindowManager() + d := newFakeDesktop(s, main, install, login) + + s.hideOtherWindows(windowInstallProgress) + login.visible = true + s.hideOtherWindows(windowBrowserLogin) + + // The login popup hid the install popup itself; closing the login popup must bring + // the install popup back rather than hand it over to its own owner. + s.restoreHiddenWindows(windowBrowserLogin) + require.True(t, install.visible, "a popup is never handed over to itself") + require.False(t, main.visible, "the main window stays with the install popup") + require.Equal(t, 0, d.raised) + require.Equal(t, []string{windowInstallProgress}, ownersOf(s.hiddenWindows)) +} + func TestRestoreHiddenWindowsUnknownOwnerKeepsEverything(t *testing.T) { main := newFakeWindow(windowMain) s := newTestWindowManager()