Fix: render podcast show descriptions as HTML instead of raw markup - #3721
Fix: render podcast show descriptions as HTML instead of raw markup#3721daniellefrappier18 wants to merge 3 commits into
Conversation
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
There was a problem hiding this comment.
Pull request overview
This PR updates the Next.js podcast UI to render podcast show descriptions as sanitized HTML (instead of plain text), matching the existing behavior for episode descriptions and fixing cases where users saw raw tags/entities.
Changes:
- Render podcast show descriptions using
dangerouslySetInnerHTMLwith DOMPurify sanitization and appropriate link handling (open external links in a new tab on the detail page; strip nested links in featured cards). - Update typography wrappers to render as
divto avoid invalid<p>nesting when descriptions contain block elements. - Add/extend frontend tests to cover XSS sanitization, entity decoding, and link behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontends/main/src/app-pages/PodcastPage/PodcastsListingPage/PodcastSection.tsx | Render featured show summaries as sanitized HTML and strip nested anchors; adjust styling for clamped summaries containing <p> tags. |
| frontends/main/src/app-pages/PodcastPage/PodcastsListingPage/PodcastSection.test.tsx | Add tests for sanitization/entity rendering and link stripping in featured cards. |
| frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.tsx | Render show description as sanitized HTML, add link styling, and open external links in a new tab. |
| frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.test.tsx | Add tests for show description sanitization/entity decoding and external-link target behavior. |
Suppressed comments (1)
frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.test.tsx:240
- After updating the fixture HTML to include the backend-provided
relattribute, the test should also assert it on the external link. This guards the assumption that links rendered withtarget="_blank"are protected against tabnabbing (noopener/noreferrer).
const externalLink = await screen.findByRole("link", { name: "OCW" })
expect(externalLink).toHaveAttribute("target", "_blank")
| podcastOverrides: { | ||
| description: | ||
| 'Relevant Resources: <a href="https://ocw.mit.edu/">OCW</a> and <a href="/search">Search</a>.', | ||
| }, |
There was a problem hiding this comment.
addExternalLinkTargets only reads href and appends target, it doesn't read, use, or need rel at all, and it passes through whatever other attributes are already in the tag unchanged. Adding rel="noopener noreferrer" to the test fixture wouldn't change what's asserted or add any new coverage; it'd just make the mock string longer to look more like real nh3 output.
| component="div" | ||
| dangerouslySetInnerHTML={{ | ||
| __html: addExternalLinkTargets( | ||
| DOMPurify.sanitize(resource.description), |
There was a problem hiding this comment.
Question:
The sibling PodcastEpisodeDetailPage.tsx deliberately renders its already-backend-sanitized (nh3) description without a client-side DOMPurify.sanitize, and memoizes the transform (useMemo, lines 274–278), with a comment (lines 267–273) explaining this keeps SSR and client output identical to avoid a hydration mismatch. Here we re-sanitize inline on every render via isomorphic-dompurify, which runs against jsdom on the server and the native DOM in the browser — the two can produce subtly different HTML, and feeding that to dangerouslySetInnerHTML during SSR is a known cause of React hydration mismatches. Since the content is already backend-sanitized, consider dropping the redundant DOMPurify.sanitize (and/or memoizing) to mirror PodcastEpisodeDetailPage.tsx.
There was a problem hiding this comment.
Good catch, you're right. I pulled it out and memoized it the same way PodcastEpisodeDetailPage does. Also had to tweak one test that was asserting a <script> tag got stripped client-side.
ahtesham-quraish
left a comment
There was a problem hiding this comment.
Overall looks good just left one comment
What are the relevant tickets?
Fixes https://github.com/mitodl/hq/issues/12690
Description (What does it do?)
The podcast detail page and the "Featured" cards on the podcasts listing page, the show-level
descriptionwas rendered as plain JSX text, so users saw raw markup like<p>tags and unescaped entities like&instead of formatted copy.The backend sanitizes show descriptions as allowlisted HTML (
nh3viaALLOWED_HTML_TAGS_WITH_LINKS) — the same treatment already given to podcast episode descriptions, which render correctly. The two show-level sites just hadn't been updated to match.PodcastDetailPage.tsx— description now sanitized (DOMPurify.sanitize) and rendered viadangerouslySetInnerHTML, with external links opening in a new tab (addExternalLinkTargets), mirroringPodcastEpisodeDetailPage.tsx.PodcastSection.tsx(featured podcast cards) — same sanitization, but links are stripped (stripAnchorTags) instead of kept clickable, since the whole card is already aLink— mirroringEpisodeItem.tsx's handling of the same situation.<p>tocomponent="div", since sanitized descriptions can contain their own<p>tags (nesting<p>in<p>is invalid HTML).a { textDecoration: underline; ... }) to the show detail page's description, matching the episode detail page, so links don't fall back to default browser blue.& poverride on the featured card's summary so paragraph tags don't break its 2-line clamp truncation.Screenshots (if appropriate):
BEFORE


AFTER

How can this be tested?
/podcast/15925/trash-talking— "&" should render properly instead of&/podcast/17779/lock-the-quill— paragraphs should render as formatted text instead of showing literal<p>tags<a>nested inside the card.Additional Context
Out of scope: a related issue: https://github.com/mitodl/hq/issues/12700 where sanitized HTML/entities leak into
<meta name="description">/ Open Graph / Twitter Card tags on the podcast (and other resource-type) pages — that needs a different fix (strip-to-plain-text, not parse-as-HTML) and touches a shared metadata utility used by all resource types, so it's being tracked separately.