fix(omnibox): separate favicon URL from display URL in row creation
- commit
- 81ee824f973962d58412c011c6733ab2504efc98
- author
- brice · · 2025-12-28 14:56 UTC
- parent
- 0dc071d
- signature
- unsigned
2 files · +32 -18
M internal/application/usecase/search_shortcuts_test.go +18 -13
| @@ -76,30 +76,35 @@ func TestFilterBangs(t *testing.T) { | |||
| 76 | 76 | ||
| 77 | 77 | func TestDetectBangKey(t *testing.T) { | |
| 78 | 78 | shortcuts := map[string]SearchShortcut{ | |
| 79 | - | "gh": {URL: "https://github.com/search?q=%s"}, | |
| 80 | - | "ddg": {URL: "https://duckduckgo.com/?q=%s"}, | |
| 79 | + | "gh": {URL: "https://github.com/search?q=%s", Description: "GitHub search"}, | |
| 80 | + | "ddg": {URL: "https://duckduckgo.com/?q=%s", Description: ""}, | |
| 81 | 81 | } | |
| 82 | 82 | uc := NewSearchShortcutsUseCase(shortcuts) | |
| 83 | 83 | ctx := context.Background() | |
| 84 | 84 | ||
| 85 | 85 | cases := []struct { | |
| 86 | - | name string | |
| 87 | - | query string | |
| 88 | - | want string | |
| 86 | + | name string | |
| 87 | + | query string | |
| 88 | + | wantKey string | |
| 89 | + | wantDescr string | |
| 89 | 90 | }{ | |
| 90 | - | {name: "no bang prefix", query: "gh test", want: ""}, | |
| 91 | - | {name: "bang only has no space", query: "!gh", want: ""}, | |
| 92 | - | {name: "space at position 1", query: "! test", want: ""}, | |
| 93 | - | {name: "unknown bang", query: "!nope test", want: ""}, | |
| 94 | - | {name: "case-insensitive match", query: "!GH test", want: "gh"}, | |
| 95 | - | {name: "valid bang key", query: "!ddg test", want: "ddg"}, | |
| 91 | + | {name: "no bang prefix", query: "gh test", wantKey: "", wantDescr: ""}, | |
| 92 | + | {name: "bang only has no space", query: "!gh", wantKey: "", wantDescr: ""}, | |
| 93 | + | {name: "space at position 1", query: "! test", wantKey: "", wantDescr: ""}, | |
| 94 | + | {name: "unknown bang", query: "!nope test", wantKey: "", wantDescr: ""}, | |
| 95 | + | {name: "case-insensitive match", query: "!GH test", wantKey: "gh", wantDescr: "GitHub search"}, | |
| 96 | + | {name: "valid bang key", query: "!ddg test", wantKey: "ddg", wantDescr: "https://duckduckgo.com/?q=%s"}, | |
| 97 | + | {name: "description fallback to URL", query: "!DDG query", wantKey: "ddg", wantDescr: "https://duckduckgo.com/?q=%s"}, | |
| 96 | 98 | } | |
| 97 | 99 | ||
| 98 | 100 | for _, tc := range cases { | |
| 99 | 101 | t.Run(tc.name, func(t *testing.T) { | |
| 100 | 102 | output := uc.DetectBangKey(ctx, DetectBangKeyInput{Query: tc.query}) | |
| 101 | - | if output.Key != tc.want { | |
| 102 | - | t.Fatalf("DetectBangKey(%q)=%q want=%q", tc.query, output.Key, tc.want) | |
| 103 | + | if output.Key != tc.wantKey { | |
| 104 | + | t.Fatalf("DetectBangKey(%q).Key=%q want=%q", tc.query, output.Key, tc.wantKey) | |
| 105 | + | } | |
| 106 | + | if output.Description != tc.wantDescr { | |
| 107 | + | t.Fatalf("DetectBangKey(%q).Description=%q want=%q", tc.query, output.Description, tc.wantDescr) | |
| 103 | 108 | } | |
| 104 | 109 | }) | |
| 105 | 110 | } | |
| @@ -76,30 +76,35 @@ func TestFilterBangs(t *testing.T) { | |||||
| 76 | 76 | ||||
| 77 | func TestDetectBangKey(t *testing.T) { | 77 | func TestDetectBangKey(t *testing.T) { | ||
| 78 | shortcuts := map[string]SearchShortcut{ | 78 | shortcuts := map[string]SearchShortcut{ | ||
| 79 | - | "gh": {URL: "https://github.com/search?q=%s"}, | 79 | + | "gh": {URL: "https://github.com/search?q=%s", Description: "GitHub search"}, |
| 80 | - | "ddg": {URL: "https://duckduckgo.com/?q=%s"}, | 80 | + | "ddg": {URL: "https://duckduckgo.com/?q=%s", Description: ""}, |
| 81 | } | 81 | } | ||
| 82 | uc := NewSearchShortcutsUseCase(shortcuts) | 82 | uc := NewSearchShortcutsUseCase(shortcuts) | ||
| 83 | ctx := context.Background() | 83 | ctx := context.Background() | ||
| 84 | 84 | ||||
| 85 | cases := []struct { | 85 | cases := []struct { | ||
| 86 | - | name string | 86 | + | name string |
| 87 | - | query string | 87 | + | query string |
| 88 | - | want string | 88 | + | wantKey string |
| 89 | + | wantDescr string | |||
| 89 | }{ | 90 | }{ | ||
| 90 | - | {name: "no bang prefix", query: "gh test", want: ""}, | 91 | + | {name: "no bang prefix", query: "gh test", wantKey: "", wantDescr: ""}, |
| 91 | - | {name: "bang only has no space", query: "!gh", want: ""}, | 92 | + | {name: "bang only has no space", query: "!gh", wantKey: "", wantDescr: ""}, |
| 92 | - | {name: "space at position 1", query: "! test", want: ""}, | 93 | + | {name: "space at position 1", query: "! test", wantKey: "", wantDescr: ""}, |
| 93 | - | {name: "unknown bang", query: "!nope test", want: ""}, | 94 | + | {name: "unknown bang", query: "!nope test", wantKey: "", wantDescr: ""}, |
| 94 | - | {name: "case-insensitive match", query: "!GH test", want: "gh"}, | 95 | + | {name: "case-insensitive match", query: "!GH test", wantKey: "gh", wantDescr: "GitHub search"}, |
| 95 | - | {name: "valid bang key", query: "!ddg test", want: "ddg"}, | 96 | + | {name: "valid bang key", query: "!ddg test", wantKey: "ddg", wantDescr: "https://duckduckgo.com/?q=%s"}, |
| 97 | + | {name: "description fallback to URL", query: "!DDG query", wantKey: "ddg", wantDescr: "https://duckduckgo.com/?q=%s"}, | |||
| 96 | } | 98 | } | ||
| 97 | 99 | ||||
| 98 | for _, tc := range cases { | 100 | for _, tc := range cases { | ||
| 99 | t.Run(tc.name, func(t *testing.T) { | 101 | t.Run(tc.name, func(t *testing.T) { | ||
| 100 | output := uc.DetectBangKey(ctx, DetectBangKeyInput{Query: tc.query}) | 102 | output := uc.DetectBangKey(ctx, DetectBangKeyInput{Query: tc.query}) | ||
| 101 | - | if output.Key != tc.want { | 103 | + | if output.Key != tc.wantKey { |
| 102 | - | t.Fatalf("DetectBangKey(%q)=%q want=%q", tc.query, output.Key, tc.want) | 104 | + | t.Fatalf("DetectBangKey(%q).Key=%q want=%q", tc.query, output.Key, tc.wantKey) |
| 105 | + | } | |||
| 106 | + | if output.Description != tc.wantDescr { | |||
| 107 | + | t.Fatalf("DetectBangKey(%q).Description=%q want=%q", tc.query, output.Description, tc.wantDescr) | |||
| 103 | } | 108 | } | ||
| 104 | }) | 109 | }) | ||
| 105 | } | 110 | } | ||
M internal/ui/component/omnibox.go +14 -5
| @@ -1017,7 +1017,14 @@ func (o *Omnibox) createFaviconImage(rawURL, fallbackIcon string) *gtk.Image { | |||
| 1017 | 1017 | } | |
| 1018 | 1018 | ||
| 1019 | 1019 | // createRowWithFavicon creates a ListBoxRow with favicon, title, URL, and shortcut badge. | |
| 1020 | + | // Uses rawURL for both favicon fetching and display. | |
| 1020 | 1021 | func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index int) *gtk.ListBoxRow { | |
| 1022 | + | return o.createRowWithFaviconURL(rawURL, title, rawURL, fallbackIcon, index) | |
| 1023 | + | } | |
| 1024 | + | ||
| 1025 | + | // createRowWithFaviconURL creates a ListBoxRow with favicon, title, URL label, and shortcut badge. | |
| 1026 | + | // faviconURL is used for async favicon fetching (can be empty to skip), displayURL is shown as secondary label. | |
| 1027 | + | func (o *Omnibox) createRowWithFaviconURL(displayURL, title, faviconURL, fallbackIcon string, index int) *gtk.ListBoxRow { | |
| 1021 | 1028 | row := gtk.NewListBoxRow() | |
| 1022 | 1029 | if row == nil { | |
| 1023 | 1030 | return nil | |
| @@ -1032,7 +1039,7 @@ func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index | |||
| 1032 | 1039 | hbox.SetHexpand(true) | |
| 1033 | 1040 | ||
| 1034 | 1041 | // Favicon image (vertically centered) | |
| 1035 | - | if favicon := o.createFaviconImage(rawURL, fallbackIcon); favicon != nil { | |
| 1042 | + | if favicon := o.createFaviconImage(faviconURL, fallbackIcon); favicon != nil { | |
| 1036 | 1043 | favicon.SetValign(gtk.AlignCenterValue) | |
| 1037 | 1044 | hbox.Append(&favicon.Widget) | |
| 1038 | 1045 | } | |
| @@ -1048,7 +1055,7 @@ func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index | |||
| 1048 | 1055 | // Title label (or URL if no title) | |
| 1049 | 1056 | displayTitle := title | |
| 1050 | 1057 | if displayTitle == "" { | |
| 1051 | - | displayTitle = rawURL | |
| 1058 | + | displayTitle = displayURL | |
| 1052 | 1059 | } | |
| 1053 | 1060 | titleLabel := gtk.NewLabel(nil) | |
| 1054 | 1061 | if titleLabel != nil { | |
| @@ -1060,10 +1067,10 @@ func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index | |||
| 1060 | 1067 | } | |
| 1061 | 1068 | ||
| 1062 | 1069 | // URL label (only if title exists and differs from URL) | |
| 1063 | - | if title != "" && title != rawURL { | |
| 1070 | + | if title != "" && title != displayURL { | |
| 1064 | 1071 | urlLabel := gtk.NewLabel(nil) | |
| 1065 | 1072 | if urlLabel != nil { | |
| 1066 | - | urlLabel.SetText(rawURL) | |
| 1073 | + | urlLabel.SetText(displayURL) | |
| 1067 | 1074 | urlLabel.AddCssClass("omnibox-suggestion-url") | |
| 1068 | 1075 | urlLabel.SetHalign(gtk.AlignStartValue) | |
| 1069 | 1076 | urlLabel.SetEllipsize(2) // PANGO_ELLIPSIZE_END | |
| @@ -1108,7 +1115,9 @@ func (o *Omnibox) createFavoriteRow(f Favorite, index int) *gtk.ListBoxRow { | |||
| 1108 | 1115 | } | |
| 1109 | 1116 | ||
| 1110 | 1117 | func (o *Omnibox) createBangRow(b BangSuggestion, index int) *gtk.ListBoxRow { | |
| 1111 | - | row := o.createRowWithFavicon(b.Description, "!"+b.Key, "system-search-symbolic", index) | |
| 1118 | + | // Pass description as URL param (displayed as secondary label) and empty | |
| 1119 | + | // faviconURL to skip async favicon fetching - bang rows use static icon only | |
| 1120 | + | row := o.createRowWithFaviconURL(b.Description, "!"+b.Key, "", "system-search-symbolic", index) | |
| 1112 | 1121 | if row != nil { | |
| 1113 | 1122 | row.AddCssClass("omnibox-row-bang") | |
| 1114 | 1123 | } | |
| @@ -1017,7 +1017,14 @@ func (o *Omnibox) createFaviconImage(rawURL, fallbackIcon string) *gtk.Image { | |||||
| 1017 | } | 1017 | } | ||
| 1018 | 1018 | ||||
| 1019 | // createRowWithFavicon creates a ListBoxRow with favicon, title, URL, and shortcut badge. | 1019 | // createRowWithFavicon creates a ListBoxRow with favicon, title, URL, and shortcut badge. | ||
| 1020 | + | // Uses rawURL for both favicon fetching and display. | |||
| 1020 | func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index int) *gtk.ListBoxRow { | 1021 | func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index int) *gtk.ListBoxRow { | ||
| 1022 | + | return o.createRowWithFaviconURL(rawURL, title, rawURL, fallbackIcon, index) | |||
| 1023 | + | } | |||
| 1024 | + | ||||
| 1025 | + | // createRowWithFaviconURL creates a ListBoxRow with favicon, title, URL label, and shortcut badge. | |||
| 1026 | + | // faviconURL is used for async favicon fetching (can be empty to skip), displayURL is shown as secondary label. | |||
| 1027 | + | func (o *Omnibox) createRowWithFaviconURL(displayURL, title, faviconURL, fallbackIcon string, index int) *gtk.ListBoxRow { | |||
| 1021 | row := gtk.NewListBoxRow() | 1028 | row := gtk.NewListBoxRow() | ||
| 1022 | if row == nil { | 1029 | if row == nil { | ||
| 1023 | return nil | 1030 | return nil | ||
| @@ -1032,7 +1039,7 @@ func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index | |||||
| 1032 | hbox.SetHexpand(true) | 1039 | hbox.SetHexpand(true) | ||
| 1033 | 1040 | ||||
| 1034 | // Favicon image (vertically centered) | 1041 | // Favicon image (vertically centered) | ||
| 1035 | - | if favicon := o.createFaviconImage(rawURL, fallbackIcon); favicon != nil { | 1042 | + | if favicon := o.createFaviconImage(faviconURL, fallbackIcon); favicon != nil { |
| 1036 | favicon.SetValign(gtk.AlignCenterValue) | 1043 | favicon.SetValign(gtk.AlignCenterValue) | ||
| 1037 | hbox.Append(&favicon.Widget) | 1044 | hbox.Append(&favicon.Widget) | ||
| 1038 | } | 1045 | } | ||
| @@ -1048,7 +1055,7 @@ func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index | |||||
| 1048 | // Title label (or URL if no title) | 1055 | // Title label (or URL if no title) | ||
| 1049 | displayTitle := title | 1056 | displayTitle := title | ||
| 1050 | if displayTitle == "" { | 1057 | if displayTitle == "" { | ||
| 1051 | - | displayTitle = rawURL | 1058 | + | displayTitle = displayURL |
| 1052 | } | 1059 | } | ||
| 1053 | titleLabel := gtk.NewLabel(nil) | 1060 | titleLabel := gtk.NewLabel(nil) | ||
| 1054 | if titleLabel != nil { | 1061 | if titleLabel != nil { | ||
| @@ -1060,10 +1067,10 @@ func (o *Omnibox) createRowWithFavicon(rawURL, title, fallbackIcon string, index | |||||
| 1060 | } | 1067 | } | ||
| 1061 | 1068 | ||||
| 1062 | // URL label (only if title exists and differs from URL) | 1069 | // URL label (only if title exists and differs from URL) | ||
| 1063 | - | if title != "" && title != rawURL { | 1070 | + | if title != "" && title != displayURL { |
| 1064 | urlLabel := gtk.NewLabel(nil) | 1071 | urlLabel := gtk.NewLabel(nil) | ||
| 1065 | if urlLabel != nil { | 1072 | if urlLabel != nil { | ||
| 1066 | - | urlLabel.SetText(rawURL) | 1073 | + | urlLabel.SetText(displayURL) |
| 1067 | urlLabel.AddCssClass("omnibox-suggestion-url") | 1074 | urlLabel.AddCssClass("omnibox-suggestion-url") | ||
| 1068 | urlLabel.SetHalign(gtk.AlignStartValue) | 1075 | urlLabel.SetHalign(gtk.AlignStartValue) | ||
| 1069 | urlLabel.SetEllipsize(2) // PANGO_ELLIPSIZE_END | 1076 | urlLabel.SetEllipsize(2) // PANGO_ELLIPSIZE_END | ||
| @@ -1108,7 +1115,9 @@ func (o *Omnibox) createFavoriteRow(f Favorite, index int) *gtk.ListBoxRow { | |||||
| 1108 | } | 1115 | } | ||
| 1109 | 1116 | ||||
| 1110 | func (o *Omnibox) createBangRow(b BangSuggestion, index int) *gtk.ListBoxRow { | 1117 | func (o *Omnibox) createBangRow(b BangSuggestion, index int) *gtk.ListBoxRow { | ||
| 1111 | - | row := o.createRowWithFavicon(b.Description, "!"+b.Key, "system-search-symbolic", index) | 1118 | + | // Pass description as URL param (displayed as secondary label) and empty |
| 1119 | + | // faviconURL to skip async favicon fetching - bang rows use static icon only | |||
| 1120 | + | row := o.createRowWithFaviconURL(b.Description, "!"+b.Key, "", "system-search-symbolic", index) | |||
| 1112 | if row != nil { | 1121 | if row != nil { | ||
| 1113 | row.AddCssClass("omnibox-row-bang") | 1122 | row.AddCssClass("omnibox-row-bang") | ||
| 1114 | } | 1123 | } | ||