Fix HN content not rendering for iframe-blocked URLs #9
Conversation
Some websites (neal.fun, reuters.com, etc.) block iframe embedding via X-Frame-Options or CSP frame-ancestors headers. Previously, these URLs showed blank pages without user feedback. Changes: - Add HTTP header pre-check before opening URL in panel - If iframe embedding is blocked, open URL in external browser instead - Show informational message to user when redirecting to browser - Add timeout-based detection in webview as fallback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @eatnug, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where certain Hacker News content URLs failed to render within the application's iframe due to websites blocking embedding. It introduces a proactive mechanism to check for iframe-blocking HTTP headers before attempting to load content. If embedding is restricted, the URL is automatically opened in an external browser, accompanied by a user notification. Additionally, a client-side fallback is implemented to robustly handle cases where iframe loading might silently fail, ensuring a better user experience for inaccessible content. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust mechanism to handle URLs that block iframe embedding. The server-side pre-check of X-Frame-Options and CSP headers is a great approach to proactively identify and handle these URLs by opening them in an external browser. My review focuses on refining the fallback logic. I've suggested making the server-side check's error handling safer by assuming non-embeddability on failure. I also pointed out some issues with the new complex client-side fallback logic in the webview, including flawed cross-origin detection and code duplication, and suggested a re-evaluation of its complexity versus its benefits.
| // On error (timeout, network issues), assume embeddable and let iframe handle it | ||
| return true; |
There was a problem hiding this comment.
The catch block currently assumes a URL is embeddable if the fetch request fails (e.g., due to a timeout). This is an optimistic approach that can lead to a poor user experience: the user waits for the check to time out, then the UI attempts to load the URL in an iframe, which might also fail, resulting in a blank panel.
A safer approach would be to assume the URL is not embeddable on error. This would redirect to an external browser, which is a better fallback than a blank panel, preventing the user from seeing a blank, non-functional view.
| // On error (timeout, network issues), assume embeddable and let iframe handle it | |
| return true; | |
| // On error (timeout, network issues), assume not embeddable and open externally | |
| return false; |
| // Track if content loaded successfully | ||
| let loadSuccessful = false; | ||
| let loadTimeout: ReturnType<typeof setTimeout> | null = null; | ||
|
|
||
| const showError = () => { | ||
| const loading = document.getElementById('content-loading'); | ||
| const error = document.getElementById('content-error'); | ||
| if (loading) loading.classList.add('hidden'); | ||
| if (error) error.classList.remove('hidden'); | ||
| iframe.style.display = 'none'; | ||
| }; | ||
|
|
||
| const showContent = () => { | ||
| loadSuccessful = true; | ||
| if (loadTimeout) { | ||
| clearTimeout(loadTimeout); | ||
| loadTimeout = null; | ||
| } | ||
| const loading = document.getElementById('content-loading'); | ||
| if (loading) loading.classList.add('hidden'); | ||
| iframe.style.display = 'block'; | ||
| }; | ||
|
|
||
| // Set a timeout to detect if iframe fails to load (X-Frame-Options, CSP blocks) | ||
| // Many sites block iframe embedding but don't trigger error events | ||
| loadTimeout = setTimeout(() => { | ||
| if (!loadSuccessful) { | ||
| // After timeout, check if iframe has any accessible content | ||
| try { | ||
| // Try to access iframe content - will throw for blocked/cross-origin frames | ||
| const doc = iframe.contentDocument; | ||
| // If we can access and it's empty or about:blank, likely blocked | ||
| if (doc && (doc.body?.innerHTML === '' || doc.URL === 'about:blank')) { | ||
| showError(); | ||
| return; | ||
| } | ||
| // If accessible and has content, show it | ||
| if (doc && doc.body?.innerHTML) { | ||
| showContent(); | ||
| return; | ||
| } | ||
| // Cross-origin but might be loaded - give benefit of doubt | ||
| showContent(); | ||
| } catch { | ||
| // Cross-origin access denied - this is normal for loaded external sites | ||
| // The iframe might still be showing content, so show it | ||
| showContent(); | ||
| } | ||
| } | ||
| }, 5000); | ||
|
|
||
| iframe.addEventListener( | ||
| 'load', | ||
| () => { | ||
| const loading = document.getElementById('content-loading'); | ||
| if (loading) loading.classList.add('hidden'); | ||
| // iframe loaded - but might be empty if blocked by X-Frame-Options | ||
| // Check if we can detect empty content | ||
| setTimeout(() => { | ||
| try { | ||
| const doc = iframe.contentDocument; | ||
| // If we can access the document and it's essentially empty, likely blocked | ||
| if (doc) { | ||
| const bodyContent = doc.body?.innerHTML?.trim() || ''; | ||
| const hasContent = bodyContent.length > 0 && doc.URL !== 'about:blank'; | ||
| if (hasContent) { | ||
| showContent(); | ||
| } else { | ||
| // Empty content likely means blocked | ||
| showError(); | ||
| } | ||
| } else { | ||
| // Can't access document (cross-origin) - assume loaded successfully | ||
| showContent(); | ||
| } | ||
| } catch { | ||
| // Cross-origin access denied - iframe is loaded with external content | ||
| showContent(); | ||
| } | ||
| }, 100); | ||
| }, | ||
| { signal: getSignal() } | ||
| ); | ||
|
|
||
| iframe.addEventListener( | ||
| 'error', | ||
| () => { | ||
| const loading = document.getElementById('content-loading'); | ||
| const error = document.getElementById('content-error'); | ||
| if (loading) loading.classList.add('hidden'); | ||
| if (error) error.classList.remove('hidden'); | ||
| if (loadTimeout) { | ||
| clearTimeout(loadTimeout); | ||
| loadTimeout = null; | ||
| } | ||
| showError(); | ||
| }, | ||
| { signal: getSignal() } | ||
| ); |
There was a problem hiding this comment.
This new client-side logic for detecting blocked iframes is quite complex and has a couple of issues:
-
Flawed Cross-Origin Detection: For a cross-origin iframe blocked by
X-Frame-Options, theloadevent typically still fires. Your logic then tries to accessiframe.contentDocument, which throws a cross-origin error. Thecatchblock then optimistically callsshowContent(), resulting in a blank iframe being shown to the user. This unfortunately doesn't solve the problem this fallback is intended to address. -
Code Duplication: The content inspection logic within the 5-second
setTimeout(lines 859-878) is nearly identical to the logic inside theloadevent handler (lines 888-907). This could be refactored into a shared helper function to improve maintainability.
Given that the new server-side pre-check is the most reliable mechanism, you might consider simplifying or removing this complex client-side fallback. The current implementation adds significant complexity without reliably handling the primary cross-origin failure case.
Summary
Problem
Some HN content URLs (neal.fun, reuters.com, etc.) fail to render because these sites block iframe embedding via HTTP headers. Users see blank pages without feedback.
Test plan
🤖 Generated with Claude Code