fix: harden omnibox hover and cache guards
- commit
- 45241850eef148b4f2870240f3b9556d472c673e
- author
- brice · · 2026-03-11 03:44 UTC
- committer
- brice · · 2026-03-11 03:45 UTC
- parent
- f7811f6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +189 -1
M internal/application/usecase/manage_favorites.go +6 -0
| @@ -209,6 +209,11 @@ func (uc *ManageFavoritesUseCase) GetAll(ctx context.Context) ([]*entity.Favorit | |||
| 209 | 209 | } | |
| 210 | 210 | ||
| 211 | 211 | uc.cacheMu.Lock() | |
| 212 | + | if time.Since(uc.cacheTime) < favoritesCacheTTL && uc.favoritesCache != nil { | |
| 213 | + | cached := uc.favoritesCache | |
| 214 | + | uc.cacheMu.Unlock() | |
| 215 | + | return cached, nil | |
| 216 | + | } | |
| 212 | 217 | uc.favoritesCache = favs | |
| 213 | 218 | uc.favoriteURLsCache = buildFavoriteURLSet(favs) | |
| 214 | 219 | uc.cacheTime = time.Now() | |
| @@ -492,6 +497,7 @@ func (uc *ManageFavoritesUseCase) DeleteFolder(ctx context.Context, id entity.Fo | |||
| 492 | 497 | Msg("failed to move favorite from deleted folder") | |
| 493 | 498 | } | |
| 494 | 499 | } | |
| 500 | + | uc.invalidateCache() | |
| 495 | 501 | ||
| 496 | 502 | // Get child folders | |
| 497 | 503 | children, err := uc.folderRepo.GetChildren(ctx, &id) | |
| @@ -209,6 +209,11 @@ func (uc *ManageFavoritesUseCase) GetAll(ctx context.Context) ([]*entity.Favorit | |||||
| 209 | } | 209 | } | ||
| 210 | 210 | ||||
| 211 | uc.cacheMu.Lock() | 211 | uc.cacheMu.Lock() | ||
| 212 | + | if time.Since(uc.cacheTime) < favoritesCacheTTL && uc.favoritesCache != nil { | |||
| 213 | + | cached := uc.favoritesCache | |||
| 214 | + | uc.cacheMu.Unlock() | |||
| 215 | + | return cached, nil | |||
| 216 | + | } | |||
| 212 | uc.favoritesCache = favs | 217 | uc.favoritesCache = favs | ||
| 213 | uc.favoriteURLsCache = buildFavoriteURLSet(favs) | 218 | uc.favoriteURLsCache = buildFavoriteURLSet(favs) | ||
| 214 | uc.cacheTime = time.Now() | 219 | uc.cacheTime = time.Now() | ||
| @@ -492,6 +497,7 @@ func (uc *ManageFavoritesUseCase) DeleteFolder(ctx context.Context, id entity.Fo | |||||
| 492 | Msg("failed to move favorite from deleted folder") | 497 | Msg("failed to move favorite from deleted folder") | ||
| 493 | } | 498 | } | ||
| 494 | } | 499 | } | ||
| 500 | + | uc.invalidateCache() | |||
| 495 | 501 | ||||
| 496 | // Get child folders | 502 | // Get child folders | ||
| 497 | children, err := uc.folderRepo.GetChildren(ctx, &id) | 503 | children, err := uc.folderRepo.GetChildren(ctx, &id) | ||
A internal/application/usecase/manage_favorites_cache_test.go +48 -0
| @@ -0,0 +1,48 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| 10 | + | "github.com/bnema/dumber/internal/logging" | |
| 11 | + | "github.com/stretchr/testify/assert" | |
| 12 | + | "github.com/stretchr/testify/require" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestManageFavoritesUseCase_GetAllDoesNotOverwriteFreshCache(t *testing.T) { | |
| 16 | + | logger := logging.NewFromConfigValues("debug", "console") | |
| 17 | + | ctx := logging.WithContext(context.Background(), logger) | |
| 18 | + | ||
| 19 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |
| 20 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |
| 21 | + | tagRepo := repomocks.NewMockTagRepository(t) | |
| 22 | + | ||
| 23 | + | fetchedFavorites := []*entity.Favorite{ | |
| 24 | + | {ID: 1, URL: "https://stale.example"}, | |
| 25 | + | } | |
| 26 | + | refreshedFavorites := []*entity.Favorite{ | |
| 27 | + | {ID: 2, URL: "https://fresh.example"}, | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | uc := NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |
| 31 | + | favoriteRepo.EXPECT().GetAll(ctx).Run(func(_ context.Context) { | |
| 32 | + | uc.cacheMu.Lock() | |
| 33 | + | uc.favoritesCache = refreshedFavorites | |
| 34 | + | uc.favoriteURLsCache = buildFavoriteURLSet(refreshedFavorites) | |
| 35 | + | uc.cacheTime = time.Now() | |
| 36 | + | uc.cacheMu.Unlock() | |
| 37 | + | }).Return(fetchedFavorites, nil).Once() | |
| 38 | + | ||
| 39 | + | favorites, err := uc.GetAll(ctx) | |
| 40 | + | require.NoError(t, err) | |
| 41 | + | assert.Equal(t, refreshedFavorites, favorites) | |
| 42 | + | ||
| 43 | + | uc.cacheMu.Lock() | |
| 44 | + | assert.Equal(t, refreshedFavorites, uc.favoritesCache) | |
| 45 | + | assert.Contains(t, uc.favoriteURLsCache, "https://fresh.example") | |
| 46 | + | assert.NotContains(t, uc.favoriteURLsCache, "https://stale.example") | |
| 47 | + | uc.cacheMu.Unlock() | |
| 48 | + | } | |
| @@ -0,0 +1,48 @@ | |||||
| 1 | + | package usecase | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 9 | + | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |||
| 10 | + | "github.com/bnema/dumber/internal/logging" | |||
| 11 | + | "github.com/stretchr/testify/assert" | |||
| 12 | + | "github.com/stretchr/testify/require" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestManageFavoritesUseCase_GetAllDoesNotOverwriteFreshCache(t *testing.T) { | |||
| 16 | + | logger := logging.NewFromConfigValues("debug", "console") | |||
| 17 | + | ctx := logging.WithContext(context.Background(), logger) | |||
| 18 | + | ||||
| 19 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |||
| 20 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |||
| 21 | + | tagRepo := repomocks.NewMockTagRepository(t) | |||
| 22 | + | ||||
| 23 | + | fetchedFavorites := []*entity.Favorite{ | |||
| 24 | + | {ID: 1, URL: "https://stale.example"}, | |||
| 25 | + | } | |||
| 26 | + | refreshedFavorites := []*entity.Favorite{ | |||
| 27 | + | {ID: 2, URL: "https://fresh.example"}, | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | uc := NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |||
| 31 | + | favoriteRepo.EXPECT().GetAll(ctx).Run(func(_ context.Context) { | |||
| 32 | + | uc.cacheMu.Lock() | |||
| 33 | + | uc.favoritesCache = refreshedFavorites | |||
| 34 | + | uc.favoriteURLsCache = buildFavoriteURLSet(refreshedFavorites) | |||
| 35 | + | uc.cacheTime = time.Now() | |||
| 36 | + | uc.cacheMu.Unlock() | |||
| 37 | + | }).Return(fetchedFavorites, nil).Once() | |||
| 38 | + | ||||
| 39 | + | favorites, err := uc.GetAll(ctx) | |||
| 40 | + | require.NoError(t, err) | |||
| 41 | + | assert.Equal(t, refreshedFavorites, favorites) | |||
| 42 | + | ||||
| 43 | + | uc.cacheMu.Lock() | |||
| 44 | + | assert.Equal(t, refreshedFavorites, uc.favoritesCache) | |||
| 45 | + | assert.Contains(t, uc.favoriteURLsCache, "https://fresh.example") | |||
| 46 | + | assert.NotContains(t, uc.favoriteURLsCache, "https://stale.example") | |||
| 47 | + | uc.cacheMu.Unlock() | |||
| 48 | + | } | |||
M internal/application/usecase/manage_favorites_test.go +40 -0
| @@ -302,3 +302,43 @@ func TestManageFavoritesUseCase_Toggle_InvalidatesCache(t *testing.T) { | |||
| 302 | 302 | require.NoError(t, err) | |
| 303 | 303 | require.Contains(t, urls, "https://new.example") | |
| 304 | 304 | } | |
| 305 | + | ||
| 306 | + | func TestManageFavoritesUseCase_DeleteFolder_InvalidatesFavoritesCache(t *testing.T) { | |
| 307 | + | ctx := testContext() | |
| 308 | + | ||
| 309 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |
| 310 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |
| 311 | + | tagRepo := repomocks.NewMockTagRepository(t) | |
| 312 | + | ||
| 313 | + | folderID := entity.FolderID(9) | |
| 314 | + | initialFolderID := folderID | |
| 315 | + | cachedFavorites := []*entity.Favorite{ | |
| 316 | + | {ID: 1, URL: "https://example.com", FolderID: &initialFolderID}, | |
| 317 | + | } | |
| 318 | + | updatedFavorites := []*entity.Favorite{ | |
| 319 | + | {ID: 1, URL: "https://example.com", FolderID: nil}, | |
| 320 | + | } | |
| 321 | + | ||
| 322 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(cachedFavorites, nil).Once() | |
| 323 | + | folderRepo.EXPECT().FindByID(mock.Anything, folderID).Return(&entity.Folder{ID: folderID}, nil).Once() | |
| 324 | + | favoriteRepo.EXPECT().GetByFolder(mock.Anything, &folderID).Return(cachedFavorites, nil).Once() | |
| 325 | + | favoriteRepo.EXPECT().SetFolder(mock.Anything, entity.FavoriteID(1), (*entity.FolderID)(nil)).Return(nil).Once() | |
| 326 | + | folderRepo.EXPECT().GetChildren(mock.Anything, &folderID).Return([]*entity.Folder{}, nil).Once() | |
| 327 | + | folderRepo.EXPECT().Delete(mock.Anything, folderID).Return(nil).Once() | |
| 328 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(updatedFavorites, nil).Once() | |
| 329 | + | ||
| 330 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |
| 331 | + | ||
| 332 | + | favoritesBefore, err := uc.GetAll(ctx) | |
| 333 | + | require.NoError(t, err) | |
| 334 | + | require.Len(t, favoritesBefore, 1) | |
| 335 | + | require.NotNil(t, favoritesBefore[0].FolderID) | |
| 336 | + | ||
| 337 | + | err = uc.DeleteFolder(ctx, folderID) | |
| 338 | + | require.NoError(t, err) | |
| 339 | + | ||
| 340 | + | favoritesAfter, err := uc.GetAll(ctx) | |
| 341 | + | require.NoError(t, err) | |
| 342 | + | require.Len(t, favoritesAfter, 1) | |
| 343 | + | assert.Nil(t, favoritesAfter[0].FolderID) | |
| 344 | + | } | |
| @@ -302,3 +302,43 @@ func TestManageFavoritesUseCase_Toggle_InvalidatesCache(t *testing.T) { | |||||
| 302 | require.NoError(t, err) | 302 | require.NoError(t, err) | ||
| 303 | require.Contains(t, urls, "https://new.example") | 303 | require.Contains(t, urls, "https://new.example") | ||
| 304 | } | 304 | } | ||
| 305 | + | ||||
| 306 | + | func TestManageFavoritesUseCase_DeleteFolder_InvalidatesFavoritesCache(t *testing.T) { | |||
| 307 | + | ctx := testContext() | |||
| 308 | + | ||||
| 309 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |||
| 310 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |||
| 311 | + | tagRepo := repomocks.NewMockTagRepository(t) | |||
| 312 | + | ||||
| 313 | + | folderID := entity.FolderID(9) | |||
| 314 | + | initialFolderID := folderID | |||
| 315 | + | cachedFavorites := []*entity.Favorite{ | |||
| 316 | + | {ID: 1, URL: "https://example.com", FolderID: &initialFolderID}, | |||
| 317 | + | } | |||
| 318 | + | updatedFavorites := []*entity.Favorite{ | |||
| 319 | + | {ID: 1, URL: "https://example.com", FolderID: nil}, | |||
| 320 | + | } | |||
| 321 | + | ||||
| 322 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(cachedFavorites, nil).Once() | |||
| 323 | + | folderRepo.EXPECT().FindByID(mock.Anything, folderID).Return(&entity.Folder{ID: folderID}, nil).Once() | |||
| 324 | + | favoriteRepo.EXPECT().GetByFolder(mock.Anything, &folderID).Return(cachedFavorites, nil).Once() | |||
| 325 | + | favoriteRepo.EXPECT().SetFolder(mock.Anything, entity.FavoriteID(1), (*entity.FolderID)(nil)).Return(nil).Once() | |||
| 326 | + | folderRepo.EXPECT().GetChildren(mock.Anything, &folderID).Return([]*entity.Folder{}, nil).Once() | |||
| 327 | + | folderRepo.EXPECT().Delete(mock.Anything, folderID).Return(nil).Once() | |||
| 328 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(updatedFavorites, nil).Once() | |||
| 329 | + | ||||
| 330 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |||
| 331 | + | ||||
| 332 | + | favoritesBefore, err := uc.GetAll(ctx) | |||
| 333 | + | require.NoError(t, err) | |||
| 334 | + | require.Len(t, favoritesBefore, 1) | |||
| 335 | + | require.NotNil(t, favoritesBefore[0].FolderID) | |||
| 336 | + | ||||
| 337 | + | err = uc.DeleteFolder(ctx, folderID) | |||
| 338 | + | require.NoError(t, err) | |||
| 339 | + | ||||
| 340 | + | favoritesAfter, err := uc.GetAll(ctx) | |||
| 341 | + | require.NoError(t, err) | |||
| 342 | + | require.Len(t, favoritesAfter, 1) | |||
| 343 | + | assert.Nil(t, favoritesAfter[0].FolderID) | |||
| 344 | + | } | |||
M internal/infrastructure/persistence/sqlite/history_repo.go +5 -1
| @@ -114,6 +114,10 @@ func (r *historyRepo) UpdateMetadata(ctx context.Context, entry *entity.HistoryE | |||
| 114 | 114 | } | |
| 115 | 115 | ||
| 116 | 116 | func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]entity.HistoryMatch, error) { | |
| 117 | + | if limit <= 0 { | |
| 118 | + | return []entity.HistoryMatch{}, nil | |
| 119 | + | } | |
| 120 | + | ||
| 117 | 121 | words := sanitizeFTS5QueryTokens(query) | |
| 118 | 122 | if len(words) == 0 { | |
| 119 | 123 | return []entity.HistoryMatch{}, nil | |
| @@ -149,7 +153,7 @@ func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]en | |||
| 149 | 153 | } | |
| 150 | 154 | ||
| 151 | 155 | ranked := rankHistoryMatches(query, words, urlRows, titleRows) | |
| 152 | - | if len(ranked) > limit { | |
| 156 | + | if limit < len(ranked) { | |
| 153 | 157 | ranked = ranked[:limit] | |
| 154 | 158 | } | |
| 155 | 159 | return ranked, nil | |
| @@ -114,6 +114,10 @@ func (r *historyRepo) UpdateMetadata(ctx context.Context, entry *entity.HistoryE | |||||
| 114 | } | 114 | } | ||
| 115 | 115 | ||||
| 116 | func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]entity.HistoryMatch, error) { | 116 | func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]entity.HistoryMatch, error) { | ||
| 117 | + | if limit <= 0 { | |||
| 118 | + | return []entity.HistoryMatch{}, nil | |||
| 119 | + | } | |||
| 120 | + | ||||
| 117 | words := sanitizeFTS5QueryTokens(query) | 121 | words := sanitizeFTS5QueryTokens(query) | ||
| 118 | if len(words) == 0 { | 122 | if len(words) == 0 { | ||
| 119 | return []entity.HistoryMatch{}, nil | 123 | return []entity.HistoryMatch{}, nil | ||
| @@ -149,7 +153,7 @@ func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]en | |||||
| 149 | } | 153 | } | ||
| 150 | 154 | ||||
| 151 | ranked := rankHistoryMatches(query, words, urlRows, titleRows) | 155 | ranked := rankHistoryMatches(query, words, urlRows, titleRows) | ||
| 152 | - | if len(ranked) > limit { | 156 | + | if limit < len(ranked) { |
| 153 | ranked = ranked[:limit] | 157 | ranked = ranked[:limit] | ||
| 154 | } | 158 | } | ||
| 155 | return ranked, nil | 159 | return ranked, nil | ||
M internal/infrastructure/persistence/sqlite/history_repo_test.go +23 -0
| @@ -809,3 +809,26 @@ func TestHistoryRepository_AboutBlank_NotDominant(t *testing.T) { | |||
| 809 | 809 | assert.Equal(t, "about:blank", results[1].URL, "about:blank should be second with capped visit count") | |
| 810 | 810 | assert.Equal(t, int64(1), results[1].VisitCount) | |
| 811 | 811 | } | |
| 812 | + | ||
| 813 | + | func TestHistoryRepository_Search_ReturnsEmptyForNonPositiveLimit(t *testing.T) { | |
| 814 | + | ctx := historyTestCtx() | |
| 815 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 816 | + | ||
| 817 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 818 | + | require.NoError(t, err) | |
| 819 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 820 | + | ||
| 821 | + | repo := sqlite.NewHistoryRepository(db) | |
| 822 | + | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |
| 823 | + | URL: "https://github.com", | |
| 824 | + | Title: "GitHub", | |
| 825 | + | })) | |
| 826 | + | ||
| 827 | + | results, err := repo.Search(ctx, "git", 0) | |
| 828 | + | require.NoError(t, err) | |
| 829 | + | assert.Empty(t, results) | |
| 830 | + | ||
| 831 | + | results, err = repo.Search(ctx, "git", -1) | |
| 832 | + | require.NoError(t, err) | |
| 833 | + | assert.Empty(t, results) | |
| 834 | + | } | |
| @@ -809,3 +809,26 @@ func TestHistoryRepository_AboutBlank_NotDominant(t *testing.T) { | |||||
| 809 | assert.Equal(t, "about:blank", results[1].URL, "about:blank should be second with capped visit count") | 809 | assert.Equal(t, "about:blank", results[1].URL, "about:blank should be second with capped visit count") | ||
| 810 | assert.Equal(t, int64(1), results[1].VisitCount) | 810 | assert.Equal(t, int64(1), results[1].VisitCount) | ||
| 811 | } | 811 | } | ||
| 812 | + | ||||
| 813 | + | func TestHistoryRepository_Search_ReturnsEmptyForNonPositiveLimit(t *testing.T) { | |||
| 814 | + | ctx := historyTestCtx() | |||
| 815 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 816 | + | ||||
| 817 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 818 | + | require.NoError(t, err) | |||
| 819 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 820 | + | ||||
| 821 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 822 | + | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |||
| 823 | + | URL: "https://github.com", | |||
| 824 | + | Title: "GitHub", | |||
| 825 | + | })) | |||
| 826 | + | ||||
| 827 | + | results, err := repo.Search(ctx, "git", 0) | |||
| 828 | + | require.NoError(t, err) | |||
| 829 | + | assert.Empty(t, results) | |||
| 830 | + | ||||
| 831 | + | results, err = repo.Search(ctx, "git", -1) | |||
| 832 | + | require.NoError(t, err) | |||
| 833 | + | assert.Empty(t, results) | |||
| 834 | + | } | |||
M internal/ui/component/omnibox.go +13 -0
| @@ -1852,6 +1852,15 @@ func (o *Omnibox) attachRowHoverSelection(row *gtk.ListBoxRow, index int) { | |||
| 1852 | 1852 | } | |
| 1853 | 1853 | ||
| 1854 | 1854 | enterCb := func(_ gtk.EventControllerMotion, _ float64, _ float64) { | |
| 1855 | + | o.mu.RLock() | |
| 1856 | + | realInput := o.realInput | |
| 1857 | + | hasGhostText := o.hasGhostText | |
| 1858 | + | hasNavigated := o.hasNavigated | |
| 1859 | + | o.mu.RUnlock() | |
| 1860 | + | ||
| 1861 | + | if !shouldPromoteHoverSelection(realInput, hasGhostText, hasNavigated) { | |
| 1862 | + | return | |
| 1863 | + | } | |
| 1855 | 1864 | o.selectIndex(index) | |
| 1856 | 1865 | } | |
| 1857 | 1866 | o.retainedCallbacks = append(o.retainedCallbacks, enterCb) | |
| @@ -1859,6 +1868,10 @@ func (o *Omnibox) attachRowHoverSelection(row *gtk.ListBoxRow, index int) { | |||
| 1859 | 1868 | row.AddController(&motionCtrl.EventController) | |
| 1860 | 1869 | } | |
| 1861 | 1870 | ||
| 1871 | + | func shouldPromoteHoverSelection(realInput string, hasGhostText, hasNavigated bool) bool { | |
| 1872 | + | return realInput == "" && !hasGhostText && !hasNavigated | |
| 1873 | + | } | |
| 1874 | + | ||
| 1862 | 1875 | // createSuggestionRow creates a ListBoxRow for a suggestion. | |
| 1863 | 1876 | func (o *Omnibox) createSuggestionRow(s Suggestion, index int) *gtk.ListBoxRow { | |
| 1864 | 1877 | row := o.createRowWithFavicon(s.URL, s.Title, "web-browser-symbolic", index) | |
| @@ -1852,6 +1852,15 @@ func (o *Omnibox) attachRowHoverSelection(row *gtk.ListBoxRow, index int) { | |||||
| 1852 | } | 1852 | } | ||
| 1853 | 1853 | ||||
| 1854 | enterCb := func(_ gtk.EventControllerMotion, _ float64, _ float64) { | 1854 | enterCb := func(_ gtk.EventControllerMotion, _ float64, _ float64) { | ||
| 1855 | + | o.mu.RLock() | |||
| 1856 | + | realInput := o.realInput | |||
| 1857 | + | hasGhostText := o.hasGhostText | |||
| 1858 | + | hasNavigated := o.hasNavigated | |||
| 1859 | + | o.mu.RUnlock() | |||
| 1860 | + | ||||
| 1861 | + | if !shouldPromoteHoverSelection(realInput, hasGhostText, hasNavigated) { | |||
| 1862 | + | return | |||
| 1863 | + | } | |||
| 1855 | o.selectIndex(index) | 1864 | o.selectIndex(index) | ||
| 1856 | } | 1865 | } | ||
| 1857 | o.retainedCallbacks = append(o.retainedCallbacks, enterCb) | 1866 | o.retainedCallbacks = append(o.retainedCallbacks, enterCb) | ||
| @@ -1859,6 +1868,10 @@ func (o *Omnibox) attachRowHoverSelection(row *gtk.ListBoxRow, index int) { | |||||
| 1859 | row.AddController(&motionCtrl.EventController) | 1868 | row.AddController(&motionCtrl.EventController) | ||
| 1860 | } | 1869 | } | ||
| 1861 | 1870 | ||||
| 1871 | + | func shouldPromoteHoverSelection(realInput string, hasGhostText, hasNavigated bool) bool { | |||
| 1872 | + | return realInput == "" && !hasGhostText && !hasNavigated | |||
| 1873 | + | } | |||
| 1874 | + | ||||
| 1862 | // createSuggestionRow creates a ListBoxRow for a suggestion. | 1875 | // createSuggestionRow creates a ListBoxRow for a suggestion. | ||
| 1863 | func (o *Omnibox) createSuggestionRow(s Suggestion, index int) *gtk.ListBoxRow { | 1876 | func (o *Omnibox) createSuggestionRow(s Suggestion, index int) *gtk.ListBoxRow { | ||
| 1864 | row := o.createRowWithFavicon(s.URL, s.Title, "web-browser-symbolic", index) | 1877 | row := o.createRowWithFavicon(s.URL, s.Title, "web-browser-symbolic", index) | ||
M internal/ui/component/omnibox_async_guard_test.go +8 -0
| @@ -35,6 +35,14 @@ func TestOmnibox_IsGhostTokenCurrent(t *testing.T) { | |||
| 35 | 35 | if o.isGhostTokenCurrent(5, "github") { | |
| 36 | 36 | t.Fatalf("expected query mismatch to be rejected") | |
| 37 | 37 | } | |
| 38 | + | ||
| 39 | + | o.visible = false | |
| 40 | + | if o.isGhostTokenCurrent(5, "git") { | |
| 41 | + | t.Fatalf("expected hidden omnibox to reject matching ghost token") | |
| 42 | + | } | |
| 43 | + | if o.isGhostTokenCurrent(4, "github") { | |
| 44 | + | t.Fatalf("expected hidden omnibox to reject stale ghost token and query") | |
| 45 | + | } | |
| 38 | 46 | } | |
| 39 | 47 | ||
| 40 | 48 | func TestOmnibox_IsBangUpdateCurrent(t *testing.T) { | |
| @@ -35,6 +35,14 @@ func TestOmnibox_IsGhostTokenCurrent(t *testing.T) { | |||||
| 35 | if o.isGhostTokenCurrent(5, "github") { | 35 | if o.isGhostTokenCurrent(5, "github") { | ||
| 36 | t.Fatalf("expected query mismatch to be rejected") | 36 | t.Fatalf("expected query mismatch to be rejected") | ||
| 37 | } | 37 | } | ||
| 38 | + | ||||
| 39 | + | o.visible = false | |||
| 40 | + | if o.isGhostTokenCurrent(5, "git") { | |||
| 41 | + | t.Fatalf("expected hidden omnibox to reject matching ghost token") | |||
| 42 | + | } | |||
| 43 | + | if o.isGhostTokenCurrent(4, "github") { | |||
| 44 | + | t.Fatalf("expected hidden omnibox to reject stale ghost token and query") | |||
| 45 | + | } | |||
| 38 | } | 46 | } | ||
| 39 | 47 | ||||
| 40 | func TestOmnibox_IsBangUpdateCurrent(t *testing.T) { | 48 | func TestOmnibox_IsBangUpdateCurrent(t *testing.T) { | ||
M internal/ui/component/omnibox_navigation_state_test.go +46 -0
| @@ -251,3 +251,49 @@ func TestVisibleGhostSuggestion(t *testing.T) { | |||
| 251 | 251 | }) | |
| 252 | 252 | } | |
| 253 | 253 | } | |
| 254 | + | ||
| 255 | + | func TestShouldPromoteHoverSelection(t *testing.T) { | |
| 256 | + | tests := []struct { | |
| 257 | + | name string | |
| 258 | + | realInput string | |
| 259 | + | hasGhostText bool | |
| 260 | + | hasNavigated bool | |
| 261 | + | want bool | |
| 262 | + | }{ | |
| 263 | + | { | |
| 264 | + | name: "initial open allows hover selection", | |
| 265 | + | want: true, | |
| 266 | + | }, | |
| 267 | + | { | |
| 268 | + | name: "typed input keeps hover from stealing authority", | |
| 269 | + | realInput: "git", | |
| 270 | + | want: false, | |
| 271 | + | }, | |
| 272 | + | { | |
| 273 | + | name: "visible ghost keeps hover from stealing authority", | |
| 274 | + | hasGhostText: true, | |
| 275 | + | want: false, | |
| 276 | + | }, | |
| 277 | + | { | |
| 278 | + | name: "keyboard navigation keeps hover from stealing authority", | |
| 279 | + | hasNavigated: true, | |
| 280 | + | want: false, | |
| 281 | + | }, | |
| 282 | + | } | |
| 283 | + | ||
| 284 | + | for _, tt := range tests { | |
| 285 | + | t.Run(tt.name, func(t *testing.T) { | |
| 286 | + | got := shouldPromoteHoverSelection(tt.realInput, tt.hasGhostText, tt.hasNavigated) | |
| 287 | + | if got != tt.want { | |
| 288 | + | t.Fatalf( | |
| 289 | + | "shouldPromoteHoverSelection(%q, %v, %v) = %v, want %v", | |
| 290 | + | tt.realInput, | |
| 291 | + | tt.hasGhostText, | |
| 292 | + | tt.hasNavigated, | |
| 293 | + | got, | |
| 294 | + | tt.want, | |
| 295 | + | ) | |
| 296 | + | } | |
| 297 | + | }) | |
| 298 | + | } | |
| 299 | + | } | |
| @@ -251,3 +251,49 @@ func TestVisibleGhostSuggestion(t *testing.T) { | |||||
| 251 | }) | 251 | }) | ||
| 252 | } | 252 | } | ||
| 253 | } | 253 | } | ||
| 254 | + | ||||
| 255 | + | func TestShouldPromoteHoverSelection(t *testing.T) { | |||
| 256 | + | tests := []struct { | |||
| 257 | + | name string | |||
| 258 | + | realInput string | |||
| 259 | + | hasGhostText bool | |||
| 260 | + | hasNavigated bool | |||
| 261 | + | want bool | |||
| 262 | + | }{ | |||
| 263 | + | { | |||
| 264 | + | name: "initial open allows hover selection", | |||
| 265 | + | want: true, | |||
| 266 | + | }, | |||
| 267 | + | { | |||
| 268 | + | name: "typed input keeps hover from stealing authority", | |||
| 269 | + | realInput: "git", | |||
| 270 | + | want: false, | |||
| 271 | + | }, | |||
| 272 | + | { | |||
| 273 | + | name: "visible ghost keeps hover from stealing authority", | |||
| 274 | + | hasGhostText: true, | |||
| 275 | + | want: false, | |||
| 276 | + | }, | |||
| 277 | + | { | |||
| 278 | + | name: "keyboard navigation keeps hover from stealing authority", | |||
| 279 | + | hasNavigated: true, | |||
| 280 | + | want: false, | |||
| 281 | + | }, | |||
| 282 | + | } | |||
| 283 | + | ||||
| 284 | + | for _, tt := range tests { | |||
| 285 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 286 | + | got := shouldPromoteHoverSelection(tt.realInput, tt.hasGhostText, tt.hasNavigated) | |||
| 287 | + | if got != tt.want { | |||
| 288 | + | t.Fatalf( | |||
| 289 | + | "shouldPromoteHoverSelection(%q, %v, %v) = %v, want %v", | |||
| 290 | + | tt.realInput, | |||
| 291 | + | tt.hasGhostText, | |||
| 292 | + | tt.hasNavigated, | |||
| 293 | + | got, | |||
| 294 | + | tt.want, | |||
| 295 | + | ) | |||
| 296 | + | } | |||
| 297 | + | }) | |||
| 298 | + | } | |||
| 299 | + | } | |||