🐛 [amp-vimeo] Added special case handler for Vimeo's new h param #37886
Conversation
|
|
| ); | ||
| let identifier = encodeURIComponent(videoid); | ||
|
|
||
| const paramParts = videoid?.split('?h=') || []; |
There was a problem hiding this comment.
I wonder if we should make this handle more url params other then just h. I think it'd be possible to generalize this to handle any url param.
There was a problem hiding this comment.
Along these lines, I'm not certain that this should be part of the videoid param. The partial parsing scheme here is a bit awkward.
@cahirodoherty-learningpool do you have an example of where this value comes from? Is it pasted from a URL, or does it come from an "embed code" modal in the Vimeo UI? This would be useful to know so that we can map the interface properly.
There was a problem hiding this comment.
Hi @alanorozco, this is indeed a url from an embed code (provided by a client).
There was a problem hiding this comment.
In what context is the URL acquired? Do you have an example or two?
There was a problem hiding this comment.
@dethstrobe I agree, the single parameter splitting here is a little strange, especially as it is referred to as 'videoid' incoming.
@alanorozco The use case was a client dropping an embed url of a private Vimeo video into our app, which parses it for the url. Upon investigation, it seems the 'share link' url to the video will work, so perhaps our app can swap out one for the other instead.
I still think url param handling may be necessary for the amp-vimeo component in some capacity, as Vimeo allows the ability to pass some things like '?quality=720p'. In its current form, this PR is not currently fit for that purpose. Happy to take direction on this issue.
|
Hi guys, any further thoughts on this issue? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
|
bump to keep PR from auto-close |
Fixes #37618
Vimeo have added a h param to new videos, which will break in iFrame handling due to encodeURIComponent