-
-
Notifications
You must be signed in to change notification settings - Fork 641
[6.x] Look up video providers via oEmbed #15460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
edalzell
wants to merge
14
commits into
statamic:6.x
Choose a base branch
from
edalzell:feature/video-oembed-lookup
base: 6.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
aed6a73
Augment video fields to a value object carrying the provider
edalzell c44253e
Accept null in the video embed url helpers
edalzell 862f032
Base video embed truthiness on the value, not the provider
edalzell f17b781
Pass unsupported video urls through the embed url modifier
edalzell 3745c70
Keep trackable embed urls trackable for augmented videos
edalzell 9240993
Treat video files as supported but not embeddable
edalzell 83df089
Detect oembed providers before falling back to video files
edalzell dc360f7
Keep video fields serializing to a string in the api
edalzell 56acf47
Assert augmented video fields behave like the raw value
edalzell cdbcc04
Update video embed tests for the corrected contract
edalzell 81c399a
Measure the string length of stringable value objects
edalzell b752d12
Add Cloudflare Stream support to the video fieldtype
edalzell 3102928
Look up video providers via oEmbed
edalzell 4700b23
Keep our own youtube and vimeo embed urls over oembed
edalzell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^2.0resolves as low as 2.0.0, and the offline-resolution behaviour this feature is built on doesn't exist down there.Embed::fromOembed()leans on Embera'sgetFakeResponse()rules viaEmbera::ONLY_FAKE_RESPONSES. I checked that method across the tags:Embera\Provider\Vimeodoesn't have it in 2.0.0, 2.0.5, 2.0.10, 2.0.15, 2.0.20, 2.0.25 or 2.0.30 — it first appears somewhere between 2.0.30 and 2.0.32.Youtubeis the same. The constant itself exists all the way back to 2.0.0, so nothing throws; the fake-response path just quietly resolves nothing and falls through to a live request.That's what
P8.3 - L12.* - prefer-lowest - shard 3/4is tripping over:It's not only a test-environment problem, though. The PR description's core argument is that YouTube, Vimeo, DailyMotion, Loom, Streamable, Rumble, Coub and Ted cost zero HTTP requests — but that's only true on the newer end of what
^2.0permits. A site whose lockfile lands on an older 2.0.x pays a network round-trip on every YouTube and Vimeo embed, which is exactly the cost the dependency was justified as avoiding.Worth confirming the floor per-provider rather than taking 2.0.32 from me — I only bisected Vimeo and Youtube, and all eight of the "offline" providers need
getFakeResponse()at whatever minimum you settle on.