Repository navigation
Display favicons for browser website entries - #18099
Conversation
Capture favicons from pages as they load and persist them with browser history entries. Display favicons in tabs, tab creation search results, and palette searches to improve visual recognition of websites and help users identify pages at a glance.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds a shared Merge Risk: 🔵 Low · up to Favicons from visited pages are stored and may later load remote images in tabs, history, and search, which can permit tracking or requests to attacker-selected network endpoints after the original page is no longer active. Existing validation and fallback behavior limit the impact, but security-owner awareness or follow-up on restricting these requests is recommended before merging. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, solution, implementation scope, and testing, but it does not follow the required template. It omits the ELI5, Why, Linked Issue, Review, AI Disclosure, and Agent skill upstream boundary sections. Visual Proof is still marked TODO, and the required checklist items remain unchecked. Resolution Complete the required template sections, provide the linked issue after “Fixes #”, attach before-and-after visual proof or write N/A with a reason if applicable, complete the ELI5 and Why sections, address AI Disclosure and Agent skill upstream boundary, and update the testing and checklist entries with the actual verification results.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: cdb67228-e0e8-47ee-9c9a-916886bf82a5
📒 Files selected for processing (21)
src/renderer/src/components/browser-favicon.tsxsrc/renderer/src/components/browser-pane/host-guest/attach-browser-page-webview.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.tssrc/renderer/src/components/tab-bar/BrowserTab.test.tsxsrc/renderer/src/components/tab-bar/BrowserTab.tsxsrc/renderer/src/components/tab-bar/TabBarCreateEntry.history.test.tsxsrc/renderer/src/components/tab-bar/TabBarCreateEntryRow.tsxsrc/renderer/src/components/tab-bar/open-tab-entry-dedupe.test.tssrc/renderer/src/components/tab-bar/open-tab-search.test.tssrc/renderer/src/components/tab-bar/open-tab-search.tssrc/renderer/src/components/tab-bar/open-tab-selection-routing.test.tssrc/renderer/src/components/worktree-jump-palette-browser-simulator-rows.tsxsrc/renderer/src/lib/browser-palette-search.test.tssrc/renderer/src/lib/browser-palette-search.tssrc/renderer/src/store/slices/browser.test.tssrc/renderer/src/store/slices/browser/browser-history-actions.tssrc/renderer/src/store/slices/browser/browser-page-state-actions.tssrc/renderer/src/store/slices/browser/browser-slice-contract.tssrc/shared/browser-workspace-types.tssrc/shared/workspace-session-browser-schema.tssrc/shared/workspace-session-schema.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
ℹ️ Minor suggestions only — the favicon retry semantics change is worth a look; otherwise this is a clean, well-tested PR.
Reviewed changes
- New
BrowserFaviconcomponent — shared<img>-with-globe-fallback that centralizes favicon rendering and URL validation (http/https/data:imageonly). - History favicon capture —
addBrowserHistoryEntrygains an optionalfaviconUrl;handleTitleUpdatepassesfaviconUrlRef.current. - History favicon refresh —
updateBrowserPageStateupdates the matching history entry when a page's favicon changes. - Type/schema + display surfaces —
faviconUrlthreaded throughBrowserHistoryEntry, the zod session schema,open-tab-search,browser-palette-search, and the tab/history/palette rows.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Reset the favicon failure cache when the favicon URL changes, enabling retry of a previously failed favicon when navigating back to the same URL. Distinguish between explicit null (clear cached favicon) and omitted (don't update history), so stale favicons don't persist incorrectly.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Fixed favicon retry after navigation —
BrowserFaviconnow resetsfailedUrlduring render on any favicon identity change (including clearing tonullwhile a page loads), so navigating back to a previously-failed URL retries instead of staying on the globe fallback. - Distinguished "unchanged" from "cleared" for history favicons —
addBrowserHistoryEntryandupdateBrowserPageStatenow treatundefinedas "leave the stored favicon alone" andnullas "clear it", with tests covering both paths.
The prior review's retry-semantics concern is cleanly addressed, and the new tests would fail without the render-time reset, so they're genuine coverage rather than theatre.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
* Display favicons for browser website entries Capture favicons from pages as they load and persist them with browser history entries. Display favicons in tabs, tab creation search results, and palette searches to improve visual recognition of websites and help users identify pages at a glance. * Fix favicon retry on back navigation after load failure Reset the favicon failure cache when the favicon URL changes, enabling retry of a previously failed favicon when navigating back to the same URL. Distinguish between explicit null (clear cached favicon) and omitted (don't update history), so stale favicons don't persist incorrectly.

Problem
Browser website entries show only a generic globe icon, making it harder to distinguish between different pages at a glance in tabs, history, and search results.
Solution
Capture favicon URLs from web pages as they load and display them in browser tabs, history rows, and search/palette results. Falls back to the globe icon if a favicon fails to load or is unavailable.
What Changed
BrowserFaviconcomponent handles favicon display with fallback to globe iconTesting
Visual Proof
TODO: Favicon display in browser tabs, history, and search results
Notes
Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Folder workspaces, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)