Skip to content

feat(web): play videos inline on bookmark cards#2895

Open
Santofer wants to merge 2 commits into
karakeep-app:mainfrom
Santofer:pr/inline-video-player
Open

feat(web): play videos inline on bookmark cards#2895
Santofer wants to merge 2 commits into
karakeep-app:mainfrom
Santofer:pr/inline-video-player

Conversation

@Santofer

Copy link
Copy Markdown

What

Bookmark cards can now play video inline instead of only linking out:

  • Self-hosted videos archived by yt-dlp (videoAssetId) play via a native HTML5 <video> player. The /api/assets route already supports range requests, so seeking works.
  • YouTube / Vimeo links show a poster + play button and load the privacy-friendly embed (youtube-nocookie / player.vimeo) only on click — so no iframe is mounted per card up front.

The player fills the card's image slot, so it works across all layouts (grid / masonry / list / compact).

Why

Today, videos only play in the bookmark preview. Since karakeep already downloads videos (yt-dlp) and serves them with range support, surfacing a player directly on the card is a natural, low-cost improvement for anyone who saves a lot of video.

Notes

  • No schema or backend changesvideoAssetId is already exposed on the link bookmark content, and /api/assets already streams with range support.
  • Embeds are loaded lazily (only on click) and use the privacy-friendly player domains.
  • lint, format and typecheck pass for @karakeep/web.

Screenshots

Self-hosted <video> player card + YouTube poster/play card — added below.

Bookmark cards can now play video inline instead of only linking out:
- self-hosted videos archived by yt-dlp (videoAssetId) use a native HTML5
  <video> player (the /api/assets route already supports range requests, so
  seeking works);
- YouTube and Vimeo links show a poster with a play button and load the
  privacy-friendly embed (youtube-nocookie / player.vimeo) only on click, so
  no iframe is mounted per card up front.

The player fills the card's image slot, so it works across all layouts
(grid / masonry / list / compact). No schema or backend changes are needed:
videoAssetId is already exposed on the link bookmark content.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds inline video playback to bookmark cards by introducing a VideoEmbed component that replaces the static image slot for video bookmarks. It covers three cases: yt-dlp-downloaded assets via a native <video> player, and YouTube/Vimeo URLs via a lazy-loaded privacy-friendly iframe.

  • Self-hosted videos (videoAssetId) render immediately with a native <video> using preload=\"metadata\" and range-request seeking.
  • YouTube/Vimeo bookmarks show a poster + play button; the iframe is only mounted on click, so no embeds fire until user interaction.
  • isVideo detection and videoAssetId prioritization logic are straightforward and consistent between LinkCard and VideoEmbed.

Confidence Score: 4/5

The change is additive and touches only one component; the core logic is straightforward and the new code path is only activated for video bookmarks. The findings are all quality/best-practice issues with no impact on correctness.

The VideoEmbed logic correctly prioritizes local assets over embeds, lazy-loads iframes on click, and reuses existing asset URL patterns. The three flagged issues — missing iframe sandbox, an inert track element, and absent Vimeo thumbnail — are all non-blocking improvements rather than functional defects.

Only apps/web/components/dashboard/bookmarks/LinkCard.tsx changed; the iframe sandbox and Vimeo thumbnail issues are worth addressing before merging but do not break any existing behaviour.

Security Review

  • apps/web/components/dashboard/bookmarks/LinkCard.tsx — The YouTube/Vimeo <iframe> is rendered without a sandbox attribute, allowing the embedded content to navigate the top-level browsing context and open popups. While both providers are trusted, applying sandbox="allow-scripts allow-same-origin allow-presentation allow-popups allow-popups-to-escape-sandbox" is the recommended defence-in-depth posture for any third-party embed.

Important Files Changed

Filename Overview
apps/web/components/dashboard/bookmarks/LinkCard.tsx Adds inline video playback to bookmark cards via a new VideoEmbed component: native HTML5 player for yt-dlp downloads, lazy-loaded iframe embed for YouTube/Vimeo. Three style/quality findings: missing iframe sandbox, empty track element, and no Vimeo thumbnail in the pre-play state.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/web/components/dashboard/bookmarks/LinkCard.tsx:114
**Missing `sandbox` attribute on embedded iframe**

Without a `sandbox` attribute the embedded YouTube/Vimeo iframe can navigate the top-level frame (`window.top.location`), open popups, and run scripts without restriction relative to its own origin. A permissive but still scoped sandbox — `"allow-scripts allow-same-origin allow-presentation allow-popups allow-popups-to-escape-sandbox"` — keeps autoplay and full-screen working while preventing top-navigation by the embed. The existing `allow` feature policy alone does not constrain these capabilities.

### Issue 2 of 3
apps/web/components/dashboard/bookmarks/LinkCard.tsx:98-100
The `<track>` element requires a `src` attribute; omitting it produces a browser console warning and the track is effectively inert. The existing video in `LinkContentSection.tsx` uses an `eslint-disable` comment to honestly skip the unfulfilled requirement. This `<track>` satisfies the static lint check but doesn't actually suppress the runtime warning.

```suggestion
      >
        {/* eslint-disable-next-line jsx-a11y/media-has-caption -- captions not (yet) available */}
      </video>
```

### Issue 3 of 3
apps/web/components/dashboard/bookmarks/LinkCard.tsx:115-139
**Vimeo preview shows no thumbnail before play**

When `vimeoId` is set and `playing` is `false`, the play button renders over a plain dark background — there is no poster image, unlike YouTube which fetches `i.ytimg.com/vi/${youTubeId}/hqdefault.jpg`. Vimeo thumbnails are available without authentication via their oEmbed endpoint (`https://vimeo.com/api/oembed.json?url=...`) or the static CDN pattern, so users see an uninformative black card until they click. The `{youTubeId && <Image … />}` guard on line 125 is the root of the discrepancy — there is no equivalent branch for Vimeo.

Reviews (1): Last reviewed commit: "feat(web): play videos inline on bookmar..." | Re-trigger Greptile

allow="autoplay; encrypted-media; picture-in-picture"
allowFullScreen
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 security Missing sandbox attribute on embedded iframe

Without a sandbox attribute the embedded YouTube/Vimeo iframe can navigate the top-level frame (window.top.location), open popups, and run scripts without restriction relative to its own origin. A permissive but still scoped sandbox — "allow-scripts allow-same-origin allow-presentation allow-popups allow-popups-to-escape-sandbox" — keeps autoplay and full-screen working while preventing top-navigation by the embed. The existing allow feature policy alone does not constrain these capabilities.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/components/dashboard/bookmarks/LinkCard.tsx
Line: 114

Comment:
**Missing `sandbox` attribute on embedded iframe**

Without a `sandbox` attribute the embedded YouTube/Vimeo iframe can navigate the top-level frame (`window.top.location`), open popups, and run scripts without restriction relative to its own origin. A permissive but still scoped sandbox — `"allow-scripts allow-same-origin allow-presentation allow-popups allow-popups-to-escape-sandbox"` — keeps autoplay and full-screen working while preventing top-navigation by the embed. The existing `allow` feature policy alone does not constrain these capabilities.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +98 to +100
>
<track kind="captions" />
</video>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The <track> element requires a src attribute; omitting it produces a browser console warning and the track is effectively inert. The existing video in LinkContentSection.tsx uses an eslint-disable comment to honestly skip the unfulfilled requirement. This <track> satisfies the static lint check but doesn't actually suppress the runtime warning.

Suggested change
>
<track kind="captions" />
</video>
>
{/* eslint-disable-next-line jsx-a11y/media-has-caption -- captions not (yet) available */}
</video>
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/components/dashboard/bookmarks/LinkCard.tsx
Line: 98-100

Comment:
The `<track>` element requires a `src` attribute; omitting it produces a browser console warning and the track is effectively inert. The existing video in `LinkContentSection.tsx` uses an `eslint-disable` comment to honestly skip the unfulfilled requirement. This `<track>` satisfies the static lint check but doesn't actually suppress the runtime warning.

```suggestion
      >
        {/* eslint-disable-next-line jsx-a11y/media-has-caption -- captions not (yet) available */}
      </video>
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +115 to +139
<button
type="button"
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
setPlaying(true);
}}
className="group/play absolute inset-0 h-full w-full"
aria-label="Play video"
>
{youTubeId && (
<Image
src={`https://i.ytimg.com/vi/${youTubeId}/hqdefault.jpg`}
alt={title ?? "video thumbnail"}
fill
unoptimized
className="object-cover"
/>
)}
<span className="absolute inset-0 flex items-center justify-center bg-black/10 transition group-hover/play:bg-black/25">
<span className="flex h-14 w-14 items-center justify-center rounded-full bg-black/55 backdrop-blur-sm transition group-hover/play:scale-110 group-hover/play:bg-black/75">
<Play className="ml-0.5 h-6 w-6 fill-white text-white" />
</span>
</span>
</button>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Vimeo preview shows no thumbnail before play

When vimeoId is set and playing is false, the play button renders over a plain dark background — there is no poster image, unlike YouTube which fetches i.ytimg.com/vi/${youTubeId}/hqdefault.jpg. Vimeo thumbnails are available without authentication via their oEmbed endpoint (https://vimeo.com/api/oembed.json?url=...) or the static CDN pattern, so users see an uninformative black card until they click. The {youTubeId && <Image … />} guard on line 125 is the root of the discrepancy — there is no equivalent branch for Vimeo.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/components/dashboard/bookmarks/LinkCard.tsx
Line: 115-139

Comment:
**Vimeo preview shows no thumbnail before play**

When `vimeoId` is set and `playing` is `false`, the play button renders over a plain dark background — there is no poster image, unlike YouTube which fetches `i.ytimg.com/vi/${youTubeId}/hqdefault.jpg`. Vimeo thumbnails are available without authentication via their oEmbed endpoint (`https://vimeo.com/api/oembed.json?url=...`) or the static CDN pattern, so users see an uninformative black card until they click. The `{youTubeId && <Image … />}` guard on line 125 is the root of the discrepancy — there is no equivalent branch for Vimeo.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

- Add a scoped `sandbox` to the YouTube/Vimeo iframe so the embed can't
  navigate the top frame (keeps autoplay / fullscreen / popups working).
- Replace the inert <track> with the same eslint-disable
  (jsx-a11y/media-has-caption) + <source> pattern already used in
  LinkContentSection.tsx, so there's no runtime console warning.
- Fall back to the bookmark's crawled image as the pre-play poster when the
  provider has no thumbnail (e.g. Vimeo, or YouTube without one).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Santofer

Copy link
Copy Markdown
Author

Thanks for the review! Addressed all three points in c6c96c1:

  • iframe sandbox — added sandbox="allow-scripts allow-same-origin allow-presentation allow-popups allow-popups-to-escape-sandbox" to the YouTube/Vimeo embed, so it can't navigate the top frame while autoplay/fullscreen/popouts keep working.
  • Inert <track> — replaced it with the same {/* eslint-disable jsx-a11y/media-has-caption */} + <source> pattern already used in LinkContentSection.tsx, so there's no runtime console warning.
  • Vimeo thumbnail — the pre-play poster now falls back to the bookmark's crawled image when the provider has no thumbnail (Vimeo, or YouTube without one), instead of a black frame.

lint, format and typecheck pass for @karakeep/web.

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.

1 participant