In the iframe created for HTML5 video playback, Youtube videos have the fullscreen button greyed out. I'll upload a simple patch to allow fullscreen in a minute.

The approach I'm taking is to add the "allowfullscreen" attribute automatically to the iframe element that is generated. I see things like "frameborder" added automatically, and I don't see a lot of people who would need to have this as a configurable option for end users.

If the behavior is not wanted automatically, I suppose a variable could be added to the module settings to add this attribute automatically, but using the 80/20 logic, I think it should just be added by default on module installation.

CommentFileSizeAuthor
#4 video_filter-allow_iframe_fullscreen-2577779-3-7.x_3_dev.patch950 bytesrvtraveller
#3 video_filter-add-allowfullscreen-attribute-2577779-3-7.patch677 bytesafinnarn
#2 video_filter-add-allow-fullscreen-attribute-2577779-2-7.patch794 bytesafinnarn

Comments

afinnarn created an issue. See original summary.

afinnarn’s picture

afinnarn commented
StatusFileSize
new video_filter-add-allow-fullscreen-attribute-2577779-2-7.patch794 bytes

uploading patch

afinnarn’s picture

afinnarn commented
StatusFileSize
new video_filter-add-allowfullscreen-attribute-2577779-3-7.patch677 bytes

adding a patch against the 7.x-3.1 tag...because my other patch wouldn't apply to the version of this module I have.

rvtraveller’s picture

rvtraveller commented
Assigned: afinnarn » Unassigned
Status: Active » Needs review
StatusFileSize
new video_filter-allow_iframe_fullscreen-2577779-3-7.x_3_dev.patch950 bytes

The patch in #3 doesn't apply to the dev version of the module because the markup seems to have changed (commit f9a7151492e30e2a10f2247653784ea3970bbdde). Adding a new patch that applies to dev and resolves the issue. It also keeps @afinnarn's approach to just giving everyone fullscreen.

afinnarn’s picture

afinnarn commented

...attributing credit to employer instead of volunteering time

fullerja’s picture

fullerja commented
Status: Needs review » Reviewed & tested by the community

Patch in #2 applies to dev and works for me

kevincrafts’s picture

kevincrafts commented

#3 worked for me.

rvtraveller’s picture

rvtraveller commented

Whoops. Completely missed that @afinnarn's patch in #2 was the same as what I did in #4. Since our patches are the same, +1 to RTBC.

  • minnur committed c87eefa on 7.x-3.x 
    Show fullscreen button disabled in iframe for Youtube videos. [#2577779]
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Fixed

Committed.

minnur’s picture

minnur commented
Status: Fixed » Closed (fixed)