Skip to content
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

FEATURE: Whitelist the `allowfullscreen` iframe attribute #8086

Merged
merged 1 commit into from Sep 11, 2019

Conversation

@rishabhnambiar
Copy link
Member

commented Sep 11, 2019

Adding this to core allows showing the fullscreen button on whitelisted, external iframe videos.

Example from Vimeo
(Tested on Chrome + Firefox on Windows 10)

iframe

@discoursebot

This comment has been minimized.

Copy link

commented Sep 11, 2019

You've signed the CLA, rishabhnambiar. Thank you! This pull request is ready for review.

@rishabhnambiar rishabhnambiar requested a review from ZogStriP Sep 11, 2019

@eviltrout
Copy link
Member

left a comment

This seems fine but I am curious why you created a new file rather than adding it to white-lister.js.es6?

FEATURE: Whitelist the `allowfullscreen` iframe attribute
Adding this to core allows showing the fullscreen button on external iframe videos like Vimeo.
https://meta.discourse.org/t/full-screen-videos-plugin/90869 will not be needed anymore.

@rishabhnambiar rishabhnambiar force-pushed the rishabhnambiar:master branch from 4abe043 to 60492cc Sep 11, 2019

@rishabhnambiar

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

Ahhh, I was looking for an existing file in ../discourse-markdown/ for some reason. TIL about white-lister.js.es6, much better thanks!

@ZogStriP ZogStriP merged commit d93586c into discourse:master Sep 11, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.