From b1c154dc3d7d2c5136d38b865b7f82d81098b5c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Tue, 15 Sep 2026 14:55:14 +0200 Subject: [PATCH] [client] Cover the hidden-window bookkeeping with tests application.Window carries unexported methods, so the hide/restore paths could not be faked and the earlier tests could only assert which entries survived a restore, never which windows were actually shown. The bookkeeping now goes through hideableWindow, the four methods it needs, with the window enumeration and the main-window raise behind seams that are nil in production. That makes the case the owner tag exists for testable end to end: an install started during SSO login restores only the login popup it hid, and leaves the main window hidden until the login popup itself closes. --- client/ui/services/windowmanager.go | 73 +++++++++++++-- client/ui/services/windowmanager_test.go | 112 ++++++++++++++++++++--- 2 files changed, 161 insertions(+), 24 deletions(-) diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go index a9b275517..a1db377cc 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -105,9 +105,19 @@ func DialogWindowOptions(name, title, url string, linuxIcon []byte) application. } } +// hideableWindow is the slice of application.Window the hide/restore bookkeeping needs. +// Narrow enough to fake in tests, which application.Window itself is not: it carries +// unexported methods. +type hideableWindow interface { + Show() application.Window + Hide() application.Window + IsVisible() bool + Name() string +} + // hiddenWindow records a window hidden by owner, the name of the popup that hid it. type hiddenWindow struct { - win application.Window + win hideableWindow owner string } @@ -126,9 +136,13 @@ 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 - mu sync.Mutex - createMu sync.Mutex - newMain func(startURL string) *application.WebviewWindow + // 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 + raiseMain func() + mu sync.Mutex + createMu sync.Mutex + newMain func(startURL string) *application.WebviewWindow // painted gates showing a window: set by the frontend's first render, or by the // fallback timer so a webview that never wakes up still becomes visible. painted map[uint]bool @@ -397,7 +411,7 @@ func (s *WindowManager) CloseRenewFlow() { if se != nil { kept := s.hiddenWindows[:0] for _, hidden := range s.hiddenWindows { - if hidden.win != application.Window(se) { + if !sameWindow(hidden.win, se) { kept = append(kept, hidden) } } @@ -764,7 +778,7 @@ func (s *WindowManager) forgetWindowLocked(w *application.WebviewWindow) { kept := s.hiddenWindows[:0] for _, hidden := range s.hiddenWindows { - if hidden.win != application.Window(w) { + if !sameWindow(hidden.win, w) { kept = append(kept, hidden) } } @@ -793,6 +807,35 @@ func (s *WindowManager) matchesGeneration(name string, gen uint64) bool { return want == gen } +func (s *WindowManager) hideableWindows() []hideableWindow { + if s.allWindows != nil { + return s.allWindows() + } + all := s.app.Window.GetAll() + windows := make([]hideableWindow, 0, len(all)) + for _, w := range all { + windows = append(windows, w) + } + return windows +} + +func (s *WindowManager) isMainWindow(w hideableWindow) bool { + if s.allWindows != nil { + return w != nil && w.Name() == "main" + } + return s.mainWindow != nil && sameWindow(w, s.mainWindow) +} + +func (s *WindowManager) raiseMainWindow() { + if s.raiseMain != nil { + s.raiseMain() + return + } + if s.mainWindow != nil { + raiseToForeground(s.mainWindow) + } +} + func (s *WindowManager) windowByName(name string) *application.WebviewWindow { s.mu.Lock() defer s.mu.Unlock() @@ -1034,7 +1077,7 @@ func (s *WindowManager) retitleAll() { // an earlier popup is skipped, leaving it tagged to the popup that actually hid it. // Caller must hold s.mu. func (s *WindowManager) hideOtherWindowsLocked(keepName string) { - for _, w := range s.app.Window.GetAll() { + for _, w := range s.hideableWindows() { if w == nil || w.Name() == keepName { continue } @@ -1062,13 +1105,13 @@ func (s *WindowManager) restoreHiddenWindowsLocked(owner string) { continue } hidden.win.Show() - if hidden.win == s.mainWindow { + if s.isMainWindow(hidden.win) { mainRestored = true } } s.hiddenWindows = kept - if mainRestored && s.mainWindow != nil { - raiseToForeground(s.mainWindow) + if mainRestored { + s.raiseMainWindow() } } @@ -1142,5 +1185,15 @@ func paintedGeneration(data any) uint64 { } } +// sameWindow reports whether a hidden entry refers to w, comparing through the interface +// so a nil entry never matches a live window. +func sameWindow(hidden hideableWindow, w *application.WebviewWindow) bool { + if hidden == nil || w == nil { + return false + } + other, ok := hidden.(*application.WebviewWindow) + return ok && other == w +} + // u32ptr returns a pointer to v, for the optional *uint32 Wails theme fields. func u32ptr(v uint32) *uint32 { return &v } diff --git a/client/ui/services/windowmanager_test.go b/client/ui/services/windowmanager_test.go index 0d83fe452..e82513d67 100644 --- a/client/ui/services/windowmanager_test.go +++ b/client/ui/services/windowmanager_test.go @@ -7,8 +7,52 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/wailsapp/wails/v3/pkg/application" ) +type fakeWindow struct { + name string + visible bool + shown int + hidden int +} + +func newFakeWindow(name string) *fakeWindow { + return &fakeWindow{name: name, visible: true} +} + +func (f *fakeWindow) Show() application.Window { + f.visible = true + f.shown++ + return nil +} + +func (f *fakeWindow) Hide() application.Window { + f.visible = false + f.hidden++ + return nil +} + +func (f *fakeWindow) IsVisible() bool { return f.visible } + +func (f *fakeWindow) Name() string { return f.name } + +func newTestWindowManager(windows ...*fakeWindow) (*WindowManager, *int) { + raised := 0 + s := &WindowManager{ + generation: map[string]uint64{}, + allWindows: func() []hideableWindow { + all := make([]hideableWindow, 0, len(windows)) + for _, w := range windows { + all = append(all, w) + } + return all + }, + raiseMain: func() { raised++ }, + } + return s, &raised +} + func ownersOf(hidden []hiddenWindow) []string { owners := make([]string, 0, len(hidden)) for _, h := range hidden { @@ -17,39 +61,79 @@ func ownersOf(hidden []hiddenWindow) []string { return owners } -func TestRestoreHiddenWindowsLockedOnlyReleasesItsOwn(t *testing.T) { - s := &WindowManager{ - hiddenWindows: []hiddenWindow{ - {owner: "browser-login"}, - {owner: "install-progress"}, - {owner: "browser-login"}, - }, - } +func TestHideOtherWindowsLockedSkipsKeepNameAndInvisible(t *testing.T) { + main := newFakeWindow("main") + settings := newFakeWindow("settings") + settings.visible = false + popup := newFakeWindow("browser-login") + s, _ := newTestWindowManager(main, settings, popup) + + s.hideOtherWindowsLocked("browser-login") + + assert.False(t, main.visible) + assert.Equal(t, 1, main.hidden) + assert.Equal(t, 0, settings.hidden, "an already hidden window must not be recorded") + assert.Equal(t, 0, popup.hidden, "the popup itself must stay visible") + assert.Equal(t, []string{"browser-login"}, ownersOf(s.hiddenWindows)) +} + +func TestInstallDuringLoginKeepsMainHiddenUntilLoginCloses(t *testing.T) { + main := newFakeWindow("main") + login := newFakeWindow("browser-login") + install := newFakeWindow("install-progress") + s, raised := newTestWindowManager(main, login, install) + + s.hideOtherWindowsLocked("browser-login") + require.False(t, main.visible) + require.False(t, install.visible) + + install.visible = true + s.hideOtherWindowsLocked("install-progress") + assert.False(t, login.visible, "the login popup is hidden by the install popup") s.restoreHiddenWindowsLocked("install-progress") + assert.True(t, login.visible, "the install popup restores the login popup it hid") + assert.False(t, main.visible, "the main window stays hidden for the login popup") + assert.Equal(t, 0, *raised) assert.Equal(t, []string{"browser-login", "browser-login"}, ownersOf(s.hiddenWindows)) s.restoreHiddenWindowsLocked("browser-login") + assert.True(t, main.visible) + assert.Equal(t, 1, *raised, "restoring the main window raises it above the SSO browser") assert.Empty(t, s.hiddenWindows) } func TestRestoreHiddenWindowsLockedUnknownOwnerKeepsEverything(t *testing.T) { - s := &WindowManager{ - hiddenWindows: []hiddenWindow{{owner: "browser-login"}}, - } + main := newFakeWindow("main") + s, raised := newTestWindowManager(main) + s.hideOtherWindowsLocked("browser-login") s.restoreHiddenWindowsLocked("welcome") + + assert.False(t, main.visible) assert.Equal(t, []string{"browser-login"}, ownersOf(s.hiddenWindows)) + assert.Equal(t, 0, *raised) +} + +func TestRestoreHiddenWindowsLockedWithoutMainDoesNotRaise(t *testing.T) { + settings := newFakeWindow("settings") + s, raised := newTestWindowManager(settings) + s.hideOtherWindowsLocked("browser-login") + + s.restoreHiddenWindowsLocked("browser-login") + + assert.True(t, settings.visible) + assert.Equal(t, 0, *raised) } func TestRestoreHiddenWindowsLockedEmptyIsNoop(t *testing.T) { - s := &WindowManager{} + s, _ := newTestWindowManager() require.NotPanics(t, func() { s.restoreHiddenWindowsLocked("browser-login") }) assert.Empty(t, s.hiddenWindows) } func TestStampGenerationTracksLatestPerWindow(t *testing.T) { - s := &WindowManager{generation: map[string]uint64{}} + s, _ := newTestWindowManager() first := s.stampGeneration("browser-login", "/#/dialog/browser-login") assert.Equal(t, "/#/dialog/browser-login?gen=1", first) @@ -62,7 +146,7 @@ func TestStampGenerationTracksLatestPerWindow(t *testing.T) { } func TestMatchesGenerationUntrackedWindowAccepts(t *testing.T) { - s := &WindowManager{generation: map[string]uint64{}} + s, _ := newTestWindowManager() assert.True(t, s.matchesGeneration("main", 0)) }