Repository navigation
Conversation
dave-atx
force-pushed
the
favicon-home-page-fallback
branch
from
September 9, 2026 01:32
30ac118 to
e332dcd
Compare
dave-atx
force-pushed
the
favicon-home-page-fallback
branch
from
October 12, 2026 15:09
e332dcd to
0198d56
Compare
This was referenced Oct 24, 2026
dave-atx
force-pushed
the
favicon-home-page-fallback
branch
4 times, most recently
from
November 8, 2026 16:48
e7313fb to
f9bebe0
Compare
findFaviconURLs returns nil whenever cachedMetadata does, and searchForFavicon then waits for .htmlMetadataAvailable. When the home page fetch fails and there's no stale record, that notification never comes, so the feed gets no favicon at all -- for as long as the failure record lasts, 11 days after a 4xx. But the last candidate, scheme://host/favicon.ico, needs no HTML to find. cachedMetadata returns nil in two different situations: the download is still in flight, or it isn't coming. Only the first is worth waiting for. HTMLMetadataDownloader now tracks the second and answers metadataIsUnavailable, posting htmlMetadataAvailable so existing observers re-query. A search started that way tries only favicon.ico. checkForNewFaviconURL doesn't use the guess, since it would otherwise replace a saved, declared favicon with favicon.ico. Sync accounts feel this most, since Miniflux, Feedly, ReaderAPI and local accounts never set faviconURL -- it's JSON-Feed-only -- so the home page is their only route to a favicon. Written with help from Claude Code.
dave-atx
force-pushed
the
favicon-home-page-fallback
branch
from
November 8, 2026 23:21
f9bebe0 to
531d3af
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #5421
When a home page can't be read,
FaviconDownloadernever tries the one favicon candidate that doesn't need its HTML.findFaviconURLsreturns nil whenevercachedMetadatadoes, andsearchForFaviconthen waits for.htmlMetadataAvailable. If the fetch failed and there's no stale record, that notification never comes, so the feed has no favicon for as long as the failure record lasts (11 days after a 4xx).scheme://host/favicon.iconeeds no HTML.cachedMetadatareturns nil in two situations: the download is still in flight, or it isn't coming. Only the first is worth waiting for.HTMLMetadataDownloadernow tracks the second and answersmetadataIsUnavailable(for:), posting.htmlMetadataAvailableso existing observers re-query. A search started that way tries onlyfavicon.ico.checkForNewFaviconURLdoesn't use the guess, since it would otherwise replace a saved, declared favicon withfavicon.ico.Sync accounts feel this most. Miniflux, Feedly, ReaderAPI and local accounts never set
faviconURL(it's JSON-Feed-only), so the home page is their only route to a favicon.I rebased this onto the new icon code, which made it a lot smaller. The per-home-page
FaviconSearchalready fixes the sharedcurrentHomePageHasOnlyFaviconICOflag the earlier version also fixed. ThefaviconCandidatesfunction and the test target are gone along with it.