Added support for preview of Facebook video #6105

[ElectroluxV2]

📝 Provide a description of the improvement

Now links like https://www.facebook.com/realitetiofficial/videos/458353101779653/ or https://www.facebook.com/watch/?v=458353101779653 will have preview.

TODO (can't do this myself)

• Fix portrait videos shrink.

📃 Details

• Browser: not applicable
• OS: not applicable
• CKEditor version: not applicable
• Installed CKEditor plugins: MediaEmbed v16.0.0

---

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

[coveralls]

Pull Request Test Coverage Report for Build 389

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 381:	0.0%
Covered Lines:	311
Relevant Lines:	311

---

💛 - Coveralls

[mlewand]

Hey, thanks for the PR. First of all I see there's a unit tests coverage drop, please make sure to add proper unit tests for the new code.

Also could you provide more information on this problem:

Fix portrait videos shrink.

Finally I think you have added allow="autoplay" I believe this is not something that should be enabled by default.

[ElectroluxV2]

Hey, thanks for the PR. First of all I see there's a unit tests coverage drop, please make sure to add proper unit tests for the new code.

It's my very first time both using github and contributing, sorry in advance for messing up.

Also could you provide more information on this problem:

Fix portrait videos shrink.

Yes, square and horizontal videos looks like this:
Title of video visible etc.

And the vertical videos looks like this:
Cropped in some about half
Had tried fixing this by css, but none of results were better than just cropped preview.
I think You can move the content of iframe to negative top (in order to show title and etc) it would be the best choice, but im failure at this point making this fix.

Finally I think you have added allow="autoplay" I believe this is not something that should be enabled by default.

Yes you right, tbh I had copied vimeo version of template which contains this too.

[ElectroluxV2]

OK, I give up, what is wrong with my test?

[mlewand]

If you look closely at assertion error message, you'll see the answer (I changed changed the formatting in pasted content to make it more readable).

AssertionError: assertion for "https://www.facebook.com/xyz/videos/1234": expected
	'<figure class="ck-widget media" contenteditable="false"><div class="ck-media__wrapper" data-oembed-url="https://www.facebook.com/xyz/videos/1234"><div style="height:0;padding-bottom:100%;position:relative"><iframe allow="encrypted-media" frameborder="0" src="https://www.facebook.com/plugins/video.php?show_text=false&href=https://www.facebook.com/watch/?v=1234" style="height:100%;left:0;position:absolute;top:0;width:100%"></iframe></div></div></figure>'
	to match
	/<figure[^>]+><div[^>]+><div style="height:0;padding-bottom:100%;position:relative"><iframe allow="encrypted-media" frameborder="0" src="https:\
/\/www.facebook.com\/plugins\/video.php?show_text=false&href=https:\/\/www.facebook.com\/watch\/?v=1234" style="height:100%;left:0;position:absolute;top:0;width:100%"><\/iframe><\/div><\/div><\/figure>/

Based on this you can say that the regexp does not match. To se what exactly is the problem I recommend to use tools like https://regexr.com/ (or any other, there are many other great regexp tools) to put the entire regexp and tested content there. At first it won't match anything - that's ok because this is what's happening.

What you want to do from there is remove parts of regexp, piece by piece until it starts matching - this will tell you where exactly the problem is. It's basically as if you'd do manual a git bisect, just for regexp.

I believe the way we're making a dynamic runtime regexp doesn't take into accounts some special characters, and you'd need to correct that.

The problem with cropped videos is a little bigger that I initially thought, as this bottom part do contain valuable information that should not be missing. So we need to have this sorted out before merging the PR.

[mlewand]

And @ElectroluxV2 since you mentioned you're fresh to GH - you can paste images directly into the GH comments - that way others will not have to leave it to see your screens, etc.