Problem/Motivation

The regex for the Vimeo provider is very strict and doesn't allow inputs containing query strings. We've had users try and input Vimeo URLs like this and it won't work because of the query string:

https://vimeo.com/123123123?fl=pl&fe=vl

I'm not sure what those params are for, but Vimeo provider refuses to parse the ID.

#3238136: Unlisted Vimeo videos don't work because they are missing the required hash parameter mentions this as a problem too, but that issue is adding a feature as well. This is just the bug fix.

Steps to reproduce

Try and add a Vimeo video using a URL like above, you'll get a validation error.

Proposed resolution

Minor change to the regex that validates a Vimeo URL to remove the $ at the end.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork video_embed_field-3569488

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bkosborne created an issue. See original summary.

bkosborne opened merge request !108

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
commented
Status: Active » Needs review

MR submitted. I noticed the unit tests for ProviderUrlParseTest were already failing, so I fixed those here too. Note that as a result of relaxing the regex, it now allows through some previously invalid inputs for time index like this:

https://vimeo.com/193517656#t= (notice missing time)

But that seems totally fine. The time is still not parsed. We just accept the URL and still parse the vimeo ID.

mably’s picture

mably commented

Hi @bkosborne, thanks for your MR.

Looks like it needs a rebase though.

mably’s picture

mably commented
Status: Needs review » Needs work

bkosborne changed the visibility of the branch 3569488-vimeo-providers-regex to hidden.

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
commented
Status: Needs work » Needs review

I'm very confused with the branches here. There is both a 3.x and a 3.0.x branch. The 3.0.x branch is behind 3.x but 3.0.x is the default branch that was chosen when I created the issue fork. Which branch is "main"?

In any case, I created a new branch from 3.x with the changes.

bkosborne opened merge request !109

mably’s picture

mably commented

@bkosborne, I updated your MR to simply update the regexp to allow any query string before the optional timestamp fragment.

It does not require to disable the existing tests.

Is it ok for you?

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
commented

Yes that's fine with me. Thank you!

  • mably committed 36e71380 on 3.x authored by bkosborne 
    fix: #3569488 Vimeo provider's regex doesn't allow query strings

By:...
mably’s picture

mably commented
Status: Needs review » Fixed

Great, thanks! Merged.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.