fix(Markdown): Escape open video tags

.nx/version-plans/version-plan-1792000000000.md

@@ -0,0 +1,7 @@
+---
+gamut: patch
+---
+
+Fix `Video` YouTube/Vimeo splash not fitting its frame after the react-player v3 upgrade. The frame now uses `aspect-ratio: 16 / 9` instead of a `padding-top` ratio, so a consumer can size it through `className` and the provider's own splash fills it rather than overflowing. `Video` also passes `wrapper="div"` so a `placeholderImage` splash shares the player's wrapper element, as in react-player v2.
+
+`Markdown` no longer rewrites `<video>` tags before parsing, so a literal `<video>` in inline code or a fenced code block is left alone. A single-line `<video></video>` is still rendered without a wrapping `<p>`.

packages/gamut/src/Markdown/__tests__/Markdown.test.tsx

@@ -213,6 +213,27 @@ describe('<Markdown />', () => {
     expect(document.querySelectorAll('video').length).toEqual(1);
     expect(screen.queryByTitle(mockTitle)).not.toBeInTheDocument();
   });
+
+  it('Leaves <video> inside inline code alone when a real video follows', () => {
+    renderView({
+      text: 'The `<video>` element needs a closing tag.\n\n<video src="/example.webm" controls>\n  Video not supported\n</video>',
+      videoOverride: { component: MarkdownVideo },
+    });
+    expect(document.querySelector('code')?.textContent).toEqual('<video>');
+    screen.getByTitle(mockTitle);
+  });
+
+  it('Leaves <video> inside fenced code blocks alone', () => {
+    renderView({
+      text: '```html\n<video src="/example.webm"></video>\n```',
+      videoOverride: { component: MarkdownVideo },
+    });
+    expect(document.querySelector('pre code')?.textContent).toContain(
+      '<video src="/example.webm"></video>'
+    );
+    expect(screen.queryByTitle(mockTitle)).not.toBeInTheDocument();
+  });
+
   it('Wraps the markdown in a div by default (block)', () => {
     renderView({ text: basicMarkdown });
     expect(

packages/gamut/src/Markdown/index.tsx

@@ -12,6 +12,7 @@ import {
   createInputOverride,
   createTagOverride,
   createVideoOverride,
+  createVideoParagraphOverride,
   MarkdownOverrideSetting,
   MarkdownOverrideSettings,
   standardOverrides,
@@ -35,20 +36,6 @@ const preprocessingInstructions = createPreprocessingInstructions();
 
 const isValidNode = () => true;
 
-/**
- * Puts each <video> block on its own lines so marked emits an HTML block.
- * A single-line <video>...</video> is parsed as paragraph text, which wraps
- * the video's block-level output in a <p>.
- */
-const normalizeVideoBlocks = (text: string) =>
-  text.replace(
-    /(<video\b[^>]*>)([\s\S]*?)(<\/video>)/gi,
-    (_match, open: string, inner: string, close: string) => {
-      const body = inner.trim();
-      return `\n\n${open}\n${body ? `${body}\n` : ''}${close}\n\n`;
-    }
-  );
-
 const MarkdownWrapper = styled.div<{ spacing: 'loose' | 'tight' | 'none' }>`
   ${({ theme, spacing }) => {
     const spacingStyleFunction = markdownStyles[spacing];
@@ -144,6 +131,7 @@ export class Markdown extends PureComponent<MarkdownProps> {
           component: Table,
           allowedAttributes: ['style'],
         }),
+      videoOverride && createVideoParagraphOverride(),
       videoOverride &&
         createVideoOverride({ tagName: 'video', Override: videoOverride }),
       !skipDefaultOverrides.details &&
@@ -168,7 +156,7 @@ export class Markdown extends PureComponent<MarkdownProps> {
     // Render markdown to html
     const rawHtml = inline
       ? marked.parseInline(text, markedOptions)
-      : marked(normalizeVideoBlocks(text), markedOptions);
+      : marked(text, markedOptions);
 
     const sanitizationConfig = {
       ...defaultSanitizationConfig,

packages/gamut/src/Markdown/libs/overrides/index.tsx

@@ -4,7 +4,13 @@ import camelCaseMap from 'html-to-react/lib/camel-case-attribute-names';
 import get from 'lodash/get';
 import * as React from 'react';
 
-import { getLabel, isCheckboxParent, isInput, isLabelText } from './utils';
+import {
+  getLabel,
+  isCheckboxParent,
+  isInput,
+  isLabelText,
+  isVideoOnlyParagraph,
+} from './utils';
 
 const processNodeDefinitions = HtmlToReact.ProcessNodeDefinitions();
 
@@ -214,6 +220,20 @@ export const createVideoOverride = ({
   },
 });
 
+// Drops the <p> marked wraps around a single-line video, since the Video component renders block-level elements
+export const createVideoParagraphOverride = () => ({
+  shouldProcessNode(node: HTMLToReactNode) {
+    return isVideoOnlyParagraph(node);
+  },
+  processNode(
+    _node: HTMLToReactNode,
+    children: React.ReactNode[],
+    key: React.Key
+  ) {
+    return <React.Fragment key={key}>{children}</React.Fragment>;
+  },
+});
+
 // Allows <CodeBlock></CodeBlock> override and overrides of standard fenced codeblocks
 export const createCodeBlockOverride = (
   tagName: string,

packages/gamut/src/Markdown/libs/overrides/utils.tsx

@@ -10,6 +10,18 @@ export const isCheckboxParent = (node: HTMLToReactNode, type: string) =>
       node.children[0]?.attribs?.type === type
   );
 
+// a <p> that only holds video tags, i.e. a single-line <video></video> that marked parsed as paragraph text
+export const isVideoOnlyParagraph = (node: HTMLToReactNode) =>
+  Boolean(
+    node.name === 'p' &&
+      node.children?.some((child) => child.name === 'video') &&
+      node.children.every(
+        (child) =>
+          child.name === 'video' ||
+          (child.type === 'text' && !child.data.trim())
+      )
+  );
+
 export const isLabelText = (node: HTMLToReactNode, type: string) => {
   // we are using this text to create the accessible label, so are basically deduplicating it here
   return Boolean(

packages/gamut/src/Video/__tests__/Video.test.tsx

@@ -12,7 +12,17 @@ jest.mock('react-player', () => {
     __esModule: true,
     // eslint-disable-next-line react/display-name
     // Mimics react-player v3: the inner provider iframe has no title.
-    default: ({ src }: { src: string }) => {
+    default: ({
+      src,
+      light,
+      className,
+      wrapper,
+    }: {
+      src: string;
+      light?: string | boolean;
+      className?: string;
+      wrapper?: string;
+    }) => {
       const hostRef = react.useRef(null);
       // Insert the iframe after mount, without ever firing onReady, to prove
       // the title doesn't depend on that callback.
@@ -31,6 +41,19 @@ jest.mock('react-player', () => {
           shadowRoot.appendChild(iframe);
         });
       }, []);
+      /*
+        Mimics react-player v3: the splash only gets gamut's className (via a
+        wrapper element) when a `wrapper` is passed; otherwise it's a bare
+        child of the frame.
+      */
+      if (light) {
+        const preview = react.createElement('div', {
+          className: 'react-player__preview',
+        });
+        return wrapper
+          ? react.createElement(wrapper, { className }, preview)
+          : preview;
+      }
       if (mockRenderInShadowRoot) {
         return react.createElement('div', { ref: hostRef });
       }
@@ -94,6 +117,23 @@ describe('Video', () => {
     await view.findByTitle('Video player');
   });
 
+  it('fills the 16:9 box with the splash preview', async () => {
+    const { view } = renderView({
+      videoUrl: 'https://www.youtube.com/watch?v=Yl8yy5tpVIM',
+      placeholderImage: 'https://example.com/splash.jpg',
+    });
+
+    await waitFor(() => {
+      const preview = view.container.querySelector<HTMLElement>(
+        '.react-player__preview'
+      );
+      expect(preview).not.toBeNull();
+      expect(getComputedStyle(preview!.parentElement!).position).toBe(
+        'absolute'
+      );
+    });
+  });
+
   it('labels a provider iframe rendered inside a shadow root', async () => {
     mockRenderInShadowRoot = true;
     const { view } = renderView({

packages/gamut/src/Video/index.tsx

@@ -8,9 +8,12 @@ import {
 import * as React from 'react';
 import { useEffect, useRef, useState } from 'react';
 
-import { Box } from '../Box';
 import { useIsMounted } from '../utils';
-import { OverlayPlayButton, ReactVideoPlayer } from './lib/ReactPlayer';
+import {
+  OverlayPlayButton,
+  ReactVideoPlayer,
+  VideoFrame,
+} from './lib/ReactPlayer';
 import { VidstackPlayer } from './lib/VidstackPlayer';
 
 export type VideoProps = {
@@ -169,13 +172,12 @@ export const Video: React.FC<VideoProps> = (props) => {
     showPlayerEmbed
   ) {
     return (
-      <Box
+      <VideoFrame
         bg={loading ? 'black' : undefined}
         borderRadius="md"
         className={className}
         overflow="hidden"
         position="relative"
-        pt={'56.25%' as any}
         ref={playerWrapperRef}
         width="100%"
       >
@@ -192,6 +194,9 @@ export const Video: React.FC<VideoProps> = (props) => {
             src={videoUrl as string}
             title={videoTitle}
             width="100%"
+            // Without a wrapper, react-player v3 drops `className` and the
+            // 100% size from its splash, so it no longer fills the frame.
+            wrapper="div"
             onPlay={onPlay}
             onReady={() => {
               labelProviderIframe();
@@ -200,7 +205,7 @@ export const Video: React.FC<VideoProps> = (props) => {
             }}
           />
         ) : null}
-      </Box>
+      </VideoFrame>
     );
   }
 

packages/gamut/src/Video/lib/ReactPlayer.tsx

@@ -3,7 +3,18 @@ import { css, theme } from '@codecademy/gamut-styles';
 import styled from '@emotion/styled';
 import ReactPlayer from 'react-player';
 
-import { FlexBox } from '../../Box';
+import { Box, FlexBox } from '../../Box';
+
+/*
+  16:9 by default, but unlike a padding-top ratio box, a height set by the
+  consumer (e.g. `height: 100%` via `className`) wins, so the player and the
+  provider's own splash fit the consumer's frame instead of overflowing it.
+*/
+export const VideoFrame = styled(Box)(
+  css({
+    aspectRatio: '16 / 9',
+  })
+);
 
 export const ReactVideoPlayer = styled(ReactPlayer)(
   css({

packages/styleguide/.storybook/main.ts

@@ -65,6 +65,7 @@ const config: StorybookConfig = {
           '../../gamut-styles/src'
         ),
         '@codecademy/gamut$': resolve(__dirname, '../../gamut/src'),
+        '@codecademy/gamut/Video$': resolve(__dirname, '../../gamut/src/Video'),
         '@codecademy/gamut-illustrations$': resolve(
           __dirname,
           '../../gamut-illustrations/src'