colonel-kurtz icon indicating copy to clipboard operation
colonel-kurtz copied to clipboard

YouTube regex might need updating

Open averyvery opened this issue 9 years ago • 5 comments

Current YouTube regex is new RegExp('(?:youtube(?:-nocookie)?\.com/(?:[^/]+/.+/|(?:v|e(?:mbed)?)/|.*[?&]v=)|youtu\.be/)([^"&?/ ]{11})', 'i')

...but I think there's a mistake in there. Escaped characters, since they're in a string, should be double-escaped like so:

new RegExp('(?:youtube(?:-nocookie)?\\.com/(?:[^/]+/.+/|(?:v|e(?:mbed)?)/|.*[?&]v=)|youtu\\.be/)([^"&?/ ]{11})', 'i')

Example: http://jsfiddle.net/averyvery/xwbt5u3L/

Not a big deal, though; I'm sure this doesn't break any existing functionality.

averyvery avatar Oct 05 '15 18:10 averyvery

Cool. Seems like it would pull out the video id regardless.

The test coverage for this is here: https://github.com/vigetlabs/colonel-kurtz/blob/master/addons/youtube/tests/youtube.test.jsx#L30-L59

I can't get to it right away, if you want to check it out now. Otherwise I can totally dig into this tomorrow or wednesday

nhunzaker avatar Oct 05 '15 18:10 nhunzaker

I don't see any reason to take care of it any time soon.

And yeah, it'll pull out the id either way; I can't think of a real case where this would cause a full failure. I was just copying a bit of it for my Vimeo module and realized the dots chunk weren't working as expected.

averyvery avatar Oct 05 '15 18:10 averyvery

Haha. it would be good to have some sort of error that checks the domain and says something like "The URL provided is not from YouTube..."

I'm less worried about the actual URL and more that it properly gets the video they want. We don't save the URL at all -- just the video ID

nhunzaker avatar Oct 05 '15 18:10 nhunzaker

Yeah, I see how it works — nbd, sorry if this is just wasting time. I was hoping it would be a little todo to be taken care of at some far future date.

averyvery avatar Oct 05 '15 18:10 averyvery

No problem at all! Talk is cheap. This would be a nice enhancement

nhunzaker avatar Oct 05 '15 18:10 nhunzaker