From 5a5e841c5dd2818b712d41a8989aa9256c3800f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 2 Sep 2026 23:53:45 -0400 Subject: [PATCH] fix(cli): pick the highest-quality favicon and keep its transparent background (#3606) * fix(cli): capture the best declared favicon, not the first one Pages routinely declare a legacy 16px .ico first and the good asset (an SVG, or a 180x180 apple-touch PNG) after it. The capture's page evaluate kept only {rel, href} and the download loop took whichever candidate fetched first, so the .ico won on every such page. The dropped sizes/type attributes are the only evidence of quality: page.html on disk does not keep the tags and only the winner's bytes are fetched, so the choice was unrecoverable downstream. Keep sizes and type, and rank candidates before downloading: SVG first, then the largest declared size (an unsized apple-touch-icon counts as 180), then .ico. The loop still falls through to the next candidate when one fails to download, so the ranking changes which icon wins, never whether one lands. Ranking is a pure function over the declared attributes, unit-tested against the link shapes three sites actually publish. * fix(cli): rank a pinned-tab mask-icon below real favicons `link[rel*="icon"]` also matches `rel="mask-icon"`, Safari's pinned-tab asset: a single-colour silhouette drawn in a browser-chosen tint, not the site mark. It is served as an SVG, so ranking by format alone promoted the outline above the page's actual colour favicon. Give mask-icon its own lowest tier rather than dropping it, so a page that declares nothing else still lands an icon instead of none. --- packages/cli/src/capture/assetDownloader.ts | 7 +- .../cli/src/capture/faviconRanker.test.ts | 137 ++++++++++++++++++ packages/cli/src/capture/faviconRanker.ts | 92 ++++++++++++ packages/cli/src/capture/index.ts | 15 +- 4 files changed, 246 insertions(+), 5 deletions(-) create mode 100644 packages/cli/src/capture/faviconRanker.test.ts create mode 100644 packages/cli/src/capture/faviconRanker.ts diff --git a/packages/cli/src/capture/assetDownloader.ts b/packages/cli/src/capture/assetDownloader.ts index 8cc2bb41f..fdcdcf94f 100644 --- a/packages/cli/src/capture/assetDownloader.ts +++ b/packages/cli/src/capture/assetDownloader.ts @@ -10,6 +10,7 @@ import { join, extname } from "node:path"; import { createHash } from "node:crypto"; import type { DesignTokens, DownloadedAsset } from "./types.js"; import type { CatalogedAsset } from "./assetCataloger.js"; +import { rankIconCandidates, type IconCandidate } from "./faviconRanker.js"; interface DownloadBudgetOptions { remainingMs?: () => number; @@ -93,7 +94,7 @@ export async function downloadAssets( tokens: DesignTokens, outputDir: string, catalogedAssets?: CatalogedAsset[], - faviconLinks?: Array<{ rel: string; href: string }>, + faviconLinks?: IconCandidate[], options: DownloadBudgetOptions = {}, ): Promise<{ assets: DownloadedAsset[]; drops: AssetDropCounts }> { const assetsDir = join(outputDir, "assets"); @@ -133,8 +134,8 @@ export async function downloadAssets( } } - // 2. Favicon - const icons = faviconLinks || []; + // 2. Favicon — best declared candidate first, falling back through the rest on failure. + const icons = rankIconCandidates(faviconLinks || []); for (const [index, icon] of icons.entries()) { const remainingMs = options.remainingMs?.() ?? 10_000; if (remainingMs <= 0) { diff --git a/packages/cli/src/capture/faviconRanker.test.ts b/packages/cli/src/capture/faviconRanker.test.ts new file mode 100644 index 000000000..669bc015f --- /dev/null +++ b/packages/cli/src/capture/faviconRanker.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it } from "vitest"; +import { parseSizes, rankIconCandidates, type IconCandidate } from "./faviconRanker.js"; + +/** + * Fixtures transcribe the `` tags these sites declare in their document head. + * They are the shapes that made DOM-order downloading land the worst icon on disk. + */ + +// linear.app: legacy .ico declared first, the SVG and the 180px apple-touch after it. +const LINEAR: IconCandidate[] = [ + { rel: "icon", href: "https://linear.app/favicon.ico", sizes: "any", type: null }, + { rel: "icon", href: "https://linear.app/favicon.svg", sizes: null, type: "image/svg+xml" }, + { + rel: "apple-touch-icon", + href: "https://linear.app/apple-touch-icon.png", + sizes: "180x180", + type: null, + }, +]; + +// notion.com: .ico first, then an apple-touch png that declares no `sizes`. +const NOTION: IconCandidate[] = [ + { rel: "icon", href: "https://www.notion.com/front-static/favicon.ico", sizes: null, type: null }, + { + rel: "apple-touch-icon", + href: "https://www.notion.com/front-static/logo-ios.png", + sizes: null, + type: null, + }, +]; + +// stripe.com: svg, a 96x96 png, a shortcut .ico, and a 180x180 apple-touch png. +const STRIPE: IconCandidate[] = [ + { + rel: "icon", + href: "https://images.stripeassets.com/x/favicon.svg", + sizes: null, + type: "image/svg+xml", + }, + { + rel: "icon", + href: "https://images.stripeassets.com/x/favicon.png?w=96&h=96", + sizes: "96x96", + type: "image/png", + }, + { + rel: "shortcut icon", + href: "https://assets.stripeassets.com/x/favicon.ico", + sizes: null, + type: null, + }, + { + rel: "apple-touch-icon", + href: "https://images.stripeassets.com/x/favicon.png?w=180&h=180", + sizes: "180x180", + type: null, + }, +]; + +const hrefs = (cs: IconCandidate[]): string[] => rankIconCandidates(cs).map((c) => c.href); + +describe("rankIconCandidates", () => { + it("puts the SVG first even though the page declares the .ico first", () => { + expect(hrefs(LINEAR)).toEqual([ + "https://linear.app/favicon.svg", + "https://linear.app/apple-touch-icon.png", + "https://linear.app/favicon.ico", + ]); + }); + + it("prefers an unsized apple-touch-icon over a .ico", () => { + expect(hrefs(NOTION)).toEqual([ + "https://www.notion.com/front-static/logo-ios.png", + "https://www.notion.com/front-static/favicon.ico", + ]); + }); + + it("orders svg, then largest declared size, then .ico", () => { + expect(hrefs(STRIPE)).toEqual([ + "https://images.stripeassets.com/x/favicon.svg", + "https://images.stripeassets.com/x/favicon.png?w=180&h=180", + "https://images.stripeassets.com/x/favicon.png?w=96&h=96", + "https://assets.stripeassets.com/x/favicon.ico", + ]); + }); + + it("does not let a Safari pinned-tab silhouette beat the real favicon", () => { + // A mask-icon is an SVG, so ranking on format alone would promote the silhouette. + const masked: IconCandidate[] = [ + { rel: "mask-icon", href: "https://x.test/pinned.svg", sizes: null, type: null }, + { rel: "icon", href: "https://x.test/favicon.png", sizes: "32x32", type: "image/png" }, + { rel: "icon", href: "https://x.test/favicon.svg", sizes: null, type: "image/svg+xml" }, + ]; + expect(hrefs(masked)).toEqual([ + "https://x.test/favicon.svg", + "https://x.test/favicon.png", + "https://x.test/pinned.svg", + ]); + }); + + it("still returns a mask-icon when the page declares nothing else", () => { + // Ranked last, not dropped: a silhouette on disk beats no icon at all. + const only: IconCandidate[] = [{ rel: "mask-icon", href: "https://x.test/pinned.svg" }]; + expect(hrefs(only)).toEqual(["https://x.test/pinned.svg"]); + }); + + it("drops candidates with no href", () => { + expect(hrefs([{ rel: "icon", href: "" }, ...NOTION])).toHaveLength(2); + }); + + it("keeps DOM order between candidates of equal rank", () => { + const same: IconCandidate[] = [ + { rel: "icon", href: "https://x.test/a.png", sizes: "32x32" }, + { rel: "icon", href: "https://x.test/b.png", sizes: "32x32" }, + ]; + expect(hrefs(same)).toEqual(["https://x.test/a.png", "https://x.test/b.png"]); + }); +}); + +describe("parseSizes", () => { + it("reads a single declaration", () => { + expect(parseSizes("32x32")).toBe(32); + }); + + it("takes the largest of a multi-size declaration", () => { + expect(parseSizes("180x180 167x167")).toBe(180); + }); + + it("scores `any` as no declared pixel size", () => { + expect(parseSizes("any")).toBe(0); + }); + + it("scores a missing attribute as no declared pixel size", () => { + expect(parseSizes(null)).toBe(0); + expect(parseSizes(undefined)).toBe(0); + }); +}); diff --git a/packages/cli/src/capture/faviconRanker.ts b/packages/cli/src/capture/faviconRanker.ts new file mode 100644 index 000000000..424189783 --- /dev/null +++ b/packages/cli/src/capture/faviconRanker.ts @@ -0,0 +1,92 @@ +/** + * Rank declared `` candidates so the capture downloads the BEST one, + * not whichever the page happened to declare first. + * + * Pages routinely declare a legacy 16px `.ico` first and the good asset (an SVG, or a + * 180x180 apple-touch PNG) after it. Downloading in DOM order therefore lands the worst + * icon on disk, and the `sizes`/`type` attributes that say so are the only evidence — + * the bytes are only fetched for the winner, so quality cannot be measured after the fact. + * + * Pure: no IO, no network. Order only. + */ + +export interface IconCandidate { + rel: string; + href: string; + /** `sizes` attribute verbatim, e.g. "32x32", "any", "180x180 167x167". */ + sizes?: string | null; + /** `type` attribute verbatim, e.g. "image/svg+xml". */ + type?: string | null; +} + +/** Apple's spec size for a `apple-touch-icon` that declares no `sizes`. */ +const APPLE_TOUCH_DEFAULT_PX = 180; + +/** + * Largest pixel edge declared in a `sizes` attribute. `any` (used by SVG and by legacy + * `.ico` files alike) declares no pixel size at all, so it scores 0 rather than Infinity. + */ +export function parseSizes(sizes: string | null | undefined): number { + if (!sizes) return 0; + let max = 0; + for (const token of sizes.trim().split(/\s+/)) { + const m = /^(\d+)x(\d+)$/i.exec(token); + if (!m) continue; // "any" and anything malformed + max = Math.max(max, Number(m[1]), Number(m[2])); + } + return max; +} + +function pathnameOf(href: string): string { + try { + return new URL(href).pathname.toLowerCase(); + } catch { + return href.split(/[#?]/)[0]!.toLowerCase(); + } +} + +function isSvg(c: IconCandidate): boolean { + return c.type?.toLowerCase() === "image/svg+xml" || pathnameOf(c.href).endsWith(".svg"); +} + +/** + * `rel="mask-icon"` is Safari's pinned-tab asset: a single-colour silhouette, drawn in whatever + * tint the browser picks. It is not the site mark, and it is served as an SVG, so ranking by + * format alone would promote a monochrome outline over the page's real colour favicon. + */ +function isMaskIcon(c: IconCandidate): boolean { + return c.rel.toLowerCase().split(/\s+/).includes("mask-icon"); +} + +function isIco(c: IconCandidate): boolean { + const t = c.type?.toLowerCase(); + return ( + t === "image/x-icon" || t === "image/vnd.microsoft.icon" || pathnameOf(c.href).endsWith(".ico") + ); +} + +function declaredSize(c: IconCandidate): number { + const parsed = parseSizes(c.sizes); + if (parsed > 0) return parsed; + return c.rel.toLowerCase().split(/\s+/).includes("apple-touch-icon") ? APPLE_TOUCH_DEFAULT_PX : 0; +} + +function tierOf(c: IconCandidate): number { + // ponytail: mask-icon is ranked last rather than filtered out, so a page that declares + // nothing else still lands an icon instead of none. + if (isMaskIcon(c)) return 3; + if (isSvg(c)) return 0; + return isIco(c) ? 2 : 1; +} + +/** + * Best-first order: SVG, then largest declared size, then `.ico`, then a pinned-tab `mask-icon` + * as a last resort. Stable within a tier, so DOM order breaks ties. + */ +export function rankIconCandidates(candidates: IconCandidate[]): IconCandidate[] { + return candidates + .filter((c) => !!c.href) + .map((c, i) => ({ c, i, tier: tierOf(c), size: declaredSize(c) })) + .sort((a, b) => a.tier - b.tier || b.size - a.size || a.i - b.i) + .map((e) => e.c); +} diff --git a/packages/cli/src/capture/index.ts b/packages/cli/src/capture/index.ts index 7b9bb834b..388508ed2 100644 --- a/packages/cli/src/capture/index.ts +++ b/packages/cli/src/capture/index.ts @@ -23,6 +23,7 @@ import { noDrops, totalDrops, } from "./assetDownloader.js"; +import type { IconCandidate } from "./faviconRanker.js"; import { extractFontMetadata } from "./fontMetadataExtractor.js"; import { normalizeErrorMessage } from "../utils/errorMessage.js"; import { diag } from "../ui/diagnostics.js"; @@ -575,10 +576,20 @@ export async function captureWebsite( const visibleTextContent = await extractVisibleText(page1); // Extract favicon links before closing page (removed from tokens to reduce noise) + // `sizes` and `type` are the only evidence of icon quality: page.html on disk does not + // keep the tags, and the bytes are only fetched for the candidate that wins, so + // dropping these attributes here makes the choice unrecoverable downstream. const faviconLinks = (await page1.evaluate(`(() => { var iconEls = Array.from(document.querySelectorAll('link[rel*="icon"], link[rel="apple-touch-icon"]')); - return iconEls.map(function(l) { return { rel: l.rel, href: l.href }; }); - })()`)) as Array<{ rel: string; href: string }>; + return iconEls.map(function(l) { + return { + rel: l.rel, + href: l.href, + sizes: l.getAttribute('sizes'), + type: l.getAttribute('type'), + }; + }); + })()`)) as IconCandidate[]; await page1.close();