Improve favicon selection in fedify nodeinfo

CHANGES.md

@@ -8,6 +8,18 @@ Version 2.5.0
 
 To be released.
 
+### @fedify/cli
+
+ -  Changed the `fedify nodeinfo` favicon selector to pick a usable bitmap
+    icon more often.  It now skips SVG icons declared with
+    `type="image/svg+xml"`, not just those with a `.svg` URL, and it considers
+    every declared size instead of only the first, so an icon offering a large
+    size is no longer discarded because its smallest size is too small.
+    [[#893], [#1187] by Lee Jeongmin\]
+
+[#893]: https://github.com/fedify-dev/fedify/issues/893
+[#1187]: https://github.com/fedify-dev/fedify/pull/1187
+
 
 Version 2.4.0
 -------------

changes.d/cli/improve-favicon-selection.md

@@ -0,0 +1,11 @@
+---
+links:
+  '#1187': https://github.com/fedify-dev/fedify/pull/1187
+  '#893': https://github.com/fedify-dev/fedify/issues/893
+---
+ -  Changed the `fedify nodeinfo` favicon selector to pick a usable bitmap
+    icon more often.  It now skips SVG icons declared with
+    `type="image/svg+xml"`, not just those with a `.svg` URL, and it considers
+    every declared size instead of only the first, so an icon offering a large
+    size is no longer discarded because its smallest size is too small.
+    [[#893], [#1187] by Lee Jeongmin]

packages/cli/src/nodeinfo.test.ts

@@ -1,7 +1,7 @@
-import assert from "node:assert/strict";
-import { test } from "node:test";
 import { Chalk } from "chalk";
 import fetchMock from "fetch-mock";
+import assert from "node:assert/strict";
+import { afterEach, test } from "node:test";
 import {
   getAsciiArt,
   getFaviconUrl,
@@ -10,6 +10,10 @@ import {
   rgbTo256Color,
 } from "./nodeinfo.ts";
 
+afterEach(() => {
+  fetchMock.hardReset();
+});
+
 const HTML_WITH_SMALL_ICON = `
 <!DOCTYPE html>
 <html>
@@ -32,8 +36,6 @@ test("getFaviconUrl - small favicon.ico and apple-touch-icon.png", async () => {
 
   const result = await getFaviconUrl("https://example.com/");
   assert.equal(result.href, "https://example.com/apple-touch-icon.png");
-
-  fetchMock.hardReset();
 });
 
 const HTML_WITH_ICON = `
@@ -58,8 +60,6 @@ test("getFaviconUrl - favicon.ico and apple-touch-icon.png", async () => {
 
   const result = await getFaviconUrl("https://example.com/");
   assert.equal(result.href, "https://example.com/favicon.ico");
-
-  fetchMock.hardReset();
 });
 
 const HTML_WITH_SVG_ONLY = `
@@ -83,16 +83,14 @@ test("getFaviconUrl - svg icons only falls back to /favicon.ico", async () => {
 
   const result = await getFaviconUrl("https://example.com/");
   assert.equal(result.href, "https://example.com/favicon.ico");
-
-  fetchMock.hardReset();
 });
 
 const HTML_WITH_UPPERCASE_SVG_ONLY = `
 <!DOCTYPE html>
 <html>
 <head>
   <title>Test Site</title>
-  <link rel="icon" href="/icon.SVG" type="image/svg+xml">
+  <link rel="icon" href="/icon.SVG">
   </head>
 <body>Test</body>
 </html>
@@ -108,16 +106,14 @@ test("getFaviconUrl - uppercase svg icons only falls back to /favicon.ico", asyn
 
   const result = await getFaviconUrl("https://example.com/");
   assert.equal(result.href, "https://example.com/favicon.ico");
-
-  fetchMock.hardReset();
 });
 
 const HTML_WITH_UPPERCASE_SVG_WITH_QUERY = `
 <!DOCTYPE html>
 <html>
 <head>
   <title>Test Site</title>
-  <link rel="icon" href="/icon.SVG?v=1#icon" type="image/svg+xml">
+  <link rel="icon" href="/icon.SVG?v=1#icon">
   </head>
 <body>Test</body>
 </html>
@@ -133,8 +129,123 @@ test("getFaviconUrl - uppercase svg icons with query and hash fall back to /favi
 
   const result = await getFaviconUrl("https://example.com/");
   assert.equal(result.href, "https://example.com/favicon.ico");
+});
 
-  fetchMock.hardReset();
+const HTML_WITH_SVG_TYPE_ONLY = `
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test Site</title>
+  <link rel="icon" href="/icon" type="image/svg+xml">
+  </head>
+<body>Test</body>
+</html>
+`;
+
+test("getFaviconUrl - svg icons with type attribute only falls back to /favicon.ico", async () => {
+  fetchMock.spyGlobal();
+
+  fetchMock.get("https://example.com/", {
+    body: HTML_WITH_SVG_TYPE_ONLY,
+    headers: { "Content-Type": "text/html" },
+  });
+
+  const result = await getFaviconUrl("https://example.com/");
+  assert.equal(result.href, "https://example.com/favicon.ico");
+});
+
+const HTML_WITH_SIZES_ANY_ICON = `
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test Site</title>
+  <link rel="icon" href="/favicon.png" sizes="any"/>
+  </head>
+<body>Test</body>
+</html>
+`;
+
+test("getFaviconUrl - icons with sizes='any' is selected", async () => {
+  fetchMock.spyGlobal();
+
+  fetchMock.get("https://example.com/", {
+    body: HTML_WITH_SIZES_ANY_ICON,
+    headers: { "Content-Type": "text/html" },
+  });
+
+  const result = await getFaviconUrl("https://example.com/");
+  assert.equal(result.href, "https://example.com/favicon.png");
+});
+
+const HTML_WITH_MULTI_SIZE_ICON = `
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test Site</title>
+  <link rel="alternate icon" type="image/x-icon" href="/multi-size-icon.ico" sizes="16x16 32x32 48x48 256x256">
+  <link rel="apple-touch-icon" href="/apple-icon-180.png">
+  </head>
+<body>Test</body>
+</html>
+`;
+
+test("getFaviconUrl - icons with multiple sizes", async () => {
+  fetchMock.spyGlobal();
+
+  fetchMock.get("https://example.com/", {
+    body: HTML_WITH_MULTI_SIZE_ICON,
+    headers: { "Content-Type": "text/html" },
+  });
+
+  const result = await getFaviconUrl("https://example.com/");
+  assert.equal(result.href, "https://example.com/multi-size-icon.ico");
+});
+
+const HTML_WITH_PREFERRED_BITMAP_ICON = `
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test Site</title>
+  <link rel="icon" href="/icon.svg" type="image/svg+xml">
+  <link rel="icon" href="/favicon.png">
+  </head>
+<body>Test</body>
+</html>
+`;
+
+test("getFaviconUrl - prefer bitmap icons", async () => {
+  fetchMock.spyGlobal();
+
+  fetchMock.get("https://example.com/", {
+    body: HTML_WITH_PREFERRED_BITMAP_ICON,
+    headers: { "Content-Type": "text/html" },
+  });
+
+  const result = await getFaviconUrl("https://example.com/");
+  assert.equal(result.href, "https://example.com/favicon.png");
+});
+
+const HTML_WITH_MULTIPLE_REL_TOKENS = `
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test Site</title>
+  <link rel="alternate icon" href="/favicon.png" sizes="64x64">
+  </head>
+<body>Test</body>
+</html>
+`;
+
+test("getFaviconUrl - icon with multiple rel tokens", async () => {
+  fetchMock.spyGlobal();
+
+  fetchMock.get("https://example.com/", {
+    body: HTML_WITH_MULTIPLE_REL_TOKENS,
+    headers: { "Content-Type": "text/html" },
+  });
+
+  const result = await getFaviconUrl("https://example.com/");
+  assert.equal(result.href, "https://example.com/favicon.png");
 });
 
 const HTML_WITHOUT_ICON = `
@@ -157,8 +268,6 @@ test("getFaviconUrl - falls back to /favicon.ico", async () => {
 
   const result = await getFaviconUrl("https://example.com/");
   assert.equal(result.href, "https://example.com/favicon.ico");
-
-  fetchMock.hardReset();
 });
 
 test("rgbTo256Color - check RGB cube", () => {

packages/cli/src/nodeinfo.ts

@@ -371,13 +371,17 @@ export async function getFaviconUrl(
     }
     const rel = attrs.rel?.toLowerCase()?.trim()?.split(/\s+/) ?? [];
     if (!rel.includes("icon") && !rel.includes("apple-touch-icon")) continue;
-    if ("sizes" in attrs && attrs.sizes.match(/\d+x\d+/)) {
-      const [w, h] = attrs.sizes.split("x").map((v) => Number.parseInt(v));
-      if (w < 38 || h < 19) continue;
-    }
+    const tokens = attrs.sizes?.match(/\d+x\d+/g);
+    if (
+      tokens != null && !tokens.some((token) => {
+        const [w, h] = token.split("x").map((v) => Number.parseInt(v));
+        return w >= 38 && h >= 19;
+      })
+    ) continue;
     if ("href" in attrs) {
       const parsedUrl = new URL(attrs.href, response.url);
       if (parsedUrl.pathname.toLowerCase().endsWith(".svg")) continue;
+      if (attrs.type?.toLowerCase()?.trim() === "image/svg+xml") continue;
       return parsedUrl;
     }
   }