refactor: apply URL normalization across browser, CLI, and favicon services
- commit
- 992a34aeae8f289083f281a04a8c9895e3f88971
- author
- bnema · · 2025-11-19 18:24 UTC
- parent
- 551b57d
- signature
- GPG Unverified
2B82207D370127A0The key is unknown
3 files · +104 -32
M internal/cli/browse.go +8 -3
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | "strings" | |
| 11 | 11 | ||
| 12 | 12 | "github.com/bnema/dumber/internal/config" | |
| 13 | + | "github.com/bnema/dumber/internal/parser" | |
| 13 | 14 | ||
| 14 | 15 | "github.com/spf13/cobra" | |
| 15 | 16 | ) | |
| @@ -89,12 +90,14 @@ func browse(cli *CLI, input string) error { | |||
| 89 | 90 | ||
| 90 | 91 | // recordVisit adds or updates a URL visit in the history | |
| 91 | 92 | func recordVisit(ctx context.Context, cli *CLI, url, title string) error { | |
| 93 | + | normalizedURL := parser.NormalizeHistoryURL(url) | |
| 94 | + | ||
| 92 | 95 | var sqlTitle sql.NullString | |
| 93 | 96 | if title != "" { | |
| 94 | 97 | sqlTitle = sql.NullString{String: title, Valid: true} | |
| 95 | 98 | } | |
| 96 | 99 | ||
| 97 | - | return cli.Queries.AddOrUpdateHistory(ctx, url, sqlTitle) | |
| 100 | + | return cli.Queries.AddOrUpdateHistory(ctx, normalizedURL, sqlTitle) | |
| 98 | 101 | } | |
| 99 | 102 | ||
| 100 | 103 | // openURL opens a URL using the configured browser | |
| @@ -141,7 +144,9 @@ func openURLWithConfig(url string, cfg *config.Config) error { | |||
| 141 | 144 | func updateFavicon(ctx context.Context, cli *CLI, pageURL string) { | |
| 142 | 145 | // For CLI usage (non-GUI), we still use the fallback method | |
| 143 | 146 | // The GUI browser handles favicons through WebKit signals | |
| 144 | - | parsedURL, err := url.Parse(pageURL) | |
| 147 | + | normalizedURL := parser.NormalizeHistoryURL(pageURL) | |
| 148 | + | ||
| 149 | + | parsedURL, err := url.Parse(normalizedURL) | |
| 145 | 150 | if err != nil { | |
| 146 | 151 | return // Silently fail for invalid URLs | |
| 147 | 152 | } | |
| @@ -159,7 +164,7 @@ func updateFavicon(ctx context.Context, cli *CLI, pageURL string) { | |||
| 159 | 164 | ||
| 160 | 165 | // Update in database using the new sqlc-generated method | |
| 161 | 166 | faviconNullString := sql.NullString{String: faviconURL, Valid: true} | |
| 162 | - | if err := cli.Queries.UpdateHistoryFavicon(ctx, faviconNullString, pageURL); err != nil { | |
| 167 | + | if err := cli.Queries.UpdateHistoryFavicon(ctx, faviconNullString, normalizedURL); err != nil { | |
| 163 | 168 | return | |
| 164 | 169 | } | |
| 165 | 170 | } | |
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | "strings" | 10 | "strings" | ||
| 11 | 11 | ||||
| 12 | "github.com/bnema/dumber/internal/config" | 12 | "github.com/bnema/dumber/internal/config" | ||
| 13 | + | "github.com/bnema/dumber/internal/parser" | |||
| 13 | 14 | ||||
| 14 | "github.com/spf13/cobra" | 15 | "github.com/spf13/cobra" | ||
| 15 | ) | 16 | ) | ||
| @@ -89,12 +90,14 @@ func browse(cli *CLI, input string) error { | |||||
| 89 | 90 | ||||
| 90 | // recordVisit adds or updates a URL visit in the history | 91 | // recordVisit adds or updates a URL visit in the history | ||
| 91 | func recordVisit(ctx context.Context, cli *CLI, url, title string) error { | 92 | func recordVisit(ctx context.Context, cli *CLI, url, title string) error { | ||
| 93 | + | normalizedURL := parser.NormalizeHistoryURL(url) | |||
| 94 | + | ||||
| 92 | var sqlTitle sql.NullString | 95 | var sqlTitle sql.NullString | ||
| 93 | if title != "" { | 96 | if title != "" { | ||
| 94 | sqlTitle = sql.NullString{String: title, Valid: true} | 97 | sqlTitle = sql.NullString{String: title, Valid: true} | ||
| 95 | } | 98 | } | ||
| 96 | 99 | ||||
| 97 | - | return cli.Queries.AddOrUpdateHistory(ctx, url, sqlTitle) | 100 | + | return cli.Queries.AddOrUpdateHistory(ctx, normalizedURL, sqlTitle) |
| 98 | } | 101 | } | ||
| 99 | 102 | ||||
| 100 | // openURL opens a URL using the configured browser | 103 | // openURL opens a URL using the configured browser | ||
| @@ -141,7 +144,9 @@ func openURLWithConfig(url string, cfg *config.Config) error { | |||||
| 141 | func updateFavicon(ctx context.Context, cli *CLI, pageURL string) { | 144 | func updateFavicon(ctx context.Context, cli *CLI, pageURL string) { | ||
| 142 | // For CLI usage (non-GUI), we still use the fallback method | 145 | // For CLI usage (non-GUI), we still use the fallback method | ||
| 143 | // The GUI browser handles favicons through WebKit signals | 146 | // The GUI browser handles favicons through WebKit signals | ||
| 144 | - | parsedURL, err := url.Parse(pageURL) | 147 | + | normalizedURL := parser.NormalizeHistoryURL(pageURL) |
| 148 | + | ||||
| 149 | + | parsedURL, err := url.Parse(normalizedURL) | |||
| 145 | if err != nil { | 150 | if err != nil { | ||
| 146 | return // Silently fail for invalid URLs | 151 | return // Silently fail for invalid URLs | ||
| 147 | } | 152 | } | ||
| @@ -159,7 +164,7 @@ func updateFavicon(ctx context.Context, cli *CLI, pageURL string) { | |||||
| 159 | 164 | ||||
| 160 | // Update in database using the new sqlc-generated method | 165 | // Update in database using the new sqlc-generated method | ||
| 161 | faviconNullString := sql.NullString{String: faviconURL, Valid: true} | 166 | faviconNullString := sql.NullString{String: faviconURL, Valid: true} | ||
| 162 | - | if err := cli.Queries.UpdateHistoryFavicon(ctx, faviconNullString, pageURL); err != nil { | 167 | + | if err := cli.Queries.UpdateHistoryFavicon(ctx, faviconNullString, normalizedURL); err != nil { |
| 163 | return | 168 | return | ||
| 164 | } | 169 | } | ||
| 165 | } | 170 | } | ||
M internal/services/browser.go +8 -3
| @@ -15,6 +15,7 @@ import ( | |||
| 15 | 15 | "github.com/bnema/dumber/internal/cache" | |
| 16 | 16 | "github.com/bnema/dumber/internal/config" | |
| 17 | 17 | "github.com/bnema/dumber/internal/db" | |
| 18 | + | "github.com/bnema/dumber/internal/parser" | |
| 18 | 19 | "github.com/bnema/dumber/pkg/webkit" | |
| 19 | 20 | ) | |
| 20 | 21 | ||
| @@ -213,9 +214,11 @@ func (s *BrowserService) UpdatePageTitle(ctx context.Context, url, title string) | |||
| 213 | 214 | return fmt.Errorf("URL and title cannot be empty") | |
| 214 | 215 | } | |
| 215 | 216 | ||
| 217 | + | normalizedURL := parser.NormalizeHistoryURL(url) | |
| 218 | + | ||
| 216 | 219 | // Use AddOrUpdateHistory to update the title | |
| 217 | 220 | titleNull := sql.NullString{String: title, Valid: true} | |
| 218 | - | err := s.dbQueries.AddOrUpdateHistory(ctx, url, titleNull) | |
| 221 | + | err := s.dbQueries.AddOrUpdateHistory(ctx, normalizedURL, titleNull) | |
| 219 | 222 | if err != nil { | |
| 220 | 223 | return err | |
| 221 | 224 | } | |
| @@ -421,6 +424,8 @@ func (s *BrowserService) GetSearchShortcuts(ctx context.Context) (map[string]con | |||
| 421 | 424 | // recordHistory adds or updates a history entry. | |
| 422 | 425 | // Now queues the update for batched processing instead of immediate DB write. | |
| 423 | 426 | func (s *BrowserService) recordHistory(ctx context.Context, url, title string) error { | |
| 427 | + | normalizedURL := parser.NormalizeHistoryURL(url) | |
| 428 | + | ||
| 424 | 429 | titleNull := sql.NullString{Valid: false} | |
| 425 | 430 | if title != "" { | |
| 426 | 431 | titleNull = sql.NullString{String: title, Valid: true} | |
| @@ -428,13 +433,13 @@ func (s *BrowserService) recordHistory(ctx context.Context, url, title string) e | |||
| 428 | 433 | ||
| 429 | 434 | // Queue the history update (non-blocking with buffer) | |
| 430 | 435 | select { | |
| 431 | - | case s.historyQueue <- historyUpdate{url: url, title: titleNull}: | |
| 436 | + | case s.historyQueue <- historyUpdate{url: normalizedURL, title: titleNull}: | |
| 432 | 437 | // Successfully queued | |
| 433 | 438 | return nil | |
| 434 | 439 | default: | |
| 435 | 440 | // Queue full, write directly (fallback) | |
| 436 | 441 | log.Printf("Warning: history queue full, writing directly") | |
| 437 | - | return s.dbQueries.AddOrUpdateHistory(ctx, url, titleNull) | |
| 442 | + | return s.dbQueries.AddOrUpdateHistory(ctx, normalizedURL, titleNull) | |
| 438 | 443 | } | |
| 439 | 444 | } | |
| 440 | 445 | ||
| @@ -15,6 +15,7 @@ import ( | |||||
| 15 | "github.com/bnema/dumber/internal/cache" | 15 | "github.com/bnema/dumber/internal/cache" | ||
| 16 | "github.com/bnema/dumber/internal/config" | 16 | "github.com/bnema/dumber/internal/config" | ||
| 17 | "github.com/bnema/dumber/internal/db" | 17 | "github.com/bnema/dumber/internal/db" | ||
| 18 | + | "github.com/bnema/dumber/internal/parser" | |||
| 18 | "github.com/bnema/dumber/pkg/webkit" | 19 | "github.com/bnema/dumber/pkg/webkit" | ||
| 19 | ) | 20 | ) | ||
| 20 | 21 | ||||
| @@ -213,9 +214,11 @@ func (s *BrowserService) UpdatePageTitle(ctx context.Context, url, title string) | |||||
| 213 | return fmt.Errorf("URL and title cannot be empty") | 214 | return fmt.Errorf("URL and title cannot be empty") | ||
| 214 | } | 215 | } | ||
| 215 | 216 | ||||
| 217 | + | normalizedURL := parser.NormalizeHistoryURL(url) | |||
| 218 | + | ||||
| 216 | // Use AddOrUpdateHistory to update the title | 219 | // Use AddOrUpdateHistory to update the title | ||
| 217 | titleNull := sql.NullString{String: title, Valid: true} | 220 | titleNull := sql.NullString{String: title, Valid: true} | ||
| 218 | - | err := s.dbQueries.AddOrUpdateHistory(ctx, url, titleNull) | 221 | + | err := s.dbQueries.AddOrUpdateHistory(ctx, normalizedURL, titleNull) |
| 219 | if err != nil { | 222 | if err != nil { | ||
| 220 | return err | 223 | return err | ||
| 221 | } | 224 | } | ||
| @@ -421,6 +424,8 @@ func (s *BrowserService) GetSearchShortcuts(ctx context.Context) (map[string]con | |||||
| 421 | // recordHistory adds or updates a history entry. | 424 | // recordHistory adds or updates a history entry. | ||
| 422 | // Now queues the update for batched processing instead of immediate DB write. | 425 | // Now queues the update for batched processing instead of immediate DB write. | ||
| 423 | func (s *BrowserService) recordHistory(ctx context.Context, url, title string) error { | 426 | func (s *BrowserService) recordHistory(ctx context.Context, url, title string) error { | ||
| 427 | + | normalizedURL := parser.NormalizeHistoryURL(url) | |||
| 428 | + | ||||
| 424 | titleNull := sql.NullString{Valid: false} | 429 | titleNull := sql.NullString{Valid: false} | ||
| 425 | if title != "" { | 430 | if title != "" { | ||
| 426 | titleNull = sql.NullString{String: title, Valid: true} | 431 | titleNull = sql.NullString{String: title, Valid: true} | ||
| @@ -428,13 +433,13 @@ func (s *BrowserService) recordHistory(ctx context.Context, url, title string) e | |||||
| 428 | 433 | ||||
| 429 | // Queue the history update (non-blocking with buffer) | 434 | // Queue the history update (non-blocking with buffer) | ||
| 430 | select { | 435 | select { | ||
| 431 | - | case s.historyQueue <- historyUpdate{url: url, title: titleNull}: | 436 | + | case s.historyQueue <- historyUpdate{url: normalizedURL, title: titleNull}: |
| 432 | // Successfully queued | 437 | // Successfully queued | ||
| 433 | return nil | 438 | return nil | ||
| 434 | default: | 439 | default: | ||
| 435 | // Queue full, write directly (fallback) | 440 | // Queue full, write directly (fallback) | ||
| 436 | log.Printf("Warning: history queue full, writing directly") | 441 | log.Printf("Warning: history queue full, writing directly") | ||
| 437 | - | return s.dbQueries.AddOrUpdateHistory(ctx, url, titleNull) | 442 | + | return s.dbQueries.AddOrUpdateHistory(ctx, normalizedURL, titleNull) |
| 438 | } | 443 | } | ||
| 439 | } | 444 | } | ||
| 440 | 445 | ||||
M internal/services/favicon_service.go +88 -26
| @@ -13,6 +13,7 @@ import ( | |||
| 13 | 13 | "time" | |
| 14 | 14 | ||
| 15 | 15 | "github.com/bnema/dumber/internal/db" | |
| 16 | + | "github.com/bnema/dumber/internal/parser" | |
| 16 | 17 | "github.com/bnema/dumber/pkg/webkit" | |
| 17 | 18 | "github.com/diamondburned/gotk4/pkg/gdk/v4" | |
| 18 | 19 | "github.com/diamondburned/gotk4/pkg/gio/v2" | |
| @@ -98,6 +99,11 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||
| 98 | 99 | return nil | |
| 99 | 100 | } | |
| 100 | 101 | ||
| 102 | + | canonicalURL := parser.NormalizeHistoryURL(pageURL) | |
| 103 | + | if canonicalURL == "" { | |
| 104 | + | canonicalURL = pageURL | |
| 105 | + | } | |
| 106 | + | ||
| 101 | 107 | // FIRST: Check if WebKit's FaviconDatabase already has this exact URI | |
| 102 | 108 | // WebKit's database persists across sessions and handles caching internally | |
| 103 | 109 | // If it's already stored, this is just a notification on refresh/reload - skip it | |
| @@ -116,7 +122,7 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||
| 116 | 122 | timestamp time.Time | |
| 117 | 123 | } | |
| 118 | 124 | ||
| 119 | - | if cached, ok := fs.recentChanges.Load(pageURL); ok { | |
| 125 | + | if cached, ok := fs.recentChanges.Load(canonicalURL); ok { | |
| 120 | 126 | if rc, ok := cached.(recentChange); ok { | |
| 121 | 127 | if time.Since(rc.timestamp) < DeduplicationWindow { | |
| 122 | 128 | // Already processed a favicon for this URL recently | |
| @@ -133,25 +139,25 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||
| 133 | 139 | ||
| 134 | 140 | // Update our history database with the favicon URI | |
| 135 | 141 | nullString := sql.NullString{String: faviconURI, Valid: faviconURI != ""} | |
| 136 | - | if err := fs.dbQueries.UpdateHistoryFavicon(ctx, nullString, pageURL); err != nil { | |
| 142 | + | if err := fs.dbQueries.UpdateHistoryFavicon(ctx, nullString, canonicalURL); err != nil { | |
| 137 | 143 | log.Printf("[favicon] Failed to update favicon URI in database for %s: %v", pageURL, err) | |
| 138 | 144 | return fmt.Errorf("failed to update favicon in database: %w", err) | |
| 139 | 145 | } | |
| 140 | 146 | ||
| 141 | 147 | // Store this change as recently processed | |
| 142 | - | fs.recentChanges.Store(pageURL, recentChange{faviconURI: faviconURI, timestamp: time.Now()}) | |
| 148 | + | fs.recentChanges.Store(canonicalURL, recentChange{faviconURI: faviconURI, timestamp: time.Now()}) | |
| 143 | 149 | ||
| 144 | 150 | // Propagate favicon to domain asynchronously to avoid blocking WebKit | |
| 145 | - | go fs.propagateFaviconToDomain(pageURL, faviconURI) | |
| 151 | + | go fs.propagateFaviconToDomain(canonicalURL, faviconURI) | |
| 146 | 152 | ||
| 147 | 153 | // Proactively render and export favicon for omnibox/CLI use (debounced) | |
| 148 | 154 | if fs.enableExport { | |
| 149 | 155 | // Check if a preload is already in progress for this URL | |
| 150 | - | if _, loaded := fs.ongoingPreloads.LoadOrStore(pageURL, true); !loaded { | |
| 156 | + | if _, loaded := fs.ongoingPreloads.LoadOrStore(canonicalURL, true); !loaded { | |
| 151 | 157 | // No preload in progress, start one | |
| 152 | 158 | go func() { | |
| 153 | - | defer fs.ongoingPreloads.Delete(pageURL) | |
| 154 | - | fs.preloadFavicon(pageURL, faviconURI) | |
| 159 | + | defer fs.ongoingPreloads.Delete(canonicalURL) | |
| 160 | + | fs.preloadFavicon(pageURL, canonicalURL, faviconURI) | |
| 155 | 161 | }() | |
| 156 | 162 | } | |
| 157 | 163 | } | |
| @@ -210,7 +216,7 @@ func (fs *FaviconService) propagateFaviconToDomain(pageURL, faviconURI string) { | |||
| 210 | 216 | // preloadFavicon renders and exports a favicon in the background. | |
| 211 | 217 | // This is called immediately when OnFaviconChanged fires to ensure | |
| 212 | 218 | // favicons are ready for omnibox suggestions as soon as possible. | |
| 213 | - | func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | |
| 219 | + | func (fs *FaviconService) preloadFavicon(pageURL, canonicalURL, faviconURI string) { | |
| 214 | 220 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | |
| 215 | 221 | defer cancel() | |
| 216 | 222 | ||
| @@ -220,7 +226,7 @@ func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | |||
| 220 | 226 | return | |
| 221 | 227 | } | |
| 222 | 228 | ||
| 223 | - | fs.maybeExportTexture(pageURL, texture) | |
| 229 | + | fs.maybeExportTexture(canonicalURL, texture) | |
| 224 | 230 | log.Printf("[favicon] Preloaded and exported favicon for %s", pageURL) | |
| 225 | 231 | } | |
| 226 | 232 | ||
| @@ -239,7 +245,12 @@ func (fs *FaviconService) GetFaviconTexture(pageURL string, callback func(*gdk.T | |||
| 239 | 245 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | |
| 240 | 246 | defer cancel() | |
| 241 | 247 | ||
| 242 | - | faviconURI, err := fs.lookupFaviconURI(ctx, pageURL) | |
| 248 | + | canonicalURL := parser.NormalizeHistoryURL(pageURL) | |
| 249 | + | if canonicalURL == "" { | |
| 250 | + | canonicalURL = pageURL | |
| 251 | + | } | |
| 252 | + | ||
| 253 | + | faviconURI, err := fs.lookupFaviconURI(ctx, canonicalURL, pageURL) | |
| 243 | 254 | if err != nil { | |
| 244 | 255 | fs.dispatchTexture(callback, nil, err) | |
| 245 | 256 | return | |
| @@ -251,7 +262,7 @@ func (fs *FaviconService) GetFaviconTexture(pageURL string, callback func(*gdk.T | |||
| 251 | 262 | return | |
| 252 | 263 | } | |
| 253 | 264 | ||
| 254 | - | fs.maybeExportTexture(pageURL, texture) | |
| 265 | + | fs.maybeExportTexture(canonicalURL, texture) | |
| 255 | 266 | ||
| 256 | 267 | fs.dispatchTexture(callback, texture, nil) | |
| 257 | 268 | }() | |
| @@ -335,23 +346,47 @@ func (fs *FaviconService) loadSVGFaviconTexture(ctx context.Context, faviconURI | |||
| 335 | 346 | return texture, nil | |
| 336 | 347 | } | |
| 337 | 348 | ||
| 338 | - | func (fs *FaviconService) lookupFaviconURI(ctx context.Context, pageURL string) (string, error) { | |
| 339 | - | entry, err := fs.dbQueries.GetHistoryEntry(ctx, pageURL) | |
| 340 | - | if err == nil { | |
| 341 | - | if entry.FaviconUrl.Valid && entry.FaviconUrl.String != "" { | |
| 342 | - | return entry.FaviconUrl.String, nil | |
| 349 | + | func (fs *FaviconService) lookupFaviconURI(ctx context.Context, canonicalURL, originalURL string) (string, error) { | |
| 350 | + | type void struct{} | |
| 351 | + | seen := make(map[string]void, 2) | |
| 352 | + | candidates := make([]string, 0, 2) | |
| 353 | + | ||
| 354 | + | addCandidate := func(val string) { | |
| 355 | + | if val == "" { | |
| 356 | + | return | |
| 357 | + | } | |
| 358 | + | if _, exists := seen[val]; exists { | |
| 359 | + | return | |
| 360 | + | } | |
| 361 | + | seen[val] = void{} | |
| 362 | + | candidates = append(candidates, val) | |
| 363 | + | } | |
| 364 | + | ||
| 365 | + | addCandidate(canonicalURL) | |
| 366 | + | addCandidate(originalURL) | |
| 367 | + | ||
| 368 | + | for _, candidate := range candidates { | |
| 369 | + | entry, err := fs.dbQueries.GetHistoryEntry(ctx, candidate) | |
| 370 | + | if err == nil { | |
| 371 | + | if entry.FaviconUrl.Valid && entry.FaviconUrl.String != "" { | |
| 372 | + | return entry.FaviconUrl.String, nil | |
| 373 | + | } | |
| 374 | + | continue | |
| 375 | + | } | |
| 376 | + | if !errors.Is(err, sql.ErrNoRows) { | |
| 377 | + | log.Printf("[favicon] failed to query history entry for %s: %v", candidate, err) | |
| 343 | 378 | } | |
| 344 | - | } else if !errors.Is(err, sql.ErrNoRows) { | |
| 345 | - | log.Printf("[favicon] failed to query history entry for %s: %v", pageURL, err) | |
| 346 | 379 | } | |
| 347 | 380 | ||
| 348 | 381 | if fs.faviconDB != nil { | |
| 349 | - | if uri := fs.faviconDB.FaviconURI(pageURL); uri != "" { | |
| 350 | - | return uri, nil | |
| 382 | + | for _, candidate := range candidates { | |
| 383 | + | if uri := fs.faviconDB.FaviconURI(candidate); uri != "" { | |
| 384 | + | return uri, nil | |
| 385 | + | } | |
| 351 | 386 | } | |
| 352 | 387 | } | |
| 353 | 388 | ||
| 354 | - | return "", fmt.Errorf("no favicon URI available for %s", pageURL) | |
| 389 | + | return "", fmt.Errorf("no favicon URI available for %s", canonicalURL) | |
| 355 | 390 | } | |
| 356 | 391 | ||
| 357 | 392 | func (fs *FaviconService) ensureTextureSize(texture *gdk.Texture) *gdk.Texture { | |
| @@ -391,7 +426,12 @@ func (fs *FaviconService) maybeExportTexture(pageURL string, texture *gdk.Textur | |||
| 391 | 426 | // GetFaviconPath returns the exported PNG path for CLI tools (dmenu, etc.) | |
| 392 | 427 | // This allows CLI tools to display favicons without WebKitGTK API access. | |
| 393 | 428 | func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | |
| 394 | - | exportPath := fs.getExportPath(pageURL) | |
| 429 | + | canonicalURL := parser.NormalizeHistoryURL(pageURL) | |
| 430 | + | if canonicalURL == "" { | |
| 431 | + | canonicalURL = pageURL | |
| 432 | + | } | |
| 433 | + | ||
| 434 | + | exportPath := fs.getExportPath(canonicalURL) | |
| 395 | 435 | if info, err := os.Stat(exportPath); err == nil { | |
| 396 | 436 | if time.Since(info.ModTime()) < 7*24*time.Hour { | |
| 397 | 437 | return exportPath, nil | |
| @@ -405,7 +445,7 @@ func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | |||
| 405 | 445 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | |
| 406 | 446 | defer cancel() | |
| 407 | 447 | ||
| 408 | - | faviconURI, err := fs.lookupFaviconURI(ctx, pageURL) | |
| 448 | + | faviconURI, err := fs.lookupFaviconURI(ctx, canonicalURL, pageURL) | |
| 409 | 449 | if err != nil { | |
| 410 | 450 | return "", err | |
| 411 | 451 | } | |
| @@ -424,8 +464,8 @@ func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | |||
| 424 | 464 | ||
| 425 | 465 | // getExportPath generates the export path for a page URL | |
| 426 | 466 | func (fs *FaviconService) getExportPath(pageURL string) string { | |
| 427 | - | // Use the same hashing scheme as the old cache for compatibility | |
| 428 | - | hash := fmt.Sprintf("%x", webkit.HashURL(pageURL)) | |
| 467 | + | exportKey := canonicalExportURL(pageURL) | |
| 468 | + | hash := fmt.Sprintf("%x", webkit.HashURL(exportKey)) | |
| 429 | 469 | return filepath.Join(fs.exportDir, hash+".png") | |
| 430 | 470 | } | |
| 431 | 471 | ||
| @@ -501,6 +541,28 @@ func (fs *FaviconService) CleanOldExports() error { | |||
| 501 | 541 | // GetExportedFaviconPath returns the expected export path for a page URL. | |
| 502 | 542 | func GetExportedFaviconPath(dataDir, pageURL string) string { | |
| 503 | 543 | exportDir := filepath.Join(dataDir, "webkit", "favicons-export") | |
| 504 | - | hash := fmt.Sprintf("%x", webkit.HashURL(pageURL)) | |
| 544 | + | exportKey := canonicalExportURL(pageURL) | |
| 545 | + | hash := fmt.Sprintf("%x", webkit.HashURL(exportKey)) | |
| 505 | 546 | return filepath.Join(exportDir, hash+".png") | |
| 506 | 547 | } | |
| 548 | + | ||
| 549 | + | func canonicalExportURL(pageURL string) string { | |
| 550 | + | parsed, err := webkit.ParseURL(pageURL) | |
| 551 | + | if err != nil || parsed.Scheme == "" || parsed.Host == "" { | |
| 552 | + | normalized := parser.NormalizeHistoryURL(pageURL) | |
| 553 | + | if normalized == "" { | |
| 554 | + | return pageURL | |
| 555 | + | } | |
| 556 | + | parsed, err = webkit.ParseURL(normalized) | |
| 557 | + | if err != nil || parsed.Scheme == "" || parsed.Host == "" { | |
| 558 | + | return normalized | |
| 559 | + | } | |
| 560 | + | } | |
| 561 | + | ||
| 562 | + | host := parsed.Host | |
| 563 | + | if host == "" { | |
| 564 | + | return fmt.Sprintf("%s://%s/", parsed.Scheme, parsed.Host) | |
| 565 | + | } | |
| 566 | + | ||
| 567 | + | return fmt.Sprintf("%s://%s/", parsed.Scheme, host) | |
| 568 | + | } | |
| @@ -13,6 +13,7 @@ import ( | |||||
| 13 | "time" | 13 | "time" | ||
| 14 | 14 | ||||
| 15 | "github.com/bnema/dumber/internal/db" | 15 | "github.com/bnema/dumber/internal/db" | ||
| 16 | + | "github.com/bnema/dumber/internal/parser" | |||
| 16 | "github.com/bnema/dumber/pkg/webkit" | 17 | "github.com/bnema/dumber/pkg/webkit" | ||
| 17 | "github.com/diamondburned/gotk4/pkg/gdk/v4" | 18 | "github.com/diamondburned/gotk4/pkg/gdk/v4" | ||
| 18 | "github.com/diamondburned/gotk4/pkg/gio/v2" | 19 | "github.com/diamondburned/gotk4/pkg/gio/v2" | ||
| @@ -98,6 +99,11 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||||
| 98 | return nil | 99 | return nil | ||
| 99 | } | 100 | } | ||
| 100 | 101 | ||||
| 102 | + | canonicalURL := parser.NormalizeHistoryURL(pageURL) | |||
| 103 | + | if canonicalURL == "" { | |||
| 104 | + | canonicalURL = pageURL | |||
| 105 | + | } | |||
| 106 | + | ||||
| 101 | // FIRST: Check if WebKit's FaviconDatabase already has this exact URI | 107 | // FIRST: Check if WebKit's FaviconDatabase already has this exact URI | ||
| 102 | // WebKit's database persists across sessions and handles caching internally | 108 | // WebKit's database persists across sessions and handles caching internally | ||
| 103 | // If it's already stored, this is just a notification on refresh/reload - skip it | 109 | // If it's already stored, this is just a notification on refresh/reload - skip it | ||
| @@ -116,7 +122,7 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||||
| 116 | timestamp time.Time | 122 | timestamp time.Time | ||
| 117 | } | 123 | } | ||
| 118 | 124 | ||||
| 119 | - | if cached, ok := fs.recentChanges.Load(pageURL); ok { | 125 | + | if cached, ok := fs.recentChanges.Load(canonicalURL); ok { |
| 120 | if rc, ok := cached.(recentChange); ok { | 126 | if rc, ok := cached.(recentChange); ok { | ||
| 121 | if time.Since(rc.timestamp) < DeduplicationWindow { | 127 | if time.Since(rc.timestamp) < DeduplicationWindow { | ||
| 122 | // Already processed a favicon for this URL recently | 128 | // Already processed a favicon for this URL recently | ||
| @@ -133,25 +139,25 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||||
| 133 | 139 | ||||
| 134 | // Update our history database with the favicon URI | 140 | // Update our history database with the favicon URI | ||
| 135 | nullString := sql.NullString{String: faviconURI, Valid: faviconURI != ""} | 141 | nullString := sql.NullString{String: faviconURI, Valid: faviconURI != ""} | ||
| 136 | - | if err := fs.dbQueries.UpdateHistoryFavicon(ctx, nullString, pageURL); err != nil { | 142 | + | if err := fs.dbQueries.UpdateHistoryFavicon(ctx, nullString, canonicalURL); err != nil { |
| 137 | log.Printf("[favicon] Failed to update favicon URI in database for %s: %v", pageURL, err) | 143 | log.Printf("[favicon] Failed to update favicon URI in database for %s: %v", pageURL, err) | ||
| 138 | return fmt.Errorf("failed to update favicon in database: %w", err) | 144 | return fmt.Errorf("failed to update favicon in database: %w", err) | ||
| 139 | } | 145 | } | ||
| 140 | 146 | ||||
| 141 | // Store this change as recently processed | 147 | // Store this change as recently processed | ||
| 142 | - | fs.recentChanges.Store(pageURL, recentChange{faviconURI: faviconURI, timestamp: time.Now()}) | 148 | + | fs.recentChanges.Store(canonicalURL, recentChange{faviconURI: faviconURI, timestamp: time.Now()}) |
| 143 | 149 | ||||
| 144 | // Propagate favicon to domain asynchronously to avoid blocking WebKit | 150 | // Propagate favicon to domain asynchronously to avoid blocking WebKit | ||
| 145 | - | go fs.propagateFaviconToDomain(pageURL, faviconURI) | 151 | + | go fs.propagateFaviconToDomain(canonicalURL, faviconURI) |
| 146 | 152 | ||||
| 147 | // Proactively render and export favicon for omnibox/CLI use (debounced) | 153 | // Proactively render and export favicon for omnibox/CLI use (debounced) | ||
| 148 | if fs.enableExport { | 154 | if fs.enableExport { | ||
| 149 | // Check if a preload is already in progress for this URL | 155 | // Check if a preload is already in progress for this URL | ||
| 150 | - | if _, loaded := fs.ongoingPreloads.LoadOrStore(pageURL, true); !loaded { | 156 | + | if _, loaded := fs.ongoingPreloads.LoadOrStore(canonicalURL, true); !loaded { |
| 151 | // No preload in progress, start one | 157 | // No preload in progress, start one | ||
| 152 | go func() { | 158 | go func() { | ||
| 153 | - | defer fs.ongoingPreloads.Delete(pageURL) | 159 | + | defer fs.ongoingPreloads.Delete(canonicalURL) |
| 154 | - | fs.preloadFavicon(pageURL, faviconURI) | 160 | + | fs.preloadFavicon(pageURL, canonicalURL, faviconURI) |
| 155 | }() | 161 | }() | ||
| 156 | } | 162 | } | ||
| 157 | } | 163 | } | ||
| @@ -210,7 +216,7 @@ func (fs *FaviconService) propagateFaviconToDomain(pageURL, faviconURI string) { | |||||
| 210 | // preloadFavicon renders and exports a favicon in the background. | 216 | // preloadFavicon renders and exports a favicon in the background. | ||
| 211 | // This is called immediately when OnFaviconChanged fires to ensure | 217 | // This is called immediately when OnFaviconChanged fires to ensure | ||
| 212 | // favicons are ready for omnibox suggestions as soon as possible. | 218 | // favicons are ready for omnibox suggestions as soon as possible. | ||
| 213 | - | func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | 219 | + | func (fs *FaviconService) preloadFavicon(pageURL, canonicalURL, faviconURI string) { |
| 214 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | 220 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | ||
| 215 | defer cancel() | 221 | defer cancel() | ||
| 216 | 222 | ||||
| @@ -220,7 +226,7 @@ func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | |||||
| 220 | return | 226 | return | ||
| 221 | } | 227 | } | ||
| 222 | 228 | ||||
| 223 | - | fs.maybeExportTexture(pageURL, texture) | 229 | + | fs.maybeExportTexture(canonicalURL, texture) |
| 224 | log.Printf("[favicon] Preloaded and exported favicon for %s", pageURL) | 230 | log.Printf("[favicon] Preloaded and exported favicon for %s", pageURL) | ||
| 225 | } | 231 | } | ||
| 226 | 232 | ||||
| @@ -239,7 +245,12 @@ func (fs *FaviconService) GetFaviconTexture(pageURL string, callback func(*gdk.T | |||||
| 239 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | 245 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | ||
| 240 | defer cancel() | 246 | defer cancel() | ||
| 241 | 247 | ||||
| 242 | - | faviconURI, err := fs.lookupFaviconURI(ctx, pageURL) | 248 | + | canonicalURL := parser.NormalizeHistoryURL(pageURL) |
| 249 | + | if canonicalURL == "" { | |||
| 250 | + | canonicalURL = pageURL | |||
| 251 | + | } | |||
| 252 | + | ||||
| 253 | + | faviconURI, err := fs.lookupFaviconURI(ctx, canonicalURL, pageURL) | |||
| 243 | if err != nil { | 254 | if err != nil { | ||
| 244 | fs.dispatchTexture(callback, nil, err) | 255 | fs.dispatchTexture(callback, nil, err) | ||
| 245 | return | 256 | return | ||
| @@ -251,7 +262,7 @@ func (fs *FaviconService) GetFaviconTexture(pageURL string, callback func(*gdk.T | |||||
| 251 | return | 262 | return | ||
| 252 | } | 263 | } | ||
| 253 | 264 | ||||
| 254 | - | fs.maybeExportTexture(pageURL, texture) | 265 | + | fs.maybeExportTexture(canonicalURL, texture) |
| 255 | 266 | ||||
| 256 | fs.dispatchTexture(callback, texture, nil) | 267 | fs.dispatchTexture(callback, texture, nil) | ||
| 257 | }() | 268 | }() | ||
| @@ -335,23 +346,47 @@ func (fs *FaviconService) loadSVGFaviconTexture(ctx context.Context, faviconURI | |||||
| 335 | return texture, nil | 346 | return texture, nil | ||
| 336 | } | 347 | } | ||
| 337 | 348 | ||||
| 338 | - | func (fs *FaviconService) lookupFaviconURI(ctx context.Context, pageURL string) (string, error) { | 349 | + | func (fs *FaviconService) lookupFaviconURI(ctx context.Context, canonicalURL, originalURL string) (string, error) { |
| 339 | - | entry, err := fs.dbQueries.GetHistoryEntry(ctx, pageURL) | 350 | + | type void struct{} |
| 340 | - | if err == nil { | 351 | + | seen := make(map[string]void, 2) |
| 341 | - | if entry.FaviconUrl.Valid && entry.FaviconUrl.String != "" { | 352 | + | candidates := make([]string, 0, 2) |
| 342 | - | return entry.FaviconUrl.String, nil | 353 | + | |
| 354 | + | addCandidate := func(val string) { | |||
| 355 | + | if val == "" { | |||
| 356 | + | return | |||
| 357 | + | } | |||
| 358 | + | if _, exists := seen[val]; exists { | |||
| 359 | + | return | |||
| 360 | + | } | |||
| 361 | + | seen[val] = void{} | |||
| 362 | + | candidates = append(candidates, val) | |||
| 363 | + | } | |||
| 364 | + | ||||
| 365 | + | addCandidate(canonicalURL) | |||
| 366 | + | addCandidate(originalURL) | |||
| 367 | + | ||||
| 368 | + | for _, candidate := range candidates { | |||
| 369 | + | entry, err := fs.dbQueries.GetHistoryEntry(ctx, candidate) | |||
| 370 | + | if err == nil { | |||
| 371 | + | if entry.FaviconUrl.Valid && entry.FaviconUrl.String != "" { | |||
| 372 | + | return entry.FaviconUrl.String, nil | |||
| 373 | + | } | |||
| 374 | + | continue | |||
| 375 | + | } | |||
| 376 | + | if !errors.Is(err, sql.ErrNoRows) { | |||
| 377 | + | log.Printf("[favicon] failed to query history entry for %s: %v", candidate, err) | |||
| 343 | } | 378 | } | ||
| 344 | - | } else if !errors.Is(err, sql.ErrNoRows) { | |||
| 345 | - | log.Printf("[favicon] failed to query history entry for %s: %v", pageURL, err) | |||
| 346 | } | 379 | } | ||
| 347 | 380 | ||||
| 348 | if fs.faviconDB != nil { | 381 | if fs.faviconDB != nil { | ||
| 349 | - | if uri := fs.faviconDB.FaviconURI(pageURL); uri != "" { | 382 | + | for _, candidate := range candidates { |
| 350 | - | return uri, nil | 383 | + | if uri := fs.faviconDB.FaviconURI(candidate); uri != "" { |
| 384 | + | return uri, nil | |||
| 385 | + | } | |||
| 351 | } | 386 | } | ||
| 352 | } | 387 | } | ||
| 353 | 388 | ||||
| 354 | - | return "", fmt.Errorf("no favicon URI available for %s", pageURL) | 389 | + | return "", fmt.Errorf("no favicon URI available for %s", canonicalURL) |
| 355 | } | 390 | } | ||
| 356 | 391 | ||||
| 357 | func (fs *FaviconService) ensureTextureSize(texture *gdk.Texture) *gdk.Texture { | 392 | func (fs *FaviconService) ensureTextureSize(texture *gdk.Texture) *gdk.Texture { | ||
| @@ -391,7 +426,12 @@ func (fs *FaviconService) maybeExportTexture(pageURL string, texture *gdk.Textur | |||||
| 391 | // GetFaviconPath returns the exported PNG path for CLI tools (dmenu, etc.) | 426 | // GetFaviconPath returns the exported PNG path for CLI tools (dmenu, etc.) | ||
| 392 | // This allows CLI tools to display favicons without WebKitGTK API access. | 427 | // This allows CLI tools to display favicons without WebKitGTK API access. | ||
| 393 | func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | 428 | func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | ||
| 394 | - | exportPath := fs.getExportPath(pageURL) | 429 | + | canonicalURL := parser.NormalizeHistoryURL(pageURL) |
| 430 | + | if canonicalURL == "" { | |||
| 431 | + | canonicalURL = pageURL | |||
| 432 | + | } | |||
| 433 | + | ||||
| 434 | + | exportPath := fs.getExportPath(canonicalURL) | |||
| 395 | if info, err := os.Stat(exportPath); err == nil { | 435 | if info, err := os.Stat(exportPath); err == nil { | ||
| 396 | if time.Since(info.ModTime()) < 7*24*time.Hour { | 436 | if time.Since(info.ModTime()) < 7*24*time.Hour { | ||
| 397 | return exportPath, nil | 437 | return exportPath, nil | ||
| @@ -405,7 +445,7 @@ func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | |||||
| 405 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | 445 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | ||
| 406 | defer cancel() | 446 | defer cancel() | ||
| 407 | 447 | ||||
| 408 | - | faviconURI, err := fs.lookupFaviconURI(ctx, pageURL) | 448 | + | faviconURI, err := fs.lookupFaviconURI(ctx, canonicalURL, pageURL) |
| 409 | if err != nil { | 449 | if err != nil { | ||
| 410 | return "", err | 450 | return "", err | ||
| 411 | } | 451 | } | ||
| @@ -424,8 +464,8 @@ func (fs *FaviconService) GetFaviconPath(pageURL string) (string, error) { | |||||
| 424 | 464 | ||||
| 425 | // getExportPath generates the export path for a page URL | 465 | // getExportPath generates the export path for a page URL | ||
| 426 | func (fs *FaviconService) getExportPath(pageURL string) string { | 466 | func (fs *FaviconService) getExportPath(pageURL string) string { | ||
| 427 | - | // Use the same hashing scheme as the old cache for compatibility | 467 | + | exportKey := canonicalExportURL(pageURL) |
| 428 | - | hash := fmt.Sprintf("%x", webkit.HashURL(pageURL)) | 468 | + | hash := fmt.Sprintf("%x", webkit.HashURL(exportKey)) |
| 429 | return filepath.Join(fs.exportDir, hash+".png") | 469 | return filepath.Join(fs.exportDir, hash+".png") | ||
| 430 | } | 470 | } | ||
| 431 | 471 | ||||
| @@ -501,6 +541,28 @@ func (fs *FaviconService) CleanOldExports() error { | |||||
| 501 | // GetExportedFaviconPath returns the expected export path for a page URL. | 541 | // GetExportedFaviconPath returns the expected export path for a page URL. | ||
| 502 | func GetExportedFaviconPath(dataDir, pageURL string) string { | 542 | func GetExportedFaviconPath(dataDir, pageURL string) string { | ||
| 503 | exportDir := filepath.Join(dataDir, "webkit", "favicons-export") | 543 | exportDir := filepath.Join(dataDir, "webkit", "favicons-export") | ||
| 504 | - | hash := fmt.Sprintf("%x", webkit.HashURL(pageURL)) | 544 | + | exportKey := canonicalExportURL(pageURL) |
| 545 | + | hash := fmt.Sprintf("%x", webkit.HashURL(exportKey)) | |||
| 505 | return filepath.Join(exportDir, hash+".png") | 546 | return filepath.Join(exportDir, hash+".png") | ||
| 506 | } | 547 | } | ||
| 548 | + | ||||
| 549 | + | func canonicalExportURL(pageURL string) string { | |||
| 550 | + | parsed, err := webkit.ParseURL(pageURL) | |||
| 551 | + | if err != nil || parsed.Scheme == "" || parsed.Host == "" { | |||
| 552 | + | normalized := parser.NormalizeHistoryURL(pageURL) | |||
| 553 | + | if normalized == "" { | |||
| 554 | + | return pageURL | |||
| 555 | + | } | |||
| 556 | + | parsed, err = webkit.ParseURL(normalized) | |||
| 557 | + | if err != nil || parsed.Scheme == "" || parsed.Host == "" { | |||
| 558 | + | return normalized | |||
| 559 | + | } | |||
| 560 | + | } | |||
| 561 | + | ||||
| 562 | + | host := parsed.Host | |||
| 563 | + | if host == "" { | |||
| 564 | + | return fmt.Sprintf("%s://%s/", parsed.Scheme, parsed.Host) | |||
| 565 | + | } | |||
| 566 | + | ||||
| 567 | + | return fmt.Sprintf("%s://%s/", parsed.Scheme, host) | |||
| 568 | + | } | |||