fix: drop favicon on cross-origin redirects

When a same-origin navigation redirects to a different origin, the favicon should be cleared to prevent stale icons from displaying the wrong site's identity.
This commit is contained in:
Jinjing
2026-09-05 13:41:29 -07:00
parent ff9f542324
commit 865d2246fd
3 changed files with 47 additions and 12 deletions
@@ -116,6 +116,7 @@ export function bindBrowserPageWebviewListeners({
const {
handleDidStartNavigation,
handleDidRedirectNavigation,
handleFullDidNavigate,
handleDidNavigateInPage,
handleTitleUpdate,
@@ -149,6 +150,7 @@ export function bindBrowserPageWebviewListeners({
webview.addEventListener('focus', dismissAddressBarSuggestions)
webview.addEventListener('did-start-loading', handleDidStartLoading)
webview.addEventListener('did-start-navigation', handleDidStartNavigation)
webview.addEventListener('did-redirect-navigation', handleDidRedirectNavigation)
webview.addEventListener('did-stop-loading', handleDidStopLoading)
// Why: close find only on full 'did-navigate', not the shared handler, which also fires on SPA in-page hash/pushState changes.
const handleFindCloseOnNavigate = (): void => {
@@ -186,6 +188,7 @@ export function bindBrowserPageWebviewListeners({
webview.removeEventListener('focus', dismissAddressBarSuggestions)
webview.removeEventListener('did-start-loading', handleDidStartLoading)
webview.removeEventListener('did-start-navigation', handleDidStartNavigation)
webview.removeEventListener('did-redirect-navigation', handleDidRedirectNavigation)
webview.removeEventListener('did-stop-loading', handleDidStopLoading)
webview.removeEventListener('did-navigate', handleFullDidNavigate)
webview.removeEventListener('did-navigate', handleFindCloseOnNavigate)
@@ -98,6 +98,21 @@ describe('favicon retention across navigations', () => {
expect(harness.updates.at(-1)).toEqual({ faviconUrl: null })
})
it('drops the icon when a same-origin navigation redirects to another origin', () => {
const harness = createHarness('https://github.com/nodejs/node')
harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] })
harness.navigateTo('https://github.com/login')
harness.navigation.handleDidRedirectNavigation({
isMainFrame: true,
isInPlace: false,
url: 'https://example.com/after-login'
} as Electron.DidRedirectNavigationEvent)
expect(harness.faviconUrlRef.current).toBeNull()
expect(harness.updates.at(-1)).toEqual({ faviconUrl: null })
})
it('does not clear on a same-document navigation', () => {
const harness = createHarness('https://github.com/nodejs/node')
harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] })
@@ -45,6 +45,7 @@ export type BrowserPageWebviewNavigationHandlersArgs = {
export type BrowserPageWebviewNavigationHandlers = {
handleDidStartNavigation: (event: Electron.DidStartNavigationEvent) => void
handleDidRedirectNavigation: (event: Electron.DidRedirectNavigationEvent) => void
handleFullDidNavigate: (event: BrowserPageNavigateEvent) => void
handleDidNavigateInPage: (event: BrowserPageNavigateEvent) => void
handleTitleUpdate: (event: { title?: string }) => void
@@ -68,6 +69,28 @@ export function createBrowserPageWebviewNavigationHandlers({
annotationViewportBridgeTokenRef,
setBrowserOverlayViewport
}: BrowserPageWebviewNavigationHandlersArgs): BrowserPageWebviewNavigationHandlers {
const clearFaviconIfOriginChanges = (
event: Electron.DidStartNavigationEvent | Electron.DidRedirectNavigationEvent
): void => {
if (!event.isMainFrame || event.isInPlace || !event.url) {
return
}
const browserStartedUrl = redactKagiSessionToken(event.url)
const startedUrl = normalizeBrowserNavigationUrl(browserStartedUrl) ?? browserStartedUrl
// Why getURL() and not lastKnownWebviewUrlRef: Orca-driven navigations point that ref at the
// destination before assigning src, so it can't identify the document being left.
let committedUrl: string | null = null
try {
committedUrl = webview.getURL() || null
} catch {
// Why: a guest that hasn't attached yet rejects getURL(); an unknown origin keeps the icon.
}
if (browserNavigationLeavesFaviconOrigin(committedUrl, startedUrl)) {
faviconUrlRef.current = null
onUpdatePageStateRef.current(browserTabId, { faviconUrl: null })
}
}
const handleDidStartNavigation = (event: Electron.DidStartNavigationEvent): void => {
if (!event.isMainFrame || event.isInPlace || !event.url) {
return
@@ -81,18 +104,11 @@ export function createBrowserPageWebviewNavigationHandlers({
// Why here and not on did-start-loading: Chromium re-announces a favicon only when the icon URL
// list changes, so clearing on every load strands same-origin navigations with no icon and no
// event that would ever restore one.
// Why getURL() and not lastKnownWebviewUrlRef: Orca-driven navigations point that ref at the
// destination before assigning src, so it can't identify the document being left.
let committedUrl: string | null = null
try {
committedUrl = webview.getURL() || null
} catch {
// Why: a guest that hasn't attached yet rejects getURL(); an unknown origin keeps the icon.
}
if (browserNavigationLeavesFaviconOrigin(committedUrl, startedUrl)) {
faviconUrlRef.current = null
onUpdatePageStateRef.current(browserTabId, { faviconUrl: null })
}
clearFaviconIfOriginChanges(event)
}
const handleDidRedirectNavigation = (event: Electron.DidRedirectNavigationEvent): void => {
clearFaviconIfOriginChanges(event)
}
const handleDidNavigate = (
@@ -187,6 +203,7 @@ export function createBrowserPageWebviewNavigationHandlers({
return {
handleDidStartNavigation,
handleDidRedirectNavigation,
handleFullDidNavigate,
handleDidNavigateInPage,
handleTitleUpdate,