fix(web): detect cross-origin support from probe redirect, fix broken thumbnail tooltip#1730
Conversation
… thumbnail tooltip Cap Pro users with custom R2/S3 buckets saw a red "Error" thumbnail on progress bar hover. The hostname heuristic in detectCrossOriginSupport returned false for R2/S3 URLs even when the fetch probe proved CORS works via a successful cross-origin redirect. This caused the <video> element to omit crossOrigin="anonymous", tainting the canvas on drawImage and throwing SecurityError on toDataURL. Fix: use response.redirected as a runtime CORS signal (if the browser followed a same-origin→cross-origin redirect in cors mode and the probe succeeded, CORS is definitively working). Extract canvas capture into a pure helper that returns undefined on all failure paths instead of red placeholder URLs. Add explicit null guard in MediaPlayerSeek so the tooltip degrades gracefully to time-only when capture fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| (_time: number): string | undefined => | ||
| captureVideoFrameDataUrl({ video: videoRef.current }), | ||
| [], | ||
| ); |
There was a problem hiding this comment.
_time parameter is permanently unused
generateVideoFrameThumbnail ignores the hovered time entirely and always captures videoRef.current (the current playback frame). The underscore prefix correctly signals the intent, and this is acknowledged in the PR description as a pre-existing issue. The consequence is that hover previews will always show the live playback frame, not the position being hovered over.
For future reference, when proper scrub previews are implemented (server-side sprite sheets + WebVTT), the parameter name and implementation will need to change.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
Line: 406-409
Comment:
**`_time` parameter is permanently unused**
`generateVideoFrameThumbnail` ignores the hovered `time` entirely and always captures `videoRef.current` (the current playback frame). The underscore prefix correctly signals the intent, and this is acknowledged in the PR description as a pre-existing issue. The consequence is that hover previews will always show the live playback frame, not the position being hovered over.
For future reference, when proper scrub previews are implemented (server-side sprite sheets + WebVTT), the parameter name and implementation will need to change.
How can I resolve this? If you propose a fix, please make it concise.| export function detectCrossOriginSupport( | ||
| url: string, | ||
| probeWasRedirected = false, | ||
| ): boolean { | ||
| if (probeWasRedirected) return true; |
There was a problem hiding this comment.
When videoSrc points directly at a presigned S3 or R2 URL (no initial redirect through /api/playlist), response.redirected is false, and the hostname blocklist still returns false, so thumbnails remain disabled even if CORS headers are correctly set on that bucket.
This is a narrow remaining gap: it affects users whose videoSrc is a bare presigned URL rather than a same-origin proxy. Not a regression introduced here, but worth a follow-up when expanding cross-origin support beyond the proxy-redirect path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/_components/playback-source.ts
Line: 66-70
Comment:
**Direct S3/R2 URLs without a redirect still blocked**
When `videoSrc` points directly at a presigned S3 or R2 URL (no initial redirect through `/api/playlist`), `response.redirected` is `false`, and the hostname blocklist still returns `false`, so thumbnails remain disabled even if CORS headers are correctly set on that bucket.
This is a narrow remaining gap: it affects users whose `videoSrc` is a bare presigned URL rather than a same-origin proxy. Not a regression introduced here, but worth a follow-up when expanding cross-origin support beyond the proxy-redirect path.
How can I resolve this? If you propose a fix, please make it concise.
|
Tested locally as working. 
|
2 participants
Summary
Cap Pro users with custom R2/S3 buckets see a red "Error" thumbnail when hovering over the video progress bar. Their CORS is configured correctly — the fetch probe follows a cross-origin redirect and the browser enforces CORS on the response — but
detectCrossOriginSupportignores the runtime signal and returnsfalsebased on a hostname blocklist, preventingcrossOrigin="anonymous"from being set on the<video>element.response.redirectedfrom the probe as a definitive signal that CORS works, short-circuiting the hostname blocklist for redirected responsescaptureVideoFrameDataUrlfunction returnsundefinedon all failure paths instead of red placeholder URLs (placeholder.pics/.../Error)MediaPlayerSeek.tooltipThumbnailSrctype and add null guard so the tooltip falls back to time-only display when capture failsRoot cause (HAR-verified)
The probe fetch to
/api/playlistreturns302redirecting to a presigned R2 URL. The browser enforces CORS on the cross-origin response —Access-Control-Allow-Originis present and correct. ButdetectCrossOriginSupportcheckshostname.includes("r2.cloudflarestorage.com")→ returnsfalse→crossOriginattribute omitted → canvas taints ondrawImage→toDataURLthrowsSecurityError→ catch returns red placeholder URL.Defense in depth
supportsCrossOriginis false →tooltipThumbnailSrcisundefined→ no canvas capture attemptedundefined(not a red error image)getThumbnailreceivesundefined→ returnsnull→ tooltip shows time onlyPre-existing issue (out of scope)
generateVideoFrameThumbnailignores itstimeparameter and always capturesvideo.currentTime. Proper scrub previews need server-side sprite sheets + WebVTT — separate, larger future PR.Test plan
pnpm --filter=@cap/web test— 783/783 pass (10 new/updated test cases)pnpm typecheck— no new errors (pre-existing PageProps issues unrelated)pnpm lint— no errors in changed filespnpm format— cleanpnpm dev:web, upload video, hover progress bar — no red "Error"🤖 Generated with Claude Code
Greptile Summary
This PR fixes a bug where Cap Pro users with custom R2/S3 buckets get a red "Error" thumbnail on video hover because
detectCrossOriginSupportwas using a hostname blocklist that ignored a successful CORS-validated redirect. The fix usesresponse.redirectedfrom the probe fetch as a definitive runtime signal to bypass the blocklist, and also extracts canvas capture into a testable helper that returnsundefined(rather than a red placeholder URL) on all failure paths.The logic is correct: if the probe
fetchfollows a cross-origin redirect and still returns a successful response, the browser has already validated that CORS headers are present, socrossOrigin=\"anonymous\"on the<video>element will work.Confidence Score: 5/5
Safe to merge — the fix is logically sound, well-tested, and only affects thumbnail behaviour for Pro users with custom buckets.
All findings are P2 (pre-existing acknowledged limitation and a style note about unused parameter). No regressions introduced; the CORS detection logic correctly uses
response.redirectedas a definitive runtime signal, and the canvas extraction refactor removes a UX-visible red error URL. 783 tests passing, types clean.No files require special attention.
Important Files Changed
detectCrossOriginSupportnow acceptsprobeWasRedirectedand short-circuits totrue, correctly enabling cross-origin mode when the probe fetch confirmed a valid CORS-redirected response.undefinedon all failure paths instead of placeholder error URLs; includesreadyState < 2guard and dependency injection for testability.captureVideoFrameDataUrl; condition for enabling thumbnails correctly changed toresolvedSrc.data?.supportsCrossOriginfor precise cross-origin gating.tooltipThumbnailSrccallback return to `stringredirected: trueon mock response and new cases for theprobeWasRedirectedparameter; S3 URL assertion correctly updated fromfalsetotrue.captureVideoFrameDataUrlfailure paths and the happy path, using dependency injection for canvas and video mocks.Sequence Diagram
sequenceDiagram participant Browser participant ProbeAPI as "/api/playlist (same-origin)" participant R2 as "R2/S3 Bucket (cross-origin)" participant Canvas Browser->>ProbeAPI: fetch(url, range bytes=0-0) ProbeAPI-->>Browser: 302 redirect to presigned R2 URL Browser->>R2: follow redirect in cors mode with Origin header R2-->>Browser: 206 Partial + Access-Control-Allow-Origin OK Note over Browser: response.redirected = true Browser->>Browser: detectCrossOriginSupport(url, true) returns true Note over Browser: video element gets crossOrigin=anonymous Browser->>Canvas: drawImage(video, 0, 0, w, h) Canvas-->>Browser: toDataURL returns jpeg data URLPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(web): detect cross-origin support fr..." | Re-trigger Greptile
(2/5) Greptile learns from your feedback when you react with thumbs up/down!