fix(favicon): correct function comments in FaviconService
- Move preloadFavicon comment to correct function (was on propagateFaviconToDomain) - Add proper comment for propagateFaviconToDomain describing domain propagation - Fixes Copilot review feedback
- commit
- a1460e4f9bc0a6dec414eb740ae007f17273cd71
- author
- bnema · · 2025-10-18 10:59 UTC
- parent
- 6d78491
- signature
- unsigned
1 file · +3 -3
M internal/services/favicon_service.go +3 -3
| @@ -109,9 +109,6 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||
| 109 | 109 | return nil | |
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | - | // preloadFavicon renders and exports a favicon in the background | |
| 113 | - | // This is called immediately when OnFaviconChanged fires to ensure | |
| 114 | - | // favicons are ready for omnibox suggestions as soon as possible | |
| 115 | 112 | // propagateFaviconToDomain updates all history entries from the same domain | |
| 116 | 113 | // with the favicon. This ensures multiple paths on the same domain share the favicon | |
| 117 | 114 | // even if WebKit only signals it once per domain (KISS + DRY principle). | |
| @@ -160,6 +157,9 @@ func (fs *FaviconService) propagateFaviconToDomain(pageURL, faviconURI string) { | |||
| 160 | 157 | } | |
| 161 | 158 | } | |
| 162 | 159 | ||
| 160 | + | // preloadFavicon renders and exports a favicon in the background. | |
| 161 | + | // This is called immediately when OnFaviconChanged fires to ensure | |
| 162 | + | // favicons are ready for omnibox suggestions as soon as possible. | |
| 163 | 163 | func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | |
| 164 | 164 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | |
| 165 | 165 | defer cancel() | |
| @@ -109,9 +109,6 @@ func (fs *FaviconService) OnFaviconChanged(pageURL, faviconURI string) error { | |||||
| 109 | return nil | 109 | return nil | ||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | - | // preloadFavicon renders and exports a favicon in the background | |||
| 113 | - | // This is called immediately when OnFaviconChanged fires to ensure | |||
| 114 | - | // favicons are ready for omnibox suggestions as soon as possible | |||
| 115 | // propagateFaviconToDomain updates all history entries from the same domain | 112 | // propagateFaviconToDomain updates all history entries from the same domain | ||
| 116 | // with the favicon. This ensures multiple paths on the same domain share the favicon | 113 | // with the favicon. This ensures multiple paths on the same domain share the favicon | ||
| 117 | // even if WebKit only signals it once per domain (KISS + DRY principle). | 114 | // even if WebKit only signals it once per domain (KISS + DRY principle). | ||
| @@ -160,6 +157,9 @@ func (fs *FaviconService) propagateFaviconToDomain(pageURL, faviconURI string) { | |||||
| 160 | } | 157 | } | ||
| 161 | } | 158 | } | ||
| 162 | 159 | ||||
| 160 | + | // preloadFavicon renders and exports a favicon in the background. | |||
| 161 | + | // This is called immediately when OnFaviconChanged fires to ensure | |||
| 162 | + | // favicons are ready for omnibox suggestions as soon as possible. | |||
| 163 | func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | 163 | func (fs *FaviconService) preloadFavicon(pageURL, faviconURI string) { | ||
| 164 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | 164 | ctx, cancel := context.WithTimeout(context.Background(), FaviconTimeout) | ||
| 165 | defer cancel() | 165 | defer cancel() | ||