Add looping videos to articles #14843

RikRootsGuardian · 2025-11-14T14:55:21Z

What does this change?

TODO: write up!

Why?

Screenshots

Before	After

github-actions · 2025-11-14T14:55:31Z

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

github-actions · 2025-11-14T15:31:42Z

Deploy build 46882 of `Mobile::mobile-apps-rendering` to CODE

All deployment options

From guardian/actions-riff-raff.

RikRootsGuardian · 2025-11-17T11:04:59Z

(AI suggestions sent me down the wrong path)

RikRootsGuardian · 2025-11-17T15:02:05Z

This appears to be a CORS issue rather than a code issue. When developing locally we test on http://localhost:3030, which leads to CORS issues for loading the video from eg https://uploads.guim.co.uk/2025/10/07/Metaxas_tests_audience_with_first_jokes_of_Talk_show___video--a182c225-e45f-4d6a-82ed-05fa50e17fbb-2.0.mp4#t=0.001 ...:

The white space appears when the poster image fails to upload for some reason? This is an intermittent issue (because: see screen shot above which loaded the poster image fine 2 mins after failing and 2 mins before failing again):

I noticed this comment in the last PR to touch the LoopVideoPlayer file:

RikRootsGuardian · 2025-11-17T15:08:16Z

Can confirm that the video src/url strings are correct (pasting the url into the browser address bar loads the video)

RikRootsGuardian · 2025-11-17T15:14:12Z

Viewing the article locally, using http://r.thegulocal.com:3030/Article/https://www.theguardian.com/media/2025/oct/08/conservative-late-night-talkshow URL. doesn't solve the issue:

RikRootsGuardian · 2025-11-18T15:46:04Z

tl;dr: Looping videos now need to be served with a crossOrigin="anonymous" header (so subtitle files can be picked up) - failure will lead to the CORS issue. This is a fixed issue, except for this PR branch where we're testing on articles with self-hosted videos which are not looping videos, thus have not had the header added and old copies have not been purged from Fastly.

Chat messages: AnnaB, MarjanK

Anna:
Hey WebEx - we're running into a CORS issue and I wondered if you might have some advice.

We introduced (and rolled back) a crossOrigin="anonymous" header to looping videos so that we can retrieve the subtitle vtt file from uploads.guim. This change meant that both the video and the subtitle files are now requested as cross-origin CORS fetches, requiring fastly to include Access-Control-Allow-Origin headers.

Unsuprisingly, uploads.guim already has https://www.theguardian.com as an allowed origin but some users were getting the following error

Access to video at 'https://uploads.guim.co.uk/2025/11/05/Front_loop__Mamdani_victory_speech--4477e94a-a4a8-4a85-96c2-4ee8cff259f9-2.0.mp4#t=0.001' from origin 'https://www.theguardian.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

Does anyone have any ideas as to why some users would be hitting this error?

I'm wondering if some users could be getting a cached mp4 response that does not have the headers attached because we did not ask for them before this change was rolled out and if we therefore need to purge the cache for some videos. Does that sound plausible? Are we able to purge certain items in the uploads.guim cache?

The fastly VCL is available here if anyone is interested in looking through it https://manage.fastly.com/configure/services/2TmfkSoyUoNo8aFNe6Htjs/versions/29/vcl/snippets/CORS%20headers

Marjan
Hi Anna, that is what I was thinking too, that we purge the cache for those videos. I think for uploads.guim.co.uk we'd need to do the purge manually in fastly by putting the whole url in Purge URL (for content & images we have this feature in the admin tool but I can't see anything there for uploads.guim)

[...etc]

github-actions · 2025-11-19T16:54:56Z

Deploy build 22626 of `dotcom:rendering-all` to CODE

All deployment options

From guardian/actions-riff-raff.

simonbyford · 2025-11-20T09:33:54Z

dotcom-rendering/src/frontend/schemas/feArticle.json

+                "elementId",
+                "id"
+            ]
+        },


Do we need to introduce LoopVideoInArticleElement? I think we should just be able to extend the existing MediaAtomBlockElement with the new attribute. Same story in renderElement.tsx, block-schema.json, content.ts.

Let me know if I've missed something!

I honestly don't know. I don't understand the purpose of these json files, though I expect they may be something to do with testing?

I defined it in renderElement.tsx because loop videos have a radically different structure to legacy self-hosted videos and components that display on a page all seem to get defined here.

Removed (alongside the same change in a different json file)

simonbyford · 2025-11-20T09:41:52Z

dotcom-rendering/src/types/content.ts

-interface VideoAssets {
-	url: string;
+export interface VideoAssets {
+	url?: string;


Why is url optional?

My fault - will attempt removal

simonbyford · 2025-11-20T09:59:30Z

dotcom-rendering/src/types/content.ts

+	subtitleSize?: string;
+	subtitleSource?: string;


These attributes (subtitleSize and subtitleSource) don't exist on the element so will always be missing.

subtitleSize can probably be hardcoded in renderElement.tsx?

subtitleSource will require some thought. Might need to add it to the frontend PR..

See this comment about computing subtitleSource.

I believe subtitleSize and subtitleSource can safely be removed here.

simonbyford · 2025-11-20T10:18:20Z

dotcom-rendering/src/components/LoopVideoInArticle.importable.tsx

+const convertSubtitleSize = (val?: string): SubtitleSize => {
+	if (val == null) {
+		return 'small' as SubtitleSize;
+	}
+	if (['small', 'medium', 'large'].includes(val)) {
+		return val as SubtitleSize;
+	}
+	return 'small' as SubtitleSize;
+};


Hopefully this isn't needed, we can just hardcode to an appropriate size (medium?)

I really hope so too. But newer loop videos with subtitles includes this data. I introduced this horrid little function to kill off a bunch of TypeScript errors. There must be a better solution!

simonbyford · 2025-11-20T10:42:41Z

dotcom-rendering/src/components/LoopVideo.importable.tsx

 	const [hasBeenPlayed, setHasBeenPlayed] = useState(false);
 	const [hasTrackedPlay, setHasTrackedPlay] = useState(false);

+	const [devicePixelRatio, setDevicePixelRatio] = useState(1);


Introducing state feels a bit unnecessary here, what was your thinking?

This is a bug in the new LoopVideo player. The bug tries to access the browser window environment for the DPR value before the element has been mounted. So I decided to move the value to state and do the window check in a useUpdate hook. Thus once we get the correct DPR the component will rectify itself.

simonbyford · 2025-11-20T10:57:02Z

dotcom-rendering/src/lib/renderElement.tsx

+								<LoopVideoInArticle
+									assets={element.assets}
+									atomId={element.id}
+									uniqueId={`${Math.random()}`}


I wonder if we could we use element.id or element.elementId here instead of using a random number?

Good call. Will update

simonbyford · 2025-11-20T10:59:22Z

dotcom-rendering/src/lib/renderElement.tsx

+									height={400}
+									width={500}


Do these numbers come from somewhere?

Magic numbers: currently represents the 5:4 aspect ratio used by loop videos. Could double to 1000/800 maybe for people using very wide displays? I think CSS overrides this in any case - need to check.

simonbyford · 2025-11-20T16:58:17Z

dotcom-rendering/src/lib/renderElement.tsx

+									caption={element.title}
+									isMainMedia={isMainMedia}
+									subtitleSize={element.subtitleSize}
+									subtitleSource={element.subtitleSource}


We should be able to grab the relevant asset with something like this:

const getSubtitleAsset = (assets: VideoAssets[]) => { // Get the first subtitle asset from assets with a mimetype of 'text/vtt' return assets.find((asset) => asset.mimeType === 'text/vtt'); }

Then:

Suggested change

subtitleSource={element.subtitleSource}

subtitleSource={getSubtitleAsset(assets)?.url}

RikRootsGuardian added 4 commits November 14, 2025 11:29

Looping video in article - initial work underway

abd631d

Merge branch 'main' into rjr-investigate-looping-video-in-articles

2940598

Looping video in article - initial work underway

54b2612

Looping video in article - initial work underway

5e3caba

RikRootsGuardian marked this pull request as draft November 14, 2025 14:55

github-actions bot added the dotcom-rendering label Nov 14, 2025

RikRootsGuardian added 2 commits November 14, 2025 15:27

Looping video in article - initial work underway

b88200a

Looping video in article - initial work underway

701f590

remove cosole.log

672c50b

RikRootsGuardian added 2 commits November 17, 2025 15:04

Investigate why loop video not displaying in article

aad6e2d

Merge branch 'main' into rjr-investigate-looping-video-in-articles

760c0aa

RikRootsGuardian added 4 commits November 18, 2025 15:53

Remove console.log

3d2050a

Reconcile with main branch code

3e683c9

Update loop test to use videoPlayerFormat attribute

3af9a68

Fix linting etc

503658c

simonbyford reviewed Nov 20, 2025

View reviewed changes

RikRootsGuardian added 2 commits November 21, 2025 11:37

Fixing remaining issues

aa2c34c

Remove commented out lines

41b8cff

	subtitleSource={element.subtitleSource}
	subtitleSource={getSubtitleAsset(assets)?.url}

Add looping videos to articles #14843

Are you sure you want to change the base?

Add looping videos to articles #14843

Uh oh!

Conversation

RikRootsGuardian commented Nov 14, 2025

What does this change?

Why?

Screenshots

Uh oh!

github-actions bot commented Nov 14, 2025

Uh oh!

github-actions bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikRootsGuardian commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikRootsGuardian commented Nov 17, 2025

Uh oh!

RikRootsGuardian commented Nov 17, 2025

Uh oh!

RikRootsGuardian commented Nov 17, 2025

Uh oh!

RikRootsGuardian commented Nov 18, 2025

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Nov 14, 2025 •

edited

Loading

RikRootsGuardian commented Nov 17, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading