fix(embeds): quality pass — close button, focus, a11y, perf, SoundCloud revert

From the quality-review agents: - Revert on.soundcloud.com support: the w.soundcloud widget doesn't follow the redirect (needs an oEmbed resolve, deferred). - Add a Close button to playing video/TikTok embeds and a Collapse button to the expanded X post — playback was previously one-way (only escapable by scrolling). - focus-visible ring on the embed facade (folds resets outline:none, leaving keyboard users with no indicator). - Only subscribe to resize postMessages while the iframe is mounted (was attaching a global listener per Instagram/Reddit facade before play). - TikTok oEmbed fetch now uses AbortController (abort on unmount) + aria-busy / 'Loading…' label on the resolving spinner. - Decorative facade thumbnails use alt="" (parent already names them); drop the dangling-colon aria-labels when there's no title. - Cap URL previews at 6 per message so a link-dump can't spawn dozens of fetches. Tests 728. No CSP change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-07 00:45:51 -04:00
parent a28c305835
commit a52c9e12a4
5 changed files with 98 additions and 24 deletions
@@ -86,7 +86,9 @@ export function RenderMessageContent({
  eventId,
 }: RenderMessageContentProps) {
  const renderUrlsPreview = (urls: string[]) => {
-    const filteredUrls = urls.filter((url) => !testMatrixTo(url));
+    // Cap previews per message so a link-dump doesn't spawn dozens of preview
+    // fetches + iframes at once.
+    const filteredUrls = urls.filter((url) => !testMatrixTo(url)).slice(0, 6);
    if (filteredUrls.length === 0) return undefined;
    return (
      <UrlPreviewHolder>
@@ -255,6 +255,12 @@ export const EmbedFacade = style([
    ':hover': {
      filter: 'brightness(0.85)',
    },
+    selectors: {
+      '&:focus-visible': {
+        outline: `2px solid ${color.Primary.Main}`,
+        outlineOffset: '-2px',
+      },
+    },
  },
 ]);

@@ -676,7 +676,27 @@ function TwitterCard({
  if (canEmbed && expanded && tweetId) {
    return (
      <>
-        {header}
+        <div className={previewCss.TwitterHeader}>
+          <XLogoIcon size={18} />
+          <Text priority="400" style={{ fontWeight: 600 }} truncate>
+            {name}
+          </Text>
+          {handle && (
+            <Text size="T200" priority="300" style={{ opacity: 0.55 }} truncate>
+              @{handle}
+            </Text>
+          )}
+          <Box grow="Yes" />
+          <IconButton
+            size="300"
+            radii="300"
+            variant="SurfaceVariant"
+            onClick={() => setExpanded(false)}
+            aria-label="Collapse post"
+          >
+            <Icon size="100" src={Icons.Cross} />
+          </IconButton>
+        </div>
        <UrlPreviewContent>
          <TweetEmbed id={tweetId} />
        </UrlPreviewContent>
@@ -1059,8 +1079,9 @@ function MediaEmbedCard({
    : embed.provider === 'instagram'
      ? INSTAGRAM_ORIGINS
      : REDDIT_ORIGINS;
+  // Only subscribe to resize messages while the iframe is actually mounted.
  const { ref: resizeRef, height: resizeHeight } = useIframeAutoHeight(
-    resizeOrigins,
+    playing ? resizeOrigins : NO_RESIZE_ORIGINS,
    embed.height ?? 480,
  );
  // 'rich' providers (esp. Reddit) can carry a bot-wall title — suppress it.
@@ -1093,7 +1114,8 @@ function MediaEmbedCard({
  const mediaStyle = fixedHeight ? { height: `${embed.height ?? 152}px` } : undefined;

  const thumb = thumbnailUrl ? (
-    <img className={previewCss.MediaThumbnailImg} src={thumbnailUrl} alt={title} loading="lazy" />
+    // alt="" — the parent button/link already carries the accessible name.
+    <img className={previewCss.MediaThumbnailImg} src={thumbnailUrl} alt="" loading="lazy" />
  ) : (
    <span className={previewCss.EmbedPlaceholder}>
      <Icon size="600" src={Icons.Play} />
@@ -1155,7 +1177,7 @@ function MediaEmbedCard({
              target="_blank"
              rel="noreferrer"
              className={previewCss.EmbedFacade}
-              aria-label={`Open on ${badge.label}: ${title}`}
+              aria-label={title ? `Open on ${badge.label}: ${title}` : `Open on ${badge.label}`}
            >
              {thumb}
              {playOverlay}
@@ -1177,11 +1199,29 @@ function MediaEmbedCard({
          >
            <SiteBadge label={badge.label} colorClass={badge.class} />
          </Text>
-          {playing && video && (
-            <Chip variant="Secondary" radii="Pill" onClick={enterFullscreen} aria-label="Fullscreen">
+          {playing && (
+            <Box alignItems="Center" gap="100" shrink="No">
+              {video && (
+                <Chip
+                  variant="Secondary"
+                  radii="Pill"
+                  onClick={enterFullscreen}
+                  aria-label="Fullscreen"
+                >
                  <Text size="T200">⛶ Fullscreen</Text>
                </Chip>
              )}
+              <IconButton
+                size="300"
+                radii="300"
+                variant="SurfaceVariant"
+                onClick={() => setPlaying(false)}
+                aria-label="Close player"
+              >
+                <Icon size="100" src={Icons.Cross} />
+              </IconButton>
+            </Box>
+          )}
        </Box>
        {/* While a video plays, drop the title/description so the player isn't
            squeezed by text below it. */}
@@ -1213,6 +1253,10 @@ function TikTokEmbedCard({ url, prev }: { url: string; prev: IPreviewUrlResponse
  const [resolving, setResolving] = useState(false);
  const [failed, setFailed] = useState(false);
  const mediaRef = useRef<HTMLDivElement>(null);
+  const abortRef = useRef<AbortController | null>(null);
+
+  // Abort an in-flight oEmbed resolve if the card unmounts (scrolled away).
+  useEffect(() => () => abortRef.current?.abort(), []);

  const mxcImage = prev['og:image'] as string | undefined;
  const rawTitle = prev['og:title'] ?? '';
@@ -1228,9 +1272,12 @@ function TikTokEmbedCard({ url, prev }: { url: string; prev: IPreviewUrlResponse
    }
    setResolving(true);
    setFailed(false);
+    const controller = new AbortController();
+    abortRef.current = controller;
    try {
-      const res = await fetch(tiktokOembedUrl(url));
+      const res = await fetch(tiktokOembedUrl(url), { signal: controller.signal });
      const id = tiktokIdFromOembed(await res.json());
+      if (controller.signal.aborted) return;
      if (id) {
        setVideoId(id);
        setPlaying(true);
@@ -1238,9 +1285,9 @@ function TikTokEmbedCard({ url, prev }: { url: string; prev: IPreviewUrlResponse
        setFailed(true);
      }
    } catch {
-      setFailed(true);
+      if (!controller.signal.aborted) setFailed(true);
    } finally {
-      setResolving(false);
+      if (!controller.signal.aborted) setResolving(false);
    }
  }, [url, videoId]);

@@ -1249,7 +1296,7 @@ function TikTokEmbedCard({ url, prev }: { url: string; prev: IPreviewUrlResponse
  const facadeInner = (
    <>
      {thumbnailUrl ? (
-        <img className={previewCss.MediaThumbnailImg} src={thumbnailUrl} alt={title} loading="lazy" />
+        <img className={previewCss.MediaThumbnailImg} src={thumbnailUrl} alt="" loading="lazy" />
      ) : (
        <span className={previewCss.EmbedPlaceholder} />
      )}
@@ -1280,7 +1327,10 @@ function TikTokEmbedCard({ url, prev }: { url: string; prev: IPreviewUrlResponse
            className={previewCss.EmbedFacade}
            onClick={start}
            disabled={resolving}
-            aria-label={`Play TikTok${title ? `: ${title}` : ''}`}
+            aria-busy={resolving}
+            aria-label={
+              resolving ? 'Loading TikTok…' : `Play TikTok${title ? `: ${title}` : ''}`
+            }
          >
            {facadeInner}
          </button>
@@ -1311,9 +1361,25 @@ function TikTokEmbedCard({ url, prev }: { url: string; prev: IPreviewUrlResponse
            <SiteBadge label="TikTok" colorClass={previewCss.BadgeTikTok} />
          </Text>
          {playing && videoId && (
-            <Chip variant="Secondary" radii="Pill" onClick={enterFullscreen} aria-label="Fullscreen">
+            <Box alignItems="Center" gap="100" shrink="No">
+              <Chip
+                variant="Secondary"
+                radii="Pill"
+                onClick={enterFullscreen}
+                aria-label="Fullscreen"
+              >
                <Text size="T200">⛶ Fullscreen</Text>
              </Chip>
+              <IconButton
+                size="300"
+                radii="300"
+                variant="SurfaceVariant"
+                onClick={() => setPlaying(false)}
+                aria-label="Close player"
+              >
+                <Icon size="100" src={Icons.Cross} />
+              </IconButton>
+            </Box>
          )}
        </Box>
        {failed && (
@@ -132,11 +132,11 @@ test('Spotify target + height', () => {
  assert.equal(spotifyEmbedHeight('album'), 352);
 });

-test('SoundCloud track detection (+ on. short link)', () => {
+test('SoundCloud track detection', () => {
  assert.equal(isSoundCloudTrack('https://soundcloud.com/artist/some-track'), true);
  assert.equal(isSoundCloudTrack('https://soundcloud.com/artist'), false); // bare profile
-  assert.equal(isSoundCloudTrack('https://on.soundcloud.com/abc123'), true); // share short link
-  assert.equal(parseMediaEmbed('https://on.soundcloud.com/abc123', 'h')?.provider, 'soundcloud');
+  // on.soundcloud.com short links intentionally not handled (need oEmbed resolve)
+  assert.equal(isSoundCloudTrack('https://on.soundcloud.com/abc123'), false);
 });

 test('buildVideoEmbedUrl: cookie-less YouTube + Vimeo', () => {
@@ -247,10 +247,10 @@ export function getSpotifyEmbedTarget(url: string): { type: SpotifyType; id: str
 export function isSoundCloudTrack(url: string): boolean {
  try {
    const { hostname, pathname } = new URL(url);
-    const h = hostname.replace(/^www\./, '');
-    // on.soundcloud.com/<code> is a share short link — the widget follows it.
-    if (h === 'on.soundcloud.com') return pathname.replace(/^\/+|\/+$/g, '').length > 0;
-    if (h !== 'soundcloud.com') return false;
+    // NOTE: on.soundcloud.com short links are NOT handled here — the w.soundcloud
+    // widget resolver doesn't follow the redirect; supporting them needs an oEmbed
+    // round-trip (soundcloud.com/oembed is CORS-enabled) to get the canonical URL.
+    if (hostname.replace(/^www\./, '') !== 'soundcloud.com') return false;
    // /<artist>/<track|sets/set> — at least two segments, not a bare profile
    const parts = pathname.replace(/^\/+|\/+$/g, '').split('/').filter(Boolean);
    return parts.length >= 2;