Skip to content

Conversation

@weiran
Copy link
Owner

@weiran weiran commented Nov 29, 2025

Replace custom hackers-thumbnails service with Google's favicon API
which extracts the domain from URLs and requests 128px favicons.

Summary by CodeRabbit

  • Improvements

    • More reliable and faster thumbnail loading using an updated external favicon service and a custom load-and-render flow.
    • Smoother in-app browsing with improved navigation state handling and asynchronous page loading.
  • New Features

    • Added an "Open in Safari" action and explicit Back/Forward/Reload controls (including an iPad-specific bottom bar) for easier navigation.

✏️ Tip: You can customize this high-level summary in your review settings.

Replace custom hackers-thumbnails service with Google's favicon API
which extracts the domain from URLs and requests 128px favicons.
@coderabbitai
Copy link

coderabbitai bot commented Nov 29, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Thumbnail loading now uses Google's favicon service by extracting the domain and requesting a 128px favicon; the embedded web view implementation was refactored to use a WebView wrapper with async loading, navigation monitoring, and UI controls for back/forward/reload and open-in-Safari.

Changes

Cohort / File(s) Summary
Thumbnail view update
DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift
Replaced custom thumbnail endpoint with Google's favicon service (www.google.com/s2/favicons). Now extracts the original URL's host (guard), builds domain and sz=128 query, replaces AsyncImage with private image state, adds loadThumbnail(from:) and cgImage(from:) helpers and a .task-driven loading/rendering flow.
Embedded web view refactor & controls
App/EmbeddedWebView.swift
Replaced WebKitView/Coordinator approach with a WebView(page) wrapper, added async load(_:) and monitorNavigations() tasks, introduced WebPage model state, navigation booleans (canGoBack, canGoForward), update helpers (updateState(), reload(), goBack(), goForward()), toolbar changes including open-in-Safari and iPad-specific bottom bar controls.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant UI as EmbeddedWebView UI
    participant WebView as WebView(page)
    participant WebPage as WebPage model
    participant System as Browser/System (Safari)

    User->>UI: Tap URL / Navigation controls
    UI->>WebView: load(URL) [async task]
    WebView->>WebView: start navigation
    WebView->>WebPage: update navigation state (URL, title, history)
    WebPage-->>WebView: state snapshot
    WebView->>UI: notify via model bindings
    UI->>User: update toolbar (back/forward enabled, title)
    alt Open in Safari
        User->>UI: Tap "Open in Safari"
        UI->>System: openURL(currentURL)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review async navigation monitoring and concurrency (@MainActor correctness).
  • Validate WebView ↔ WebPage binding consistency and race conditions during rapid navigations.
  • Inspect new toolbar and iPad bottom bar UI states and disabled logic for edge cases.
  • Confirm thumbnail data-to-CGImage conversion handles malformed/partial data and memory implications.

Poem

A rabbit nibbles on a code line, 🐇
Favicons fetched, the view looks fine,
Pages load with back and cheer,
Buttons hum and states appear,
Hopping through tabs, I toast the design. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Switch to Google's favicon service for thumbnails' accurately and concisely summarises the main change—replacing the custom thumbnail service with Google's favicon API.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 944c240 and 7246c85.

📒 Files selected for processing (2)
  • App/EmbeddedWebView.swift (3 hunks)
  • DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift (2 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47ed1c2 and 944c240.

📒 Files selected for processing (1)
  • DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: build
🔇 Additional comments (1)
DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift (1)

20-20: Good defensive programming.

The guard clause properly handles URLs without a host, preventing potential crashes or malformed thumbnail URLs.

Comment on lines +23 to +28
components.host = "www.google.com"
components.path = "/s2/favicons"
components.queryItems = [
URLQueryItem(name: "domain", value: host),
URLQueryItem(name: "sz", value: "128")
]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find and examine the ThumbnailView.swift file
find . -name "ThumbnailView.swift" -type f

Repository: weiran/Hackers

Length of output: 124


🏁 Script executed:

#!/bin/bash
# Read the full ThumbnailView.swift file with line numbers
cat -n ./DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift

Repository: weiran/Hackers

Length of output: 1982


🏁 Script executed:

#!/bin/bash
# Search for any documentation or comments about this favicon service change
rg -i "favicon|hackers-thumbnails" --type swift --type markdown --type txt -C 3

Repository: weiran/Hackers

Length of output: 813


🌐 Web query:

Google /s2/favicons official API documentation

💡 Result:

Short answer: Google does not publish an official, full API doc for /s2/favicons. The endpoint commonly used is:

https://www.google.com/s2/favicons?domain=example.com
Optional params (commonly supported by clients): sz (size), domain_url (full URL). It returns a PNG favicon (fallback to a generic icon if none). This endpoint is widely used but unofficial/undocumented and has no guaranteed SLA or formal rate limits. [1][2][3]

Sources:

  • Community/Stack Overflow discussion noting the endpoint is not a public API. [1]
  • Community-written reference/gist with usage examples (domain, sz, domain_url). [2]
  • Third‑party documentation that documents usage and warns it’s unofficial. [3]

🏁 Script executed:

#!/bin/bash
# Check if there are any tests for ThumbnailView to understand expected behavior
find . -name "*ThumbnailView*" -o -name "*test*" -o -name "*Test*" | grep -i thumbnail

Repository: weiran/Hackers

Length of output: 124


Verify privacy policy alignment and consider risks of undocumented external dependency.

The Google /s2/favicons endpoint is undocumented and unofficial with no SLA or rate limits, creating architectural risks:

  1. Privacy: User browsing domains are sent to Google's servers. Verify this aligns with your privacy policy and complies with GDPR/CCPA requirements.

  2. Reliability: Google could change, rate-limit, or disable this endpoint without notice. While AsyncImage gracefully falls back to a placeholder, unexpected behavior or degraded performance could occur.

  3. Documentation: Document why this endpoint was chosen over alternatives (self-hosted favicon service, CDN, etc.) for future maintainers.

Consider: documenting this decision, verifying privacy implications with your legal/compliance team, and evaluating whether a self-hosted or documented alternative is more appropriate for production use.

🤖 Prompt for AI Agents
In DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift around lines
23–28, the code uses Google's undocumented /s2/favicons endpoint which exposes
user browsing domains to an external provider and carries reliability risk;
update the codebase by (1) adding an in-file comment documenting why this
endpoint was chosen and listing risks (privacy, rate-limits, lack of SLA), (2)
moving the host/path into a configurable constant or environment-driven setting
so it can be switched to a self-hosted or documented CDN service without code
changes, (3) adding a short TODO to consult legal/compliance to confirm
GDPR/CCPA alignment before shipping, and (4) adding a note to the project
documentation/release notes describing the decision and recommended alternatives
(self-hosted favicon service, caching proxy, or approved third-party) so future
maintainers can revisit the choice.

@weiran weiran merged commit 3d9d317 into master Dec 3, 2025
2 of 4 checks passed
@weiran weiran deleted the claude/google-favicon-service-01RSQ3EZ5Lcw4kruHR2kh5JA branch December 3, 2025 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants