-
Notifications
You must be signed in to change notification settings - Fork 107
Switch to Google's favicon service for thumbnails #330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to Google's favicon service for thumbnails #330
Conversation
Replace custom hackers-thumbnails service with Google's favicon API which extracts the domain from URLs and requests 128px favicons.
|
Caution Review failedThe pull request is closed. WalkthroughThumbnail 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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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.
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
📒 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.
| components.host = "www.google.com" | ||
| components.path = "/s2/favicons" | ||
| components.queryItems = [ | ||
| URLQueryItem(name: "domain", value: host), | ||
| URLQueryItem(name: "sz", value: "128") | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and examine the ThumbnailView.swift file
find . -name "ThumbnailView.swift" -type fRepository: 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.swiftRepository: 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 3Repository: 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 thumbnailRepository: 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:
-
Privacy: User browsing domains are sent to Google's servers. Verify this aligns with your privacy policy and complies with GDPR/CCPA requirements.
-
Reliability: Google could change, rate-limit, or disable this endpoint without notice. While
AsyncImagegracefully falls back to a placeholder, unexpected behavior or degraded performance could occur. -
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.
Replace custom hackers-thumbnails service with Google's favicon API
which extracts the domain from URLs and requests 128px favicons.
Summary by CodeRabbit
Improvements
New Features
✏️ Tip: You can customize this high-level summary in your review settings.