fix(omnibox): guard stale favorite updates and retain standalone callbacks
- commit
- 8ba7b367f3ac01754f24f19166033440bf3456cd
- author
- brice · · 2026-04-08 06:10 UTC
- parent
- a961ae7
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +249 -24
M internal/ui/component/omnibox.go +40 -3
| @@ -33,6 +33,11 @@ const ( | |||
| 33 | 33 | minGhostInputLength = 1 | |
| 34 | 34 | ) | |
| 35 | 35 | ||
| 36 | + | type favoriteRowIndicatorUpdate struct { | |
| 37 | + | Apply bool | |
| 38 | + | ShowStarSlot bool | |
| 39 | + | } | |
| 40 | + | ||
| 36 | 41 | // ViewMode distinguishes history search from favorites display. | |
| 37 | 42 | type ViewMode string | |
| 38 | 43 | ||
| @@ -2235,7 +2240,7 @@ func (o *Omnibox) toggleFavorite() { | |||
| 2235 | 2240 | ||
| 2236 | 2241 | // Update row CSS and show toast on GTK main thread | |
| 2237 | 2242 | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 2238 | - | o.updateRowFavoriteIndicator(idx, result.Added) | |
| 2243 | + | o.updateRowFavoriteIndicator(idx, s.URL, result.Added) | |
| 2239 | 2244 | if o.onToast != nil { | |
| 2240 | 2245 | o.onToast(ctx, result.Message, ToastSuccess) | |
| 2241 | 2246 | } | |
| @@ -2297,11 +2302,22 @@ func (o *Omnibox) toggleFavorite() { | |||
| 2297 | 2302 | ||
| 2298 | 2303 | // updateRowFavoriteIndicator updates a single row's favorite indicator CSS class. | |
| 2299 | 2304 | // Must be called from GTK main thread (via glib.IdleAdd). | |
| 2300 | - | func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) { | |
| 2305 | + | func (o *Omnibox) updateRowFavoriteIndicator(index int, expectedURL string, isFavorite bool) { | |
| 2301 | 2306 | row := o.listBox.GetRowAtIndex(index) | |
| 2302 | 2307 | if row == nil { | |
| 2303 | 2308 | return | |
| 2304 | 2309 | } | |
| 2310 | + | ||
| 2311 | + | o.mu.RLock() | |
| 2312 | + | mode := o.viewMode | |
| 2313 | + | suggestions := append([]Suggestion(nil), o.suggestions...) | |
| 2314 | + | o.mu.RUnlock() | |
| 2315 | + | ||
| 2316 | + | update := resolveFavoriteRowIndicatorUpdate(mode, index, expectedURL, suggestions, isFavorite) | |
| 2317 | + | if !update.Apply { | |
| 2318 | + | return | |
| 2319 | + | } | |
| 2320 | + | ||
| 2305 | 2321 | if isFavorite { | |
| 2306 | 2322 | row.AddCssClass("omnibox-row-favorite") | |
| 2307 | 2323 | } else { | |
| @@ -2310,7 +2326,9 @@ func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) { | |||
| 2310 | 2326 | if slot := favoriteStarSlotForRow(row); slot != nil { | |
| 2311 | 2327 | o.syncFavoriteStarSlot(slot, isFavorite) | |
| 2312 | 2328 | slot.QueueDraw() | |
| 2313 | - | slot.Show() | |
| 2329 | + | if update.ShowStarSlot { | |
| 2330 | + | slot.Show() | |
| 2331 | + | } | |
| 2314 | 2332 | } | |
| 2315 | 2333 | if child := row.GetChild(); child != nil { | |
| 2316 | 2334 | child.QueueDraw() | |
| @@ -2320,6 +2338,25 @@ func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) { | |||
| 2320 | 2338 | row.Show() | |
| 2321 | 2339 | } | |
| 2322 | 2340 | ||
| 2341 | + | func resolveFavoriteRowIndicatorUpdate( | |
| 2342 | + | mode ViewMode, | |
| 2343 | + | index int, | |
| 2344 | + | expectedURL string, | |
| 2345 | + | suggestions []Suggestion, | |
| 2346 | + | isFavorite bool, | |
| 2347 | + | ) favoriteRowIndicatorUpdate { | |
| 2348 | + | if mode != ViewModeHistory { | |
| 2349 | + | return favoriteRowIndicatorUpdate{} | |
| 2350 | + | } | |
| 2351 | + | if index < 0 || index >= len(suggestions) { | |
| 2352 | + | return favoriteRowIndicatorUpdate{} | |
| 2353 | + | } | |
| 2354 | + | if suggestions[index].URL != expectedURL { | |
| 2355 | + | return favoriteRowIndicatorUpdate{} | |
| 2356 | + | } | |
| 2357 | + | return favoriteRowIndicatorUpdate{Apply: true, ShowStarSlot: isFavorite} | |
| 2358 | + | } | |
| 2359 | + | ||
| 2323 | 2360 | // yankSelectedURL copies the URL of the selected item to clipboard. | |
| 2324 | 2361 | func (o *Omnibox) yankSelectedURL() { | |
| 2325 | 2362 | log := logging.FromContext(o.ctx) | |
| @@ -33,6 +33,11 @@ const ( | |||||
| 33 | minGhostInputLength = 1 | 33 | minGhostInputLength = 1 | ||
| 34 | ) | 34 | ) | ||
| 35 | 35 | ||||
| 36 | + | type favoriteRowIndicatorUpdate struct { | |||
| 37 | + | Apply bool | |||
| 38 | + | ShowStarSlot bool | |||
| 39 | + | } | |||
| 40 | + | ||||
| 36 | // ViewMode distinguishes history search from favorites display. | 41 | // ViewMode distinguishes history search from favorites display. | ||
| 37 | type ViewMode string | 42 | type ViewMode string | ||
| 38 | 43 | ||||
| @@ -2235,7 +2240,7 @@ func (o *Omnibox) toggleFavorite() { | |||||
| 2235 | 2240 | ||||
| 2236 | // Update row CSS and show toast on GTK main thread | 2241 | // Update row CSS and show toast on GTK main thread | ||
| 2237 | cb := glib.SourceFunc(func(_ uintptr) bool { | 2242 | cb := glib.SourceFunc(func(_ uintptr) bool { | ||
| 2238 | - | o.updateRowFavoriteIndicator(idx, result.Added) | 2243 | + | o.updateRowFavoriteIndicator(idx, s.URL, result.Added) |
| 2239 | if o.onToast != nil { | 2244 | if o.onToast != nil { | ||
| 2240 | o.onToast(ctx, result.Message, ToastSuccess) | 2245 | o.onToast(ctx, result.Message, ToastSuccess) | ||
| 2241 | } | 2246 | } | ||
| @@ -2297,11 +2302,22 @@ func (o *Omnibox) toggleFavorite() { | |||||
| 2297 | 2302 | ||||
| 2298 | // updateRowFavoriteIndicator updates a single row's favorite indicator CSS class. | 2303 | // updateRowFavoriteIndicator updates a single row's favorite indicator CSS class. | ||
| 2299 | // Must be called from GTK main thread (via glib.IdleAdd). | 2304 | // Must be called from GTK main thread (via glib.IdleAdd). | ||
| 2300 | - | func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) { | 2305 | + | func (o *Omnibox) updateRowFavoriteIndicator(index int, expectedURL string, isFavorite bool) { |
| 2301 | row := o.listBox.GetRowAtIndex(index) | 2306 | row := o.listBox.GetRowAtIndex(index) | ||
| 2302 | if row == nil { | 2307 | if row == nil { | ||
| 2303 | return | 2308 | return | ||
| 2304 | } | 2309 | } | ||
| 2310 | + | ||||
| 2311 | + | o.mu.RLock() | |||
| 2312 | + | mode := o.viewMode | |||
| 2313 | + | suggestions := append([]Suggestion(nil), o.suggestions...) | |||
| 2314 | + | o.mu.RUnlock() | |||
| 2315 | + | ||||
| 2316 | + | update := resolveFavoriteRowIndicatorUpdate(mode, index, expectedURL, suggestions, isFavorite) | |||
| 2317 | + | if !update.Apply { | |||
| 2318 | + | return | |||
| 2319 | + | } | |||
| 2320 | + | ||||
| 2305 | if isFavorite { | 2321 | if isFavorite { | ||
| 2306 | row.AddCssClass("omnibox-row-favorite") | 2322 | row.AddCssClass("omnibox-row-favorite") | ||
| 2307 | } else { | 2323 | } else { | ||
| @@ -2310,7 +2326,9 @@ func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) { | |||||
| 2310 | if slot := favoriteStarSlotForRow(row); slot != nil { | 2326 | if slot := favoriteStarSlotForRow(row); slot != nil { | ||
| 2311 | o.syncFavoriteStarSlot(slot, isFavorite) | 2327 | o.syncFavoriteStarSlot(slot, isFavorite) | ||
| 2312 | slot.QueueDraw() | 2328 | slot.QueueDraw() | ||
| 2313 | - | slot.Show() | 2329 | + | if update.ShowStarSlot { |
| 2330 | + | slot.Show() | |||
| 2331 | + | } | |||
| 2314 | } | 2332 | } | ||
| 2315 | if child := row.GetChild(); child != nil { | 2333 | if child := row.GetChild(); child != nil { | ||
| 2316 | child.QueueDraw() | 2334 | child.QueueDraw() | ||
| @@ -2320,6 +2338,25 @@ func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) { | |||||
| 2320 | row.Show() | 2338 | row.Show() | ||
| 2321 | } | 2339 | } | ||
| 2322 | 2340 | ||||
| 2341 | + | func resolveFavoriteRowIndicatorUpdate( | |||
| 2342 | + | mode ViewMode, | |||
| 2343 | + | index int, | |||
| 2344 | + | expectedURL string, | |||
| 2345 | + | suggestions []Suggestion, | |||
| 2346 | + | isFavorite bool, | |||
| 2347 | + | ) favoriteRowIndicatorUpdate { | |||
| 2348 | + | if mode != ViewModeHistory { | |||
| 2349 | + | return favoriteRowIndicatorUpdate{} | |||
| 2350 | + | } | |||
| 2351 | + | if index < 0 || index >= len(suggestions) { | |||
| 2352 | + | return favoriteRowIndicatorUpdate{} | |||
| 2353 | + | } | |||
| 2354 | + | if suggestions[index].URL != expectedURL { | |||
| 2355 | + | return favoriteRowIndicatorUpdate{} | |||
| 2356 | + | } | |||
| 2357 | + | return favoriteRowIndicatorUpdate{Apply: true, ShowStarSlot: isFavorite} | |||
| 2358 | + | } | |||
| 2359 | + | ||||
| 2323 | // yankSelectedURL copies the URL of the selected item to clipboard. | 2360 | // yankSelectedURL copies the URL of the selected item to clipboard. | ||
| 2324 | func (o *Omnibox) yankSelectedURL() { | 2361 | func (o *Omnibox) yankSelectedURL() { | ||
| 2325 | log := logging.FromContext(o.ctx) | 2362 | log := logging.FromContext(o.ctx) | ||
M internal/ui/component/omnibox_favorite_icon_test.go +65 -0
| @@ -34,3 +34,68 @@ func TestShouldApplyEmptyResultsState(t *testing.T) { | |||
| 34 | 34 | }) | |
| 35 | 35 | } | |
| 36 | 36 | } | |
| 37 | + | ||
| 38 | + | func TestResolveFavoriteRowIndicatorUpdate(t *testing.T) { | |
| 39 | + | tests := []struct { | |
| 40 | + | name string | |
| 41 | + | mode ViewMode | |
| 42 | + | index int | |
| 43 | + | expectedURL string | |
| 44 | + | suggestions []Suggestion | |
| 45 | + | isFavorite bool | |
| 46 | + | want favoriteRowIndicatorUpdate | |
| 47 | + | }{ | |
| 48 | + | { | |
| 49 | + | name: "matching history row keeps update and shows star", | |
| 50 | + | mode: ViewModeHistory, | |
| 51 | + | index: 0, | |
| 52 | + | expectedURL: "https://example.com", | |
| 53 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |
| 54 | + | isFavorite: true, | |
| 55 | + | want: favoriteRowIndicatorUpdate{Apply: true, ShowStarSlot: true}, | |
| 56 | + | }, | |
| 57 | + | { | |
| 58 | + | name: "matching history row keeps update and hides star when removed", | |
| 59 | + | mode: ViewModeHistory, | |
| 60 | + | index: 0, | |
| 61 | + | expectedURL: "https://example.com", | |
| 62 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |
| 63 | + | isFavorite: false, | |
| 64 | + | want: favoriteRowIndicatorUpdate{Apply: true, ShowStarSlot: false}, | |
| 65 | + | }, | |
| 66 | + | { | |
| 67 | + | name: "mismatched URL skips stale row update", | |
| 68 | + | mode: ViewModeHistory, | |
| 69 | + | index: 0, | |
| 70 | + | expectedURL: "https://expected.example.com", | |
| 71 | + | suggestions: []Suggestion{{URL: "https://current.example.com"}}, | |
| 72 | + | isFavorite: true, | |
| 73 | + | want: favoriteRowIndicatorUpdate{}, | |
| 74 | + | }, | |
| 75 | + | { | |
| 76 | + | name: "favorites mode never applies history row update", | |
| 77 | + | mode: ViewModeFavorites, | |
| 78 | + | index: 0, | |
| 79 | + | expectedURL: "https://example.com", | |
| 80 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |
| 81 | + | isFavorite: true, | |
| 82 | + | want: favoriteRowIndicatorUpdate{}, | |
| 83 | + | }, | |
| 84 | + | { | |
| 85 | + | name: "out of range index skips update", | |
| 86 | + | mode: ViewModeHistory, | |
| 87 | + | index: 1, | |
| 88 | + | expectedURL: "https://example.com", | |
| 89 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |
| 90 | + | isFavorite: true, | |
| 91 | + | want: favoriteRowIndicatorUpdate{}, | |
| 92 | + | }, | |
| 93 | + | } | |
| 94 | + | ||
| 95 | + | for _, tt := range tests { | |
| 96 | + | t.Run(tt.name, func(t *testing.T) { | |
| 97 | + | got := resolveFavoriteRowIndicatorUpdate(tt.mode, tt.index, tt.expectedURL, tt.suggestions, tt.isFavorite) | |
| 98 | + | assert.Equal(t, tt.want, got) | |
| 99 | + | }) | |
| 100 | + | } | |
| 101 | + | } | |
| @@ -34,3 +34,68 @@ func TestShouldApplyEmptyResultsState(t *testing.T) { | |||||
| 34 | }) | 34 | }) | ||
| 35 | } | 35 | } | ||
| 36 | } | 36 | } | ||
| 37 | + | ||||
| 38 | + | func TestResolveFavoriteRowIndicatorUpdate(t *testing.T) { | |||
| 39 | + | tests := []struct { | |||
| 40 | + | name string | |||
| 41 | + | mode ViewMode | |||
| 42 | + | index int | |||
| 43 | + | expectedURL string | |||
| 44 | + | suggestions []Suggestion | |||
| 45 | + | isFavorite bool | |||
| 46 | + | want favoriteRowIndicatorUpdate | |||
| 47 | + | }{ | |||
| 48 | + | { | |||
| 49 | + | name: "matching history row keeps update and shows star", | |||
| 50 | + | mode: ViewModeHistory, | |||
| 51 | + | index: 0, | |||
| 52 | + | expectedURL: "https://example.com", | |||
| 53 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |||
| 54 | + | isFavorite: true, | |||
| 55 | + | want: favoriteRowIndicatorUpdate{Apply: true, ShowStarSlot: true}, | |||
| 56 | + | }, | |||
| 57 | + | { | |||
| 58 | + | name: "matching history row keeps update and hides star when removed", | |||
| 59 | + | mode: ViewModeHistory, | |||
| 60 | + | index: 0, | |||
| 61 | + | expectedURL: "https://example.com", | |||
| 62 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |||
| 63 | + | isFavorite: false, | |||
| 64 | + | want: favoriteRowIndicatorUpdate{Apply: true, ShowStarSlot: false}, | |||
| 65 | + | }, | |||
| 66 | + | { | |||
| 67 | + | name: "mismatched URL skips stale row update", | |||
| 68 | + | mode: ViewModeHistory, | |||
| 69 | + | index: 0, | |||
| 70 | + | expectedURL: "https://expected.example.com", | |||
| 71 | + | suggestions: []Suggestion{{URL: "https://current.example.com"}}, | |||
| 72 | + | isFavorite: true, | |||
| 73 | + | want: favoriteRowIndicatorUpdate{}, | |||
| 74 | + | }, | |||
| 75 | + | { | |||
| 76 | + | name: "favorites mode never applies history row update", | |||
| 77 | + | mode: ViewModeFavorites, | |||
| 78 | + | index: 0, | |||
| 79 | + | expectedURL: "https://example.com", | |||
| 80 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |||
| 81 | + | isFavorite: true, | |||
| 82 | + | want: favoriteRowIndicatorUpdate{}, | |||
| 83 | + | }, | |||
| 84 | + | { | |||
| 85 | + | name: "out of range index skips update", | |||
| 86 | + | mode: ViewModeHistory, | |||
| 87 | + | index: 1, | |||
| 88 | + | expectedURL: "https://example.com", | |||
| 89 | + | suggestions: []Suggestion{{URL: "https://example.com"}}, | |||
| 90 | + | isFavorite: true, | |||
| 91 | + | want: favoriteRowIndicatorUpdate{}, | |||
| 92 | + | }, | |||
| 93 | + | } | |||
| 94 | + | ||||
| 95 | + | for _, tt := range tests { | |||
| 96 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 97 | + | got := resolveFavoriteRowIndicatorUpdate(tt.mode, tt.index, tt.expectedURL, tt.suggestions, tt.isFavorite) | |||
| 98 | + | assert.Equal(t, tt.want, got) | |||
| 99 | + | }) | |||
| 100 | + | } | |||
| 101 | + | } | |||
M internal/ui/standalone_omnibox.go +77 -21
| @@ -67,8 +67,48 @@ var ( | |||
| 67 | 67 | layerShellSetKeyboardMode = func(window *gtk.ApplicationWindow, mode layershell.KeyboardMode) { | |
| 68 | 68 | layershell.SetKeyboardMode(&window.Window, mode) | |
| 69 | 69 | } | |
| 70 | + | buildStandaloneOmniboxHostFn = buildStandaloneOmniboxHost | |
| 71 | + | newStandaloneOmniboxWindow = func(app *gtk.Application) *gtk.ApplicationWindow { | |
| 72 | + | return gtk.NewApplicationWindow(app) | |
| 73 | + | } | |
| 74 | + | configureStandaloneOmniboxWindowFn = configureStandaloneOmniboxWindow | |
| 75 | + | presentStandaloneOmniboxWindow = func(window *gtk.ApplicationWindow) { | |
| 76 | + | window.Present() | |
| 77 | + | } | |
| 78 | + | logStandaloneOmniboxHostAllocationFn = logStandaloneOmniboxHostAllocation | |
| 79 | + | showStandaloneOmniboxFn = func(ctx context.Context, omnibox *component.Omnibox) { | |
| 80 | + | omnibox.Show(ctx, "") | |
| 81 | + | } | |
| 70 | 82 | ) | |
| 71 | 83 | ||
| 84 | + | type standaloneOmniboxActivationRetention struct { | |
| 85 | + | window *gtk.ApplicationWindow | |
| 86 | + | host *gtk.Widget | |
| 87 | + | omnibox *component.Omnibox | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | func (r *standaloneOmniboxActivationRetention) retain( | |
| 91 | + | window *gtk.ApplicationWindow, | |
| 92 | + | host *gtk.Widget, | |
| 93 | + | omnibox *component.Omnibox, | |
| 94 | + | ) { | |
| 95 | + | if r == nil { | |
| 96 | + | return | |
| 97 | + | } | |
| 98 | + | r.window = window | |
| 99 | + | r.host = host | |
| 100 | + | r.omnibox = omnibox | |
| 101 | + | } | |
| 102 | + | ||
| 103 | + | func (r *standaloneOmniboxActivationRetention) release() { | |
| 104 | + | if r == nil { | |
| 105 | + | return | |
| 106 | + | } | |
| 107 | + | r.window = nil | |
| 108 | + | r.host = nil | |
| 109 | + | r.omnibox = nil | |
| 110 | + | } | |
| 111 | + | ||
| 72 | 112 | func DefaultStandaloneOmniboxWindowConfig() StandaloneOmniboxWindowConfig { | |
| 73 | 113 | return StandaloneOmniboxWindowConfig{ | |
| 74 | 114 | Namespace: standaloneOmniboxNamespace, | |
| @@ -269,6 +309,38 @@ func logStandaloneOmniboxHostAllocation(ctx context.Context, host *gtk.Widget) { | |||
| 269 | 309 | glib.IdleAdd(&cb, 0) | |
| 270 | 310 | } | |
| 271 | 311 | ||
| 312 | + | func activateStandaloneOmnibox( | |
| 313 | + | ctx context.Context, | |
| 314 | + | runtimeCfg *StandaloneOmniboxRuntime, | |
| 315 | + | gtkApp *gtk.Application, | |
| 316 | + | windowCfg StandaloneOmniboxWindowConfig, | |
| 317 | + | retention *standaloneOmniboxActivationRetention, | |
| 318 | + | ) { | |
| 319 | + | child, omnibox := buildStandaloneOmniboxHostFn(ctx, runtimeCfg, gtkApp) | |
| 320 | + | if child == nil || omnibox == nil { | |
| 321 | + | gtkApp.Quit() | |
| 322 | + | return | |
| 323 | + | } | |
| 324 | + | ||
| 325 | + | window := newStandaloneOmniboxWindow(gtkApp) | |
| 326 | + | if window == nil { | |
| 327 | + | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox window") | |
| 328 | + | gtkApp.Quit() | |
| 329 | + | return | |
| 330 | + | } | |
| 331 | + | ||
| 332 | + | retention.retain(window, child, omnibox) | |
| 333 | + | configureStandaloneOmniboxWindowFn(window, windowCfg, child) | |
| 334 | + | if runtimeCfg.ApplyTheme != nil { | |
| 335 | + | if display := window.GetDisplay(); display != nil { | |
| 336 | + | runtimeCfg.ApplyTheme(display) | |
| 337 | + | } | |
| 338 | + | } | |
| 339 | + | presentStandaloneOmniboxWindow(window) | |
| 340 | + | logStandaloneOmniboxHostAllocationFn(ctx, child) | |
| 341 | + | showStandaloneOmniboxFn(ctx, omnibox) | |
| 342 | + | } | |
| 343 | + | ||
| 272 | 344 | func RunStandaloneOmnibox(ctx context.Context, runtimeCfg *StandaloneOmniboxRuntime) int { | |
| 273 | 345 | if runtimeCfg == nil { | |
| 274 | 346 | logging.FromContext(ctx).Error().Msg("standalone omnibox runtime not configured") | |
| @@ -284,31 +356,15 @@ func RunStandaloneOmnibox(ctx context.Context, runtimeCfg *StandaloneOmniboxRunt | |||
| 284 | 356 | return 1 | |
| 285 | 357 | } | |
| 286 | 358 | defer gtkApp.Unref() | |
| 359 | + | retention := &standaloneOmniboxActivationRetention{} | |
| 287 | 360 | ||
| 288 | 361 | activateCb := func(_ gio.Application) { | |
| 289 | - | child, omnibox := buildStandaloneOmniboxHost(ctx, runtimeCfg, gtkApp) | |
| 290 | - | if child == nil || omnibox == nil { | |
| 291 | - | gtkApp.Quit() | |
| 292 | - | return | |
| 293 | - | } | |
| 294 | - | ||
| 295 | - | window := gtk.NewApplicationWindow(gtkApp) | |
| 296 | - | if window == nil { | |
| 297 | - | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox window") | |
| 298 | - | gtkApp.Quit() | |
| 299 | - | return | |
| 300 | - | } | |
| 301 | - | ||
| 302 | - | configureStandaloneOmniboxWindow(window, windowCfg, child) | |
| 303 | - | if display := window.GetDisplay(); display != nil && runtimeCfg.ApplyTheme != nil { | |
| 304 | - | runtimeCfg.ApplyTheme(display) | |
| 305 | - | } | |
| 306 | - | window.Present() | |
| 307 | - | logStandaloneOmniboxHostAllocation(ctx, child) | |
| 308 | - | omnibox.Show(ctx, "") | |
| 362 | + | activateStandaloneOmnibox(ctx, runtimeCfg, gtkApp, windowCfg, retention) | |
| 309 | 363 | } | |
| 310 | 364 | gtkApp.ConnectActivate(&activateCb) | |
| 311 | 365 | ||
| 312 | 366 | argv := standaloneOmniboxArgv(os.Args) | |
| 313 | - | return gtkApp.Run(len(argv), argv) | |
| 367 | + | code := gtkApp.Run(len(argv), argv) | |
| 368 | + | retention.release() | |
| 369 | + | return code | |
| 314 | 370 | } | |
| @@ -67,8 +67,48 @@ var ( | |||||
| 67 | layerShellSetKeyboardMode = func(window *gtk.ApplicationWindow, mode layershell.KeyboardMode) { | 67 | layerShellSetKeyboardMode = func(window *gtk.ApplicationWindow, mode layershell.KeyboardMode) { | ||
| 68 | layershell.SetKeyboardMode(&window.Window, mode) | 68 | layershell.SetKeyboardMode(&window.Window, mode) | ||
| 69 | } | 69 | } | ||
| 70 | + | buildStandaloneOmniboxHostFn = buildStandaloneOmniboxHost | |||
| 71 | + | newStandaloneOmniboxWindow = func(app *gtk.Application) *gtk.ApplicationWindow { | |||
| 72 | + | return gtk.NewApplicationWindow(app) | |||
| 73 | + | } | |||
| 74 | + | configureStandaloneOmniboxWindowFn = configureStandaloneOmniboxWindow | |||
| 75 | + | presentStandaloneOmniboxWindow = func(window *gtk.ApplicationWindow) { | |||
| 76 | + | window.Present() | |||
| 77 | + | } | |||
| 78 | + | logStandaloneOmniboxHostAllocationFn = logStandaloneOmniboxHostAllocation | |||
| 79 | + | showStandaloneOmniboxFn = func(ctx context.Context, omnibox *component.Omnibox) { | |||
| 80 | + | omnibox.Show(ctx, "") | |||
| 81 | + | } | |||
| 70 | ) | 82 | ) | ||
| 71 | 83 | ||||
| 84 | + | type standaloneOmniboxActivationRetention struct { | |||
| 85 | + | window *gtk.ApplicationWindow | |||
| 86 | + | host *gtk.Widget | |||
| 87 | + | omnibox *component.Omnibox | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | func (r *standaloneOmniboxActivationRetention) retain( | |||
| 91 | + | window *gtk.ApplicationWindow, | |||
| 92 | + | host *gtk.Widget, | |||
| 93 | + | omnibox *component.Omnibox, | |||
| 94 | + | ) { | |||
| 95 | + | if r == nil { | |||
| 96 | + | return | |||
| 97 | + | } | |||
| 98 | + | r.window = window | |||
| 99 | + | r.host = host | |||
| 100 | + | r.omnibox = omnibox | |||
| 101 | + | } | |||
| 102 | + | ||||
| 103 | + | func (r *standaloneOmniboxActivationRetention) release() { | |||
| 104 | + | if r == nil { | |||
| 105 | + | return | |||
| 106 | + | } | |||
| 107 | + | r.window = nil | |||
| 108 | + | r.host = nil | |||
| 109 | + | r.omnibox = nil | |||
| 110 | + | } | |||
| 111 | + | ||||
| 72 | func DefaultStandaloneOmniboxWindowConfig() StandaloneOmniboxWindowConfig { | 112 | func DefaultStandaloneOmniboxWindowConfig() StandaloneOmniboxWindowConfig { | ||
| 73 | return StandaloneOmniboxWindowConfig{ | 113 | return StandaloneOmniboxWindowConfig{ | ||
| 74 | Namespace: standaloneOmniboxNamespace, | 114 | Namespace: standaloneOmniboxNamespace, | ||
| @@ -269,6 +309,38 @@ func logStandaloneOmniboxHostAllocation(ctx context.Context, host *gtk.Widget) { | |||||
| 269 | glib.IdleAdd(&cb, 0) | 309 | glib.IdleAdd(&cb, 0) | ||
| 270 | } | 310 | } | ||
| 271 | 311 | ||||
| 312 | + | func activateStandaloneOmnibox( | |||
| 313 | + | ctx context.Context, | |||
| 314 | + | runtimeCfg *StandaloneOmniboxRuntime, | |||
| 315 | + | gtkApp *gtk.Application, | |||
| 316 | + | windowCfg StandaloneOmniboxWindowConfig, | |||
| 317 | + | retention *standaloneOmniboxActivationRetention, | |||
| 318 | + | ) { | |||
| 319 | + | child, omnibox := buildStandaloneOmniboxHostFn(ctx, runtimeCfg, gtkApp) | |||
| 320 | + | if child == nil || omnibox == nil { | |||
| 321 | + | gtkApp.Quit() | |||
| 322 | + | return | |||
| 323 | + | } | |||
| 324 | + | ||||
| 325 | + | window := newStandaloneOmniboxWindow(gtkApp) | |||
| 326 | + | if window == nil { | |||
| 327 | + | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox window") | |||
| 328 | + | gtkApp.Quit() | |||
| 329 | + | return | |||
| 330 | + | } | |||
| 331 | + | ||||
| 332 | + | retention.retain(window, child, omnibox) | |||
| 333 | + | configureStandaloneOmniboxWindowFn(window, windowCfg, child) | |||
| 334 | + | if runtimeCfg.ApplyTheme != nil { | |||
| 335 | + | if display := window.GetDisplay(); display != nil { | |||
| 336 | + | runtimeCfg.ApplyTheme(display) | |||
| 337 | + | } | |||
| 338 | + | } | |||
| 339 | + | presentStandaloneOmniboxWindow(window) | |||
| 340 | + | logStandaloneOmniboxHostAllocationFn(ctx, child) | |||
| 341 | + | showStandaloneOmniboxFn(ctx, omnibox) | |||
| 342 | + | } | |||
| 343 | + | ||||
| 272 | func RunStandaloneOmnibox(ctx context.Context, runtimeCfg *StandaloneOmniboxRuntime) int { | 344 | func RunStandaloneOmnibox(ctx context.Context, runtimeCfg *StandaloneOmniboxRuntime) int { | ||
| 273 | if runtimeCfg == nil { | 345 | if runtimeCfg == nil { | ||
| 274 | logging.FromContext(ctx).Error().Msg("standalone omnibox runtime not configured") | 346 | logging.FromContext(ctx).Error().Msg("standalone omnibox runtime not configured") | ||
| @@ -284,31 +356,15 @@ func RunStandaloneOmnibox(ctx context.Context, runtimeCfg *StandaloneOmniboxRunt | |||||
| 284 | return 1 | 356 | return 1 | ||
| 285 | } | 357 | } | ||
| 286 | defer gtkApp.Unref() | 358 | defer gtkApp.Unref() | ||
| 359 | + | retention := &standaloneOmniboxActivationRetention{} | |||
| 287 | 360 | ||||
| 288 | activateCb := func(_ gio.Application) { | 361 | activateCb := func(_ gio.Application) { | ||
| 289 | - | child, omnibox := buildStandaloneOmniboxHost(ctx, runtimeCfg, gtkApp) | 362 | + | activateStandaloneOmnibox(ctx, runtimeCfg, gtkApp, windowCfg, retention) |
| 290 | - | if child == nil || omnibox == nil { | |||
| 291 | - | gtkApp.Quit() | |||
| 292 | - | return | |||
| 293 | - | } | |||
| 294 | - | ||||
| 295 | - | window := gtk.NewApplicationWindow(gtkApp) | |||
| 296 | - | if window == nil { | |||
| 297 | - | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox window") | |||
| 298 | - | gtkApp.Quit() | |||
| 299 | - | return | |||
| 300 | - | } | |||
| 301 | - | ||||
| 302 | - | configureStandaloneOmniboxWindow(window, windowCfg, child) | |||
| 303 | - | if display := window.GetDisplay(); display != nil && runtimeCfg.ApplyTheme != nil { | |||
| 304 | - | runtimeCfg.ApplyTheme(display) | |||
| 305 | - | } | |||
| 306 | - | window.Present() | |||
| 307 | - | logStandaloneOmniboxHostAllocation(ctx, child) | |||
| 308 | - | omnibox.Show(ctx, "") | |||
| 309 | } | 363 | } | ||
| 310 | gtkApp.ConnectActivate(&activateCb) | 364 | gtkApp.ConnectActivate(&activateCb) | ||
| 311 | 365 | ||||
| 312 | argv := standaloneOmniboxArgv(os.Args) | 366 | argv := standaloneOmniboxArgv(os.Args) | ||
| 313 | - | return gtkApp.Run(len(argv), argv) | 367 | + | code := gtkApp.Run(len(argv), argv) |
| 368 | + | retention.release() | |||
| 369 | + | return code | |||
| 314 | } | 370 | } | ||
M internal/ui/standalone_omnibox_test.go +67 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package ui | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/dumber/internal/ui/component" | |
| @@ -175,6 +176,72 @@ func TestStandaloneOmniboxArgv_RemovesOnlyLeadingOmniboxSubcommand(t *testing.T) | |||
| 175 | 176 | } | |
| 176 | 177 | } | |
| 177 | 178 | ||
| 179 | + | func TestActivateStandaloneOmnibox_RetainsObjectsUntilReleased(t *testing.T) { | |
| 180 | + | originalBuildHost := buildStandaloneOmniboxHostFn | |
| 181 | + | originalNewWindow := newStandaloneOmniboxWindow | |
| 182 | + | originalConfigureWindow := configureStandaloneOmniboxWindowFn | |
| 183 | + | originalPresentWindow := presentStandaloneOmniboxWindow | |
| 184 | + | originalLogHostAllocation := logStandaloneOmniboxHostAllocationFn | |
| 185 | + | originalShowOmnibox := showStandaloneOmniboxFn | |
| 186 | + | t.Cleanup(func() { | |
| 187 | + | buildStandaloneOmniboxHostFn = originalBuildHost | |
| 188 | + | newStandaloneOmniboxWindow = originalNewWindow | |
| 189 | + | configureStandaloneOmniboxWindowFn = originalConfigureWindow | |
| 190 | + | presentStandaloneOmniboxWindow = originalPresentWindow | |
| 191 | + | logStandaloneOmniboxHostAllocationFn = originalLogHostAllocation | |
| 192 | + | showStandaloneOmniboxFn = originalShowOmnibox | |
| 193 | + | }) | |
| 194 | + | ||
| 195 | + | expectedHost := >k.Widget{} | |
| 196 | + | expectedOmnibox := &component.Omnibox{} | |
| 197 | + | expectedWindow := >k.ApplicationWindow{} | |
| 198 | + | refs := &standaloneOmniboxActivationRetention{} | |
| 199 | + | runtimeCfg := &StandaloneOmniboxRuntime{} | |
| 200 | + | windowCfg := DefaultStandaloneOmniboxWindowConfig() | |
| 201 | + | ||
| 202 | + | buildStandaloneOmniboxHostFn = func(_ context.Context, _ *StandaloneOmniboxRuntime, _ *gtk.Application) (*gtk.Widget, *component.Omnibox) { | |
| 203 | + | return expectedHost, expectedOmnibox | |
| 204 | + | } | |
| 205 | + | newStandaloneOmniboxWindow = func(_ *gtk.Application) *gtk.ApplicationWindow { | |
| 206 | + | return expectedWindow | |
| 207 | + | } | |
| 208 | + | configureStandaloneOmniboxWindowFn = func(window *gtk.ApplicationWindow, cfg StandaloneOmniboxWindowConfig, child *gtk.Widget) { | |
| 209 | + | if window != expectedWindow || child != expectedHost || cfg != windowCfg { | |
| 210 | + | t.Fatalf("unexpected configure arguments: window=%p child=%p cfg=%#v", window, child, cfg) | |
| 211 | + | } | |
| 212 | + | } | |
| 213 | + | presentStandaloneOmniboxWindow = func(window *gtk.ApplicationWindow) { | |
| 214 | + | if window != expectedWindow { | |
| 215 | + | t.Fatalf("unexpected window presented: %p", window) | |
| 216 | + | } | |
| 217 | + | } | |
| 218 | + | logStandaloneOmniboxHostAllocationFn = func(_ context.Context, host *gtk.Widget) { | |
| 219 | + | if host != expectedHost { | |
| 220 | + | t.Fatalf("unexpected host logged: %p", host) | |
| 221 | + | } | |
| 222 | + | } | |
| 223 | + | showStandaloneOmniboxFn = func(_ context.Context, omnibox *component.Omnibox) { | |
| 224 | + | if omnibox != expectedOmnibox { | |
| 225 | + | t.Fatalf("unexpected omnibox shown: %p", omnibox) | |
| 226 | + | } | |
| 227 | + | if refs.window != expectedWindow || refs.host != expectedHost || refs.omnibox != expectedOmnibox { | |
| 228 | + | t.Fatalf("expected standalone activation refs to retain GTK objects during show, got %#v", refs) | |
| 229 | + | } | |
| 230 | + | } | |
| 231 | + | ||
| 232 | + | activateStandaloneOmnibox(context.Background(), runtimeCfg, nil, windowCfg, refs) | |
| 233 | + | ||
| 234 | + | if refs.window != expectedWindow || refs.host != expectedHost || refs.omnibox != expectedOmnibox { | |
| 235 | + | t.Fatalf("expected standalone activation refs to retain GTK objects, got %#v", refs) | |
| 236 | + | } | |
| 237 | + | ||
| 238 | + | refs.release() | |
| 239 | + | ||
| 240 | + | if refs.window != nil || refs.host != nil || refs.omnibox != nil { | |
| 241 | + | t.Fatalf("expected standalone activation refs to release GTK objects, got %#v", refs) | |
| 242 | + | } | |
| 243 | + | } | |
| 244 | + | ||
| 178 | 245 | func TestTryInitLayerShell_NilWindowReturnsFalse(t *testing.T) { | |
| 179 | 246 | if tryInitLayerShell(nil, DefaultStandaloneOmniboxWindowConfig()) { | |
| 180 | 247 | t.Fatalf("expected nil window to skip layer-shell") | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package ui | 1 | package ui | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/bnema/dumber/internal/ui/component" | 7 | "github.com/bnema/dumber/internal/ui/component" | ||
| @@ -175,6 +176,72 @@ func TestStandaloneOmniboxArgv_RemovesOnlyLeadingOmniboxSubcommand(t *testing.T) | |||||
| 175 | } | 176 | } | ||
| 176 | } | 177 | } | ||
| 177 | 178 | ||||
| 179 | + | func TestActivateStandaloneOmnibox_RetainsObjectsUntilReleased(t *testing.T) { | |||
| 180 | + | originalBuildHost := buildStandaloneOmniboxHostFn | |||
| 181 | + | originalNewWindow := newStandaloneOmniboxWindow | |||
| 182 | + | originalConfigureWindow := configureStandaloneOmniboxWindowFn | |||
| 183 | + | originalPresentWindow := presentStandaloneOmniboxWindow | |||
| 184 | + | originalLogHostAllocation := logStandaloneOmniboxHostAllocationFn | |||
| 185 | + | originalShowOmnibox := showStandaloneOmniboxFn | |||
| 186 | + | t.Cleanup(func() { | |||
| 187 | + | buildStandaloneOmniboxHostFn = originalBuildHost | |||
| 188 | + | newStandaloneOmniboxWindow = originalNewWindow | |||
| 189 | + | configureStandaloneOmniboxWindowFn = originalConfigureWindow | |||
| 190 | + | presentStandaloneOmniboxWindow = originalPresentWindow | |||
| 191 | + | logStandaloneOmniboxHostAllocationFn = originalLogHostAllocation | |||
| 192 | + | showStandaloneOmniboxFn = originalShowOmnibox | |||
| 193 | + | }) | |||
| 194 | + | ||||
| 195 | + | expectedHost := >k.Widget{} | |||
| 196 | + | expectedOmnibox := &component.Omnibox{} | |||
| 197 | + | expectedWindow := >k.ApplicationWindow{} | |||
| 198 | + | refs := &standaloneOmniboxActivationRetention{} | |||
| 199 | + | runtimeCfg := &StandaloneOmniboxRuntime{} | |||
| 200 | + | windowCfg := DefaultStandaloneOmniboxWindowConfig() | |||
| 201 | + | ||||
| 202 | + | buildStandaloneOmniboxHostFn = func(_ context.Context, _ *StandaloneOmniboxRuntime, _ *gtk.Application) (*gtk.Widget, *component.Omnibox) { | |||
| 203 | + | return expectedHost, expectedOmnibox | |||
| 204 | + | } | |||
| 205 | + | newStandaloneOmniboxWindow = func(_ *gtk.Application) *gtk.ApplicationWindow { | |||
| 206 | + | return expectedWindow | |||
| 207 | + | } | |||
| 208 | + | configureStandaloneOmniboxWindowFn = func(window *gtk.ApplicationWindow, cfg StandaloneOmniboxWindowConfig, child *gtk.Widget) { | |||
| 209 | + | if window != expectedWindow || child != expectedHost || cfg != windowCfg { | |||
| 210 | + | t.Fatalf("unexpected configure arguments: window=%p child=%p cfg=%#v", window, child, cfg) | |||
| 211 | + | } | |||
| 212 | + | } | |||
| 213 | + | presentStandaloneOmniboxWindow = func(window *gtk.ApplicationWindow) { | |||
| 214 | + | if window != expectedWindow { | |||
| 215 | + | t.Fatalf("unexpected window presented: %p", window) | |||
| 216 | + | } | |||
| 217 | + | } | |||
| 218 | + | logStandaloneOmniboxHostAllocationFn = func(_ context.Context, host *gtk.Widget) { | |||
| 219 | + | if host != expectedHost { | |||
| 220 | + | t.Fatalf("unexpected host logged: %p", host) | |||
| 221 | + | } | |||
| 222 | + | } | |||
| 223 | + | showStandaloneOmniboxFn = func(_ context.Context, omnibox *component.Omnibox) { | |||
| 224 | + | if omnibox != expectedOmnibox { | |||
| 225 | + | t.Fatalf("unexpected omnibox shown: %p", omnibox) | |||
| 226 | + | } | |||
| 227 | + | if refs.window != expectedWindow || refs.host != expectedHost || refs.omnibox != expectedOmnibox { | |||
| 228 | + | t.Fatalf("expected standalone activation refs to retain GTK objects during show, got %#v", refs) | |||
| 229 | + | } | |||
| 230 | + | } | |||
| 231 | + | ||||
| 232 | + | activateStandaloneOmnibox(context.Background(), runtimeCfg, nil, windowCfg, refs) | |||
| 233 | + | ||||
| 234 | + | if refs.window != expectedWindow || refs.host != expectedHost || refs.omnibox != expectedOmnibox { | |||
| 235 | + | t.Fatalf("expected standalone activation refs to retain GTK objects, got %#v", refs) | |||
| 236 | + | } | |||
| 237 | + | ||||
| 238 | + | refs.release() | |||
| 239 | + | ||||
| 240 | + | if refs.window != nil || refs.host != nil || refs.omnibox != nil { | |||
| 241 | + | t.Fatalf("expected standalone activation refs to release GTK objects, got %#v", refs) | |||
| 242 | + | } | |||
| 243 | + | } | |||
| 244 | + | ||||
| 178 | func TestTryInitLayerShell_NilWindowReturnsFalse(t *testing.T) { | 245 | func TestTryInitLayerShell_NilWindowReturnsFalse(t *testing.T) { | ||
| 179 | if tryInitLayerShell(nil, DefaultStandaloneOmniboxWindowConfig()) { | 246 | if tryInitLayerShell(nil, DefaultStandaloneOmniboxWindowConfig()) { | ||
| 180 | t.Fatalf("expected nil window to skip layer-shell") | 247 | t.Fatalf("expected nil window to skip layer-shell") | ||