authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-09-05 00:28:48-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-05 00:28:48-07:00
log3e544f8cda371dc565c806508a9ab38b80c96ff7
treea151e167784c34865b091e23caf2eb9c1ed1c705
parent4e59b06b84fdfa36f78b4fc9079dff88b4ff3dc4
signature Signed by PGP key B5690EEEBB952194

refactor(shared): test favicon selection through public API (#10005)

2 files changed, 18 insertions(+), 13 deletions(-)

packages/shared/src/favicon.test.ts+16-11
...@@ -1,11 +1,6 @@...@@ -1,11 +1,6 @@
1import { describe, expect, it } from "@effect/vitest";1import { describe, expect, it } from "@effect/vitest";
22
3import {3import { faviconUrlForOrigin, toolActivityFaviconUrl } from "./favicon.ts";
4 explicitFaviconUrl,
5 faviconUrlForOrigin,
6 faviconUrlForPage,
7 toolActivityFaviconUrl,
8} from "./favicon.ts";
94
10describe("faviconUrlForOrigin", () => {5describe("faviconUrlForOrigin", () => {
11 it.each([6 it.each([
...@@ -46,12 +41,12 @@ describe("faviconUrlForOrigin", () => {...@@ -46,12 +41,12 @@ describe("faviconUrlForOrigin", () => {
46 );41 );
47});42});
4843
49describe("faviconUrlForPage", () => {44describe("toolActivityFaviconUrl", () => {
50 it("uses the page origin instead of a third-party favicon service", () => {45 it("uses the page origin instead of a third-party favicon service", () => {
51 expect(faviconUrlForPage("https://example.com/docs/page?q=1")).toBe(46 expect(toolActivityFaviconUrl({ pageUrl: "https://example.com/docs/page?q=1" }, "light")).toBe(
52 "https://example.com/favicon.ico",47 "https://example.com/favicon.ico",
53 );48 );
54 expect(faviconUrlForPage("http://localhost:5173/app")).toBe(49 expect(toolActivityFaviconUrl({ pageUrl: "http://localhost:5173/app" }, "light")).toBe(
55 "http://localhost:5173/favicon.ico",50 "http://localhost:5173/favicon.ico",
56 );51 );
57 });52 });
...@@ -86,7 +81,17 @@ describe("faviconUrlForPage", () => {...@@ -86,7 +81,17 @@ describe("faviconUrlForPage", () => {
86 });81 });
8782
88 it("accepts provider-supplied image URLs but rejects extension URLs", () => {83 it("accepts provider-supplied image URLs but rejects extension URLs", () => {
89 expect(explicitFaviconUrl("https://example.com/icon.png")).toBe("https://example.com/icon.png");84 expect(
90 expect(explicitFaviconUrl("chrome-extension://example/_favicon/")).toBeNull();85 toolActivityFaviconUrl(
86 { pageUrl: "https://example.com/docs", faviconUrl: "https://example.com/icon.png" },
87 "light",
88 ),
89 ).toBe("https://example.com/icon.png");
90 expect(
91 toolActivityFaviconUrl(
92 { pageUrl: "https://example.com/docs", faviconUrl: "chrome-extension://example/_favicon/" },
93 "light",
94 ),
95 ).toBe("https://example.com/favicon.ico");
91 });96 });
92});97});
packages/shared/src/favicon.ts+2-2
...@@ -5,7 +5,7 @@ import { isPublicFaviconHost } from "./hostClassification.ts";...@@ -5,7 +5,7 @@ import { isPublicFaviconHost } from "./hostClassification.ts";
5 * conventional favicon and let the image element fall back to a browser glyph.5 * conventional favicon and let the image element fall back to a browser glyph.
6 * Chrome-backed tools can pass their tab's explicit favicon URL separately.6 * Chrome-backed tools can pass their tab's explicit favicon URL separately.
7 */7 */
8export function faviconUrlForPage(rawUrl: string | null | undefined, _size = 32): string | null {8function faviconUrlForPage(rawUrl: string | null | undefined, _size = 32): string | null {
9 if (!rawUrl || rawUrl.length > 4096) return null;9 if (!rawUrl || rawUrl.length > 4096) return null;
10 try {10 try {
11 const pageUrl = new URL(rawUrl);11 const pageUrl = new URL(rawUrl);
...@@ -40,7 +40,7 @@ function themedFaviconUrlForPage(...@@ -40,7 +40,7 @@ function themedFaviconUrlForPage(
40}40}
4141
42/** Accepts image URLs supplied by a trusted provider event. */42/** Accepts image URLs supplied by a trusted provider event. */
43export function explicitFaviconUrl(rawUrl: string | null | undefined): string | null {43function explicitFaviconUrl(rawUrl: string | null | undefined): string | null {
44 if (!rawUrl || rawUrl.length > 4096) return null;44 if (!rawUrl || rawUrl.length > 4096) return null;
45 try {45 try {
46 const url = new URL(rawUrl);46 const url = new URL(rawUrl);