Closed Bug 1279613 Opened 3 years ago Closed 3 years ago
Video in non-sandboxed iframe with allowfullscreen is blocked from going fullscreen
58 bytes, text/x-review-board-request
@ Xidorn: is this a bug (a regression from bug 1270648) or by-design? The video's iframe has the allowfullscreen and mozallowfullscreen attributes and, AFAICT, is *not* a sandboxed iframe. STR: 1. Load http://arstechnica.co.uk/the-multiverse/2016/06/sunspring-movie-watch-written-by-ai-details-interview/ 2. Play the video. (The fullscreen button is not shown until the video is playing.) 3. Click the video player's fullscreen button. RESULT: Firefox logs the following console warning (from bug 1270648) and does not allow the video to go fullscreen: > Request for fullscreen was denied because at least one of the document’s containing elements is not an iframe or does not have an “allowfullscreen” attribute. Chrome and Edge allow the video to go fullscreen. The video's iframe has the allowfullscreen and mozallowfullscreen attributes and, AFAICT, is *not* a sandboxed iframe: <iframe style="position:absolute;top:0;left:0;width:1px !important;height:1px !important;min-width:100%;min-height:100%;" allowfullscreen="" webkitallowfullscreen="" mozallowfullscreen="" msallowfullscreen="" allowtransparency="true" scrolling="no" frameborder="0"></iframe>
This sounds like something related to the ongoing discussion in WHATWG: https://github.com/whatwg/html/issues/1385 The code is in insertEmbed function in http://player.cnevids.com/embedjs/57587c44ba4aa1013c00000c/5511d76261646d5566020000.js and this code inserts an iframe and then injects player code into the document inside the iframe directly, which means no document other than about:blank is actually loaded, and thus the fullscreen enabled flag is not set. So it looks like the conformance fix in bug 1270648 breaks actual web content, which should be fixed from the spec side, and we should track this for Firefox 49 in case we fail to address this issue.
The spec bug 1270648 implemented is clearly wrong. We should fix our implementation for initial about:blank and not wait for the spec update, which I expect will be forthcoming soon anyway.
Review commit: https://reviewboard.mozilla.org/r/59370/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/59370/
Attachment #8762980 - Flags: review?(bugs)
I guess I still need to update UUID of that IDL file?
no need for uuid updates anymore.
huh, I did bad job with Bug 1270648 :/
Comment on attachment 8762980 [details] Bug 1279613 - Apply fullscreen enabled flag to about:blank as well. https://reviewboard.mozilla.org/r/59370/#review56646
Comment on attachment 8762980 [details] Bug 1279613 - Apply fullscreen enabled flag to about:blank as well. Approval Request Comment [Feature/regressing bug #]: bug 1270648 [User impact if declined]: sometimes websites may not be able to enter fullscreen when they should be allowed to [Describe test coverage new/current, TreeHerder]: add new web-platform test [Risks and why]: considered low risk, code is simple and only touch sandbox and fullscreen, whose related code should have a good test coverage [String/UUID change made/needed]: no UUID change but there is an IDL file updated
Attachment #8762980 - Flags: approval-mozilla-aurora?
Comment on attachment 8762980 [details] Bug 1279613 - Apply fullscreen enabled flag to about:blank as well. Fix for recent regression, let's uplift to aurora.
Attachment #8762980 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.