mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-07-22 07:48:27 +00:00
A pinned web app showed a generic placeholder instead of its icon —
ditto.pub failed while brainstorm.nosfabrica.com worked. The
discriminator is what each site declares:
brainstorm: favicon.svg AND favicon.png <- the PNG is why it works
ditto: logo.svg only (+ an apple-touch-icon nobody read)
Icons were captured solely through `WebChromeClient.onReceivedIcon`,
which hands back a rasterized Bitmap. Android WebView does not decode SVG
favicons into that callback, so a site offering a raster alternative gets
picked up and an SVG-only site never fires it at all: nothing is
recorded, `iconModelFor()` returns null forever, and it fails silently —
no error, no log line, just a permanently generic icon. SVG-only
declarations are increasingly the norm, so this was set to affect more
apps over time.
Adds a sniffer that, after a main-frame load, ranks the page's declared
icons (raster `rel="icon"` → SVG `rel="icon"` → apple-touch-icon →
/favicon.ico) and fetches the best one IN PAGE CONTEXT, falling through
on failure. The bytes are size-bounded and magic-byte validated before
being relayed down the existing record path; anything unrecognised is
dropped.
Fetching in page context is the point, not an implementation detail. The
registry captures from the WebView deliberately — "an alternative to the
main app fetching host/favicon.ico itself, which would bypass Tor and
leak the request". Ditto's /favicon.ico exists and returns a valid icon,
so fetching it from the app would have been the easy fix and the wrong
one. Every byte still comes through the sandbox WebView's own network
path.
No rasterisation needed: coil3's SVG decoder is registered on the
singleton loader and sniffs by content rather than extension, so stored
SVG bytes decode even under the registry's .png filename. Only a leading
BOM/whitespace trim was required so byte 0 is the '<' the sniffer wants.
Also fixes a second gap found while diagnosing: `NappletBrowserService` —
the EMBEDDED path backing pinned bottom-nav tabs — had no icon capture at
all, so a pinned app's icon depended on having once opened it full
screen. It now gets both the missing raster callback and the sniff.
`NappletHostService` deliberately left alone: it serves verified blobs
over a synthetic internal host, and applet icons already come from the
manifest, so capture there would be dead code.
The sniff runs after a delay and skips when the raster callback already
claimed the host, so sites that worked before are untouched.
Verified on device: ditto.pub now shows its real icon on the favorite
card, the pinned sidebar tab and the recents row, via both the embedded
and full-screen paths; Brainstorm is unchanged; example.com (no usable
icon) degrades to the placeholder with zero record calls and no hang; no
crashes.
Pre-existing, not fixed: `BrowserIconRegistry.record` writes the file on
the IPC handler thread, tripping StrictMode. It did so before this change
— which now simply gives it more occasions. Moving that write off-main is
a cheap follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>