Skip to content

Fix interactive HTML media display for Arweave iframe content#2293

Merged
simo6529 merged 2 commits into6529-Collections:mainfrom
maybehotcarl:fix/arweave-iframe-gateway-timeout-and-csp
Apr 21, 2026
Merged

Fix interactive HTML media display for Arweave iframe content#2293
simo6529 merged 2 commits into6529-Collections:mainfrom
maybehotcarl:fix/arweave-iframe-gateway-timeout-and-csp

Conversation

@maybehotcarl
Copy link
Copy Markdown
Contributor

@maybehotcarl maybehotcarl commented Apr 17, 2026

Summary

  • Gateway fallback timeout fires while iframe is off-screen: The 8-second fallback timeout in InteractiveHtmlMediaDisplay started on component mount, but SandboxedExternalIframe defers iframe rendering until its IntersectionObserver fires. Off-screen cards (e.g. below the fold in a list) exhausted all gateway URLs before the user scrolled to them, causing the iframe to land on a broken/blocked gateway. Added an onVisible callback from SandboxedExternalIframe so the timeout only starts once the iframe is actually in the viewport.

  • CSP blocks gateway.ar.io redirect target: gateway.ar.io redirects interactive content to <txid>.ar.io (not <txid>.gateway.ar.io), but frame-src only allowed *.gateway.ar.io. Added ar.io to CSP sources via a separate ADDITIONAL_CSP_HOSTS array — this keeps it out of the fallback gateway retry list while covering the redirect target in the Content Security Policy.

Reproduction

Any text/html drop hosted on Arweave (e.g. interactive meme card submissions) would glitch and error when:

  1. The card was off-screen long enough for the gateway timeout to cycle through all 4 gateways (~32 seconds)
  2. The final gateway (gateway.ar.io) redirected to *.ar.io which was blocked by CSP

Files changed

  • lib/media/arweave-gateways.ts — new ADDITIONAL_CSP_HOSTS array, included in ARWEAVE_GATEWAY_CSP_SOURCES
  • components/common/SandboxedExternalIframe.tsx — new optional onVisible callback prop, fired when IntersectionObserver triggers
  • components/drops/view/item/content/media/MediaDisplay.tsxInteractiveHtmlMediaDisplay defers fallback timeout until onVisible fires

Test plan

  • Load a wave with an interactive HTML (text/html) Arweave submission
  • Verify the card renders without glitching when scrolled into view
  • Verify the card still falls back to alternate gateways if the primary gateway fails (after becoming visible)
  • Verify image/video drops are unaffected
  • Check browser console for CSP violations — *.ar.io should no longer be blocked

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced media loading reliability through improved iframe visibility detection for fallback timing
  • Chores

    • Added ar.io gateway support to security policies

Two bugs caused interactive HTML drops (text/html via Arweave) to glitch
and fail to render:

1. Gateway fallback timeout fired while iframe was off-screen — the
   8-second timeout in InteractiveHtmlMediaDisplay started on mount, but
   SandboxedExternalIframe defers rendering until the IntersectionObserver
   fires. Off-screen cards exhausted all gateway URLs before the user
   scrolled to them. Fixed by adding an onVisible callback so the timeout
   only starts once the iframe is actually in the viewport.

2. CSP blocked gateway.ar.io redirect target — gateway.ar.io redirects
   interactive content to <txid>.ar.io, but frame-src only allowed
   *.gateway.ar.io. Added ar.io to CSP sources (without adding it to the
   fallback gateway list) via a separate ADDITIONAL_CSP_HOSTS array.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough

Walkthrough

Added an optional onVisible callback prop to SandboxedExternalIframe that fires when the iframe becomes visible via IntersectionObserver. Integrated this callback into MediaDisplay to track visibility state and gate the iframe fallback URL progression. Expanded CSP sources to include ar.io domain.

Changes

Cohort / File(s) Summary
Iframe Visibility Integration
components/common/SandboxedExternalIframe.tsx, components/drops/view/item/content/media/MediaDisplay.tsx
Added optional onVisible callback prop to SandboxedExternalIframe that invokes when iframe transitions to render-ready. MediaDisplay now tracks iframe visibility state and gates fallback URL advancement to require the iframe be visible before proceeding.
CSP Configuration
lib/media/arweave-gateways.ts
Introduced ADDITIONAL_CSP_HOSTS constant containing ar.io and expanded ARWEAVE_GATEWAY_CSP_SOURCES generation to include both original gateway hosts and additional CSP hosts, permitting ar.io and its subdomains.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Html rendering arweave #2152: Touches the same iframe visibility/fallback flow and Arweave gateway configuration in both SandboxedExternalIframe and lib/media/arweave-gateways.ts.
  • Homepage interactive winner #2138: Modifies MediaDisplay's iframe rendering logic and SandboxedExternalIframe integration, directly related to the main component changes.
  • Submit html #1592: Introduced the SandboxedExternalIframe component itself; this PR extends its API with the onVisible prop for visibility tracking.

Suggested reviewers

  • simo6529
  • ragnep

Poem

🐰 A frame once hidden in the shadows deep,
Now signals when it wakes from slumber's keep—
onVisible whispers, "I'm ready now!"
The fallback stands at attention, we vow.
From ar.io gates, new worlds unfold! 🌐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 directly describes the main change: fixing interactive HTML media display for Arweave iframe content, which aligns with the PR's core objective of addressing two issues preventing proper rendering.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
Copy Markdown

@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

🧹 Nitpick comments (1)
components/common/SandboxedExternalIframe.tsx (1)

77-98: Missing onVisible in the effect's dependency array.

onVisible is referenced inside the effect but omitted from deps on Line 98. This will trip react-hooks/exhaustive-deps and, if a caller passes a non-memoized callback whose closure matters, could fire a stale reference. Functionally fine today because the effect is single-shot (short-circuits on isVisible) and callers only use it to flip a boolean, but add it to deps for correctness and to keep the lint rule clean.

Proposed fix
-  }, [canonicalSrc, isVisible]);
+  }, [canonicalSrc, isVisible, onVisible]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/common/SandboxedExternalIframe.tsx` around lines 77 - 98, The
useEffect that observes the iframe references onVisible but does not include it
in the dependency array; update the effect's deps to include onVisible (i.e.,
change the dependency array from [canonicalSrc, isVisible] to [canonicalSrc,
isVisible, onVisible]) so the hook honors the latest callback reference and
satisfies react-hooks/exhaustive-deps while leaving the existing short-circuit
logic in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/media/arweave-gateways.ts`:
- Around line 9-11: The ADDITIONAL_CSP_HOSTS constant currently broadens CSP to
all ar.io subdomains; narrow it by either (A) replacing the single "ar.io" entry
with a more specific pattern used only where necessary (e.g., only add
"https://*.ar.io" into FRAME_CSP_HOSTS and remove it from CONNECT/MEDIA lists)
or (B) keep ADDITIONAL_CSP_HOSTS as "ar.io" but change callers to only include
it in frame-src (not connect-src/media-src); update references to
ADDITIONAL_CSP_HOSTS or the CSP construction logic so that frame-related CSPs
allow ar.io redirects while connect/media CSPs do not include the broad ar.io
wildcard.

---

Nitpick comments:
In `@components/common/SandboxedExternalIframe.tsx`:
- Around line 77-98: The useEffect that observes the iframe references onVisible
but does not include it in the dependency array; update the effect's deps to
include onVisible (i.e., change the dependency array from [canonicalSrc,
isVisible] to [canonicalSrc, isVisible, onVisible]) so the hook honors the
latest callback reference and satisfies react-hooks/exhaustive-deps while
leaving the existing short-circuit logic in place.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e8576819-6c64-487f-827f-e32772b24aef

📥 Commits

Reviewing files that changed from the base of the PR and between 7b94e57 and 0a07254.

📒 Files selected for processing (3)
  • components/common/SandboxedExternalIframe.tsx
  • components/drops/view/item/content/media/MediaDisplay.tsx
  • lib/media/arweave-gateways.ts

Comment thread lib/media/arweave-gateways.ts
@sonarqubecloud
Copy link
Copy Markdown

@simo6529 simo6529 merged commit 057e2c7 into 6529-Collections:main Apr 21, 2026
4 checks passed
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