Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/renderer/src/components/browser-favicon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { useState } from 'react'
import { Globe } from 'lucide-react'
import { cn } from '@/lib/utils'

function displayableFaviconUrl(faviconUrl: string | null | undefined): string | null {
const trimmed = faviconUrl?.trim()
if (!trimmed) {
return null
}
if (trimmed.startsWith('data:image/')) {
return trimmed
}
try {
const url = new URL(trimmed)
return url.protocol === 'http:' || url.protocol === 'https:' ? trimmed : null
} catch {
return null
}
}

export function BrowserFavicon({
faviconUrl,
className,
fallbackClassName
}: {
faviconUrl: string | null | undefined
className?: string
fallbackClassName?: string
}): React.JSX.Element {
const displayUrl = displayableFaviconUrl(faviconUrl)
const [failedUrl, setFailedUrl] = useState<string | null>(null)

// Why: reset during render on any favicon identity change — including a clear to null while
// a page loads — so navigating back to the same url retries instead of keeping the fallback.
if (failedUrl !== null && failedUrl !== displayUrl) {
setFailedUrl(null)
}

if (displayUrl && failedUrl !== displayUrl) {
Comment thread
AmethystLiang marked this conversation as resolved.
Resolved
return (
<img
src={displayUrl}
alt=""
aria-hidden
draggable={false}
decoding="async"
loading="lazy"
fetchPriority="low"
className={cn(
'shrink-0 rounded-sm object-contain drop-shadow-[0_0_1px_var(--foreground)]',
className
)}
onError={() => setFailedUrl(displayUrl)}
/>
)
}

return <Globe className={cn('shrink-0', className, fallbackClassName)} aria-hidden="true" />
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ export type AttachBrowserPageWebviewArgs = {
setPendingAnnotationPayload: Dispatch<SetStateAction<BrowserGrabPayload | null>>
setBrowserOverlayViewport: Dispatch<SetStateAction<BrowserOverlayViewport>>
setAddressBarValue: Dispatch<SetStateAction<string>>
addBrowserHistoryEntryRef: MutableRefObject<(url: string, title: string) => void>
addBrowserHistoryEntryRef: MutableRefObject<
(url: string, title: string, faviconUrl?: string | null) => void
>
annotationViewportBridgeTokenRef: MutableRefObject<string>
initialBrowserUrlRef: MutableRefObject<string>
validateVisibleGuestRegistrationRef: MutableRefObject<() => void>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ export type BrowserPageWebviewNavigationHandlersArgs = {
addressBarInputRef: RefObject<HTMLInputElement | null>
onSetUrlRef: MutableRefObject<BrowserPageUrlSetter>
onUpdatePageStateRef: MutableRefObject<(tabId: string, updates: BrowserTabPageState) => void>
addBrowserHistoryEntryRef: MutableRefObject<(url: string, title: string) => void>
addBrowserHistoryEntryRef: MutableRefObject<
(url: string, title: string, faviconUrl?: string | null) => void
>
faviconUrlRef: MutableRefObject<string | null>
setAddressBarValue: Dispatch<SetStateAction<string>>
annotationViewportBridgeTokenRef: MutableRefObject<string>
Expand Down Expand Up @@ -127,7 +129,7 @@ export function createBrowserPageWebviewNavigationHandlers({
const browserModelUrl = redactKagiSessionToken(currentUrl)
const title = getBrowserDisplayTitle(event.title, browserModelUrl)
onUpdatePageStateRef.current(browserTabId, { title })
addBrowserHistoryEntryRef.current(browserModelUrl, title)
addBrowserHistoryEntryRef.current(browserModelUrl, title, faviconUrlRef.current)
} catch {
// Why: title-updated can fire before dom-ready, making getURL() throw.
}
Expand Down
34 changes: 32 additions & 2 deletions src/renderer/src/components/tab-bar/BrowserTab.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ describe('BrowserTab favicon', { timeout: 30_000 }, () => {
expect(images[0].props.alt).toBe('')
expect(images[0].props['aria-hidden']).toBe(true)
expect(images[0].props.draggable).toBe(false)
expect(images[0].props.className).toContain('size-3 mr-1 shrink-0')
expect(images[0].props.className).toContain('size-3')
expect(images[0].props.className).toContain('mr-1')
expect(images[0].props.className).toContain('shrink-0')
expect(images[0].props.className).toContain('object-contain')
expect(images[0].props.className).toContain('drop-shadow-[0_0_1px_var(--foreground)]')
expect(findElementsByType(element, 'Globe')).toHaveLength(0)
Expand All @@ -274,7 +276,9 @@ describe('BrowserTab favicon', { timeout: 30_000 }, () => {
expect(findElementsByType(element, 'img')).toHaveLength(0)
const globes = findElementsByType(element, 'Globe')
expect(globes).toHaveLength(1)
expect(globes[0].props.className).toContain('size-3 mr-1 shrink-0')
expect(globes[0].props.className).toContain('size-3')
expect(globes[0].props.className).toContain('mr-1')
expect(globes[0].props.className).toContain('shrink-0')
expect(globes[0].props.className).toContain('text-blue-500')
})

Expand Down Expand Up @@ -309,4 +313,30 @@ describe('BrowserTab favicon', { timeout: 30_000 }, () => {
expect(images[0].props.src).toBe(nextIconUrl)
expect(findElementsByType(resetRender, 'Globe')).toHaveLength(0)
})

it('retries a failed favicon after a navigation clears and restores the same url', async () => {
const iconUrl = 'https://example.com/favicon.ico'
const tab = baseBrowserTab({ faviconUrl: iconUrl })
const firstRender = await renderExpandedBrowserTab(tab)
const image = findElementsByType(firstRender, 'img')[0]

;(image.props.onError as () => void)()
const failedRender = await renderExpandedBrowserTab(tab)
expect(findElementsByType(failedRender, 'Globe')).toHaveLength(1)

// A load clears the favicon, then the same site reports it again.
const loadingRender = await renderExpandedBrowserTab(
baseBrowserTab({ id: tab.id, faviconUrl: null })
)
expect(findElementsByType(loadingRender, 'Globe')).toHaveLength(1)

const retryRender = await renderExpandedBrowserTab(
baseBrowserTab({ id: tab.id, faviconUrl: iconUrl })
)

const images = findElementsByType(retryRender, 'img')
expect(images).toHaveLength(1)
expect(images[0].props.src).toBe(iconUrl)
expect(findElementsByType(retryRender, 'Globe')).toHaveLength(0)
})
})
53 changes: 6 additions & 47 deletions src/renderer/src/components/tab-bar/BrowserTab.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { useEffect, useState } from 'react'
import { useSortable } from '@dnd-kit/sortable'
import {
Globe,
X,
ExternalLink,
Copy,
Expand Down Expand Up @@ -39,6 +38,7 @@ import { TabWorkspaceLayoutMenuSection } from './TabWorkspaceLayoutMenuSection'
import { useTabStripPointerActivation } from './tab-strip-pointer-activation'
import { TAB_CONTEXT_MENU_CONTENT_CLASS } from './tab-context-menu-sizing'
import { cn } from '@/lib/utils'
import { BrowserFavicon } from '@/components/browser-favicon'

export function formatBrowserTabUrlLabel(url: string): string {
if (url === ORCA_BROWSER_BLANK_URL || url === 'about:blank') {
Expand Down Expand Up @@ -68,51 +68,6 @@ function isBlankBrowserTab(tab: BrowserTabState): boolean {
return tab.url === ORCA_BROWSER_BLANK_URL || tab.url === 'about:blank'
}

type FailedFavicon = {
tabId: string
faviconUrl: string
}

function BrowserTabFavicon({
tabId,
faviconUrl
}: {
tabId: string
faviconUrl: string | null
}): React.JSX.Element {
const displayFaviconUrl = faviconUrl?.trim() ? faviconUrl : null
const [failedFavicon, setFailedFavicon] = useState<FailedFavicon | null>(null)

// Why: reset during render so a new favicon identity retries before the tab
// commits one frame with the stale fallback icon.
if (
failedFavicon &&
(failedFavicon.tabId !== tabId || failedFavicon.faviconUrl !== displayFaviconUrl)
) {
setFailedFavicon(null)
}

const currentFaviconFailed =
failedFavicon?.tabId === tabId && failedFavicon.faviconUrl === displayFaviconUrl

if (displayFaviconUrl && !currentFaviconFailed) {
return (
<img
src={displayFaviconUrl}
alt=""
aria-hidden
draggable={false}
// Why: transparent dark/light-mode favicons can disappear against tab
// chrome; a token-colored 1px shadow keeps the 12px mark legible.
className="size-3 mr-1 shrink-0 rounded-sm object-contain drop-shadow-[0_0_1px_var(--foreground)]"
onError={() => setFailedFavicon({ tabId, faviconUrl: displayFaviconUrl })}
/>
)
}

return <Globe className="size-3 mr-1 shrink-0 text-blue-500" />
}

export default function BrowserTab({
tab,
isActive,
Expand Down Expand Up @@ -234,7 +189,11 @@ export default function BrowserTab({
browser tabs at a glance even when the strip is saturated. We
keep full color on both active and inactive tabs — dimming to
muted-foreground made the icon read as "disabled" in practice. */}
<BrowserTabFavicon tabId={tab.id} faviconUrl={tab.faviconUrl} />
<BrowserFavicon
faviconUrl={tab.faviconUrl}
className="size-3 mr-1"
fallbackClassName="text-blue-500"
/>
{isPinned && <Pin className="mr-1 size-3 shrink-0 text-muted-foreground" aria-hidden />}
<span className={`${TAB_LABEL_WIDTH_CLASSES} mr-1`}>{tabLabel}</span>
{tab.loading && !tab.loadError && !isBlankBrowserTab(tab) && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,16 @@ describe('TabBarCreateEntry browser history rows', () => {
expect(rowTexts().some((text) => text.includes('Open page'))).toBe(false)
})

it('uses the favicon captured with a history entry', () => {
const faviconUrl = 'https://linear.app/favicon.ico'
historyStoreMock.entries = [historyEntry({ ...linear, faviconUrl })]
mount()

setQuery('linear')

expect(container.querySelector<HTMLImageElement>('[role="option"] img')?.src).toBe(faviconUrl)
})

it('skips history for a path-shaped query and for a forced search', () => {
pathLikeMock.value = true
mount()
Expand All @@ -192,7 +202,8 @@ describe('TabBarCreateEntry browser history rows', () => {
contentType: 'browser',
pageId: 'page-1',
workspaceId: 'ws-1',
url: 'https://linear.app/acme/team/ORC/active'
url: 'https://linear.app/acme/team/ORC/active',
faviconUrl: null
}
]
mount()
Expand Down
5 changes: 3 additions & 2 deletions src/renderer/src/components/tab-bar/TabBarCreateEntryRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { translate } from '@/i18n/i18n'
import { SEARCH_ENGINE_LABELS } from '../../../../shared/browser-url'
import { formatBrowserHistoryUrl } from '@/lib/browser-history-match'
import type { ActiveOption } from './tab-create-entry-active-option'
import { BrowserFavicon } from '@/components/browser-favicon'

export const RESULT_LISTBOX_ID = 'tab-create-entry-results'

Expand Down Expand Up @@ -138,7 +139,7 @@ function getOpenTabIcon(option: Extract<ActiveOption, { kind: 'tab' }>['option']
return <TerminalSquare className="size-3.5 shrink-0" aria-hidden="true" />
}
if (contentType === 'browser') {
return <Globe className="size-3.5 shrink-0" aria-hidden="true" />
return <BrowserFavicon faviconUrl={option.faviconUrl} className="size-3.5" />
}
if (contentType === 'simulator') {
return <Smartphone className="size-3.5 shrink-0" aria-hidden="true" />
Expand Down Expand Up @@ -191,7 +192,7 @@ function getActionPresentation(option: ActiveOption): {
// Why the title is detail, not label: the label span is shrink-0 whenever a
// detail shows, so a variable-length title there would refuse to truncate.
detail: entry.title ? `${entry.title} · ${url}` : url,
icon: <Globe className="size-3.5 shrink-0" aria-hidden="true" />,
icon: <BrowserFavicon faviconUrl={entry.faviconUrl} className="size-3.5" />,
label: translate('auto.components.tab.bar.TabBarCreateEntry.openPage', 'Open page'),
showDetail: true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ describe('dropFileEntriesCoveredByTabResults', () => {
contentType: 'browser',
pageId: 'page-1',
url: 'https://example.com/zebra',
workspaceId: 'ws-1'
workspaceId: 'ws-1',
faviconUrl: null
},
{
executionHostId: 'local',
Expand Down
10 changes: 7 additions & 3 deletions src/renderer/src/components/tab-bar/open-tab-search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,14 @@ function makeBrowserPage({
id,
title,
url = 'https://example.com/one',
faviconUrl = null,
workspaceLabel = null,
isCurrentPage = false
}: {
id: string
title: string
url?: string
faviconUrl?: string | null
workspaceLabel?: string | null
isCurrentPage?: boolean
}): SearchableBrowserPage {
Expand All @@ -140,7 +142,7 @@ function makeBrowserPage({
url,
title,
loading: false,
faviconUrl: null,
faviconUrl,
canGoBack: false,
canGoForward: false,
loadError: null,
Expand Down Expand Up @@ -445,10 +447,11 @@ describe('searchOpenTabs result fields', () => {
})

it('carries the activation identifiers each source needs', () => {
const faviconUrl = 'https://example.com/favicon.ico'
const results = search({
query: 'zebra',
workspaceTabs: [makeWorkspaceTab({ id: 'tab-1', title: 'Zebra tab' })],
browserPages: [makeBrowserPage({ id: 'page-1', title: 'Zebra page' })],
browserPages: [makeBrowserPage({ id: 'page-1', title: 'Zebra page', faviconUrl })],
simulatorTabs: [makeSimulatorTab({ id: 'sim-1', label: 'Zebra emulator' })]
})

Expand All @@ -467,7 +470,8 @@ describe('searchOpenTabs result fields', () => {
contentType: 'browser',
pageId: 'page-1',
workspaceId: 'page-1-ws',
worktreeId: 'wt-1'
worktreeId: 'wt-1',
faviconUrl
},
{
source: 'simulator',
Expand Down
4 changes: 3 additions & 1 deletion src/renderer/src/components/tab-bar/open-tab-search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export type OpenTabSearchResult =
pageId: string
workspaceId: string
url: string
faviconUrl: string | null
})
| (OpenTabSearchResultBase & {
source: 'simulator'
Expand Down Expand Up @@ -206,7 +207,8 @@ export function searchOpenTabs({
contentType: 'browser',
pageId: result.pageId,
workspaceId: result.workspaceId,
url: result.url
url: result.url,
faviconUrl: result.faviconUrl
})),
...rank('simulator', searchSimulatorTabs([...simulatorTabs], trimmed), (result) => ({
...baseResult('simulator', result.tabId, result, executionHostId),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ const browserResult: OpenTabSearchResult = {
contentType: 'browser',
pageId: 'page-1',
workspaceId: 'ws-1',
url: 'https://example.com/docs'
url: 'https://example.com/docs',
faviconUrl: null
}

const simulatorResult: OpenTabSearchResult = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type React from 'react'
import { Globe, Smartphone } from 'lucide-react'
import { Smartphone } from 'lucide-react'
import { CommandItem } from '@/components/ui/command'
import { RepoBadgeMark } from '@/components/repo/RepoBadgeLabel'
import { getPaletteHostBadge } from '@/components/cmd-j/palette-host-badge'
Expand All @@ -15,6 +15,7 @@ import {
} from './worktree-jump-palette-primitives'
import { formatPaletteSessionAge } from '@/components/cmd-j/palette-session-age'
import { resolvePaletteRepoForWorktree } from '@/lib/palette-repo-resolution'
import { BrowserFavicon } from '@/components/browser-favicon'

export function WorktreeJumpPaletteSimulatorRow({
entry,
Expand Down Expand Up @@ -147,7 +148,7 @@ export function WorktreeJumpPaletteBrowserRow({
)}
>
<div className="flex h-5 w-4 shrink-0 items-center justify-center self-start text-muted-foreground/85">
<Globe className="size-3.5" aria-hidden="true" />
<BrowserFavicon faviconUrl={result.faviconUrl} className="size-3.5" />
</div>
<div className="min-w-0 flex-1 overflow-hidden">
<div className="flex items-center justify-between gap-2.5">
Expand Down
Loading
Loading