Skip to content

[6.x] Augment video fields to a value object carrying the provider - #15458

Open
edalzell wants to merge 1 commit into
statamic:6.xfrom
edalzell:feature/video-value-object
Open

edalzell wants to merge 1 commit into
statamic:6.xfrom
edalzell:feature/video-value-object

Conversation

@edalzell

Copy link
Copy Markdown
Contributor

The video fieldtype doesn't implement augment(), so the raw string is all a template ever gets. There's no provider discriminator, and no first-class way to get an embed URL — {{ video_field | embed_url }} works only because CoreModifiers::embedUrl() happens to know how to rewrite YouTube and Vimeo URLs by hand.

This augments to a Statamic\Fieldtypes\Video\Video value object exposing provider, id, url and embed_url, following the ArrayableString/ArrayableLink convention: __toString() returns the original value, so {{ video_field }} is unchanged, and it implements Arrayable, ArrayAccess, Boolable and JsonSerializable. It recognises YouTube, Vimeo, direct video files (via FileTypes::video()) and cloudflare:<id> values, falling back to an unsupported provider.

The YouTube/Vimeo URL→embed logic is moved out of CoreModifiers::embedUrl() onto the value object, and the modifier now delegates to it — one implementation rather than two. embedUrl, trackableEmbedUrl and isEmbeddable also accept the object directly. The existing modifier tests pass unchanged, which is the regression proof for the move.

Worth a look: augment() returns the object for all values, including plain URLs. __toString() covers direct output, modifiers and GraphQL (the existing GraphQL test still passes unchanged), but a strict is_string() check downstream would now see an object. Happy to keep plain URLs as strings and only return the object for cloudflare: values if you'd rather not change that surface in a major.

Split out of #11871 so the augmentation contract can be settled on its own, ahead of the Cloudflare Stream and oEmbed work. No new dependencies.

@edalzell
edalzell force-pushed the feature/video-value-object branch from 62b7e0b to ca149d0 Compare September 14, 2026 23:10

@jasonvarga jasonvarga left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice direction — a provider discriminator and a first-class embed_url is the right shape, and {{ video:provider }} / {{ video:embed_url }} work well. The move of the URL rewriting onto the value object is a good de-duplication too.

The problem is that augment() now returns the object for every value, and several modifier and template paths behave differently for an actual video field than they do for the equivalent raw string. The new tests only ever exercise the string path, so CI is green while the field itself regresses.

I verified this by running a probe against this branch, then reverting the three source files to the merge base (ffbecaa9) and running the identical probe:

Template 6.x This PR
{{ empty_video | embed_url }} null uncaught TypeError
{{ empty_video | is_embeddable }} false uncaught TypeError
{{ if video }} where value is https://example.com/nope TRUE FALSE
{{ video | embed_url }}, unsupported provider https://example.com/nope ''
{{ video | trackable_embed_url }}, YouTube https://www.youtube.com/embed/… https://www.youtube-nocookie.com/embed/…
{{ video | trackable_embed_url }}, Vimeo https://player.vimeo.com/video/… …?dnt=1
{{ video | embed_url }}, Vimeo progressive_redirect .mp4 …?dnt=1&loc=… …?loc=…, no dnt
{{ video | is_embeddable }}, direct .mp4 false true

{{ video }}, {{ if video == "…" }} and the GraphQL output are all unchanged, so the __toString strategy is holding up — it's the modifier short-circuits and toBool() that need another look. Details inline.

One thing not covered inline: Value::jsonSerialize() returns the Embed, so a video field in the REST API changes from "https://vimeo.com/123" to {"embed_url":…,"id":…,"provider":…,"url":…}. There's precedent for that (ArrayableString, ArrayableLink both do it) and a major is the right place for it, but it'll need an upgrade guide entry since the description only discusses the template surface.

On the open question in the description — returning the object only for cloudflare: values wouldn't help much. The null, unsupported and file paths below are the sharp edges, and they'd all still be there.

Also, the description refers to the value object as Statamic\Fieldtypes\Video\Video; it's …\Video\Embed.

/**
* Turn a link that's direct to a video's page into its embeddable equivalent.
*/
public static function embedUrl(string $url): string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

string $url here (and on isEmbeddable() below) makes the modifiers fatal on a null value.

An empty video field augments to null, so {{ video | embed_url }} on a blank field throws:

TypeError: Statamic\Fieldtypes\Video\Embed::embedUrl(): Argument #1 ($url) must be of type string, null given

This one isn't recoverable: Modify::modify() catches Exception, and TypeError is an Error, so it escapes as a 500 rather than a ModifierException. On 6.x both calls returned quietly (null and false respectively).

It isn't limited to video fields either — {{ some_null_var | embed_url }} was previously a harmless no-op and now fatals too.

Making both params ?string with an early return, or guarding in the modifier before delegating, covers it.


public function toBool(): bool
{
return $this->isSupported();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This flips {{ if video }} to false for any URL the class doesn't recognise.

Boolable is honoured by Antlers conditions (PathDataManager.php:969, Environment.php:358), so a field holding a perfectly valid stored value that isn't YouTube/Vimeo/a known file extension/cloudflare: — a self-hosted player page, a Loom or Wistia link, an unusual extension — goes from truthy to falsey. {{ if video }}<iframe …>{{ /if }} silently stops rendering on upgrade, with nothing in the logs.

That's the one I'd most want changed, because it's invisible.

ArrayableString::toBool() — the convention the description cites — is (bool) $this->value, i.e. "is there a value", not "do I recognise it". Matching that and leaving recognition to isSupported() / {{ video:provider }} keeps both questions answerable.

: static::unsupported($value);
}

if (static::isVideoFile($value)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isVideoFile() running before oembedProvider() misclassifies Vimeo's direct-playback URLs.

They end in .mp4, so they land on provider: file with embedUrl set to the untouched URL — which means the augmented value silently loses ?dnt=1.

This isn't hypothetical: EmbedUrlTest::it_transforms_vimeo_file_links asserts ?dnt=1 gets added to exactly this URL shape, and vimeoEmbedUrl() below carries a dedicated progressive_redirect guard for it. That test still passes here only because it feeds a raw string.

Swapping the order so oembedProvider() is checked first fixes it.

}

#[\ReturnTypeWillChange]
public function jsonSerialize()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Worth flagging explicitly since it's easy to miss from the diff: because Value::jsonSerialize() hands the Embed straight through, this changes a video field in the REST API from "https://vimeo.com/123" to {"embed_url":…,"id":…,"provider":…,"url":…}.

Consistent with ArrayableString/ArrayableLink, and a major is the right time — it just needs to be a deliberate, documented change rather than a side effect. Either an upgrade guide entry, or return (string) $this here and keep the structured form to toArray().


return $url;
if ($url instanceof Embed) {
return $url->embedUrl;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

embedUrl is null for the unsupported provider, so this returns empty where the modifier used to pass the URL through untouched.

EmbedUrlTest::it_leaves_urls_from_unknown_providers_untouched asserts that contract, and passes here only because it feeds a raw string rather than an augmented value.

Suggested change
return $url->embedUrl;
return $url->embedUrl ?? $url->url;

Comment on lines +3219 to +3221
if ($url instanceof Embed) {
return $url->embedUrl;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This makes trackable_embed_url return the non-trackable URL.

$url->embedUrl is the privacy-preserving form — youtube-nocookie.com, and Vimeo with ?dnt=1 — which is precisely what this modifier exists to avoid. The implementation directly below deliberately keeps youtube.com and adds no dnt, so for any augmented field the modifier now does the opposite of its name:

YouTube: https://www.youtube.com/embed/…  →  https://www.youtube-nocookie.com/embed/…
Vimeo:   https://player.vimeo.com/video/… →  https://player.vimeo.com/video/…?dnt=1

Running the existing logic below against $url->url instead of short-circuiting gives the right result.

Comment on lines +3255 to +3257
if ($url instanceof Embed) {
return $url->isSupported();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isSupported() is true for provider: file, so is_embeddable now returns true for direct video files that previously returned false.

The practical effect is that {{ if video | is_embeddable }}<iframe src="{{ video | embed_url }}"> starts emitting an iframe pointing at a raw .mp4 for self-hosted files.

"Supported" and "embeddable" are different questions — file is the former but not the latter. Checking the provider against the embeddable set, or adding an isEmbeddable() to the value object distinct from isSupported(), keeps them apart.

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.

2 participants