Closed Bug 711276 Opened 10 years ago Closed 1 year ago

Consider to change allowfullscreen to allow="fullscreen"

Categories

(Core :: DOM: Core & HTML, defect, P5)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1608358

People

(Reporter: smaug, Unassigned)

References

Details

(Keywords: compat, parity-chrome)

... that way more features, like pointerlock, could be added to allow list.

http://lists.w3.org/Archives/Public/public-webapps/2011OctDec/1569.html

allow="fullscreen pointerlock"
Blocks: 633602
Version: unspecified → Trunk
I'm not as enthusiastic now as I was yesterday.
I wonder why. We need something for pointerlock, or otherwise we need to limit it to top level
browsing context.
what's wrong with "allowfullscreen", "allowpointerlock" etc?

It's easier to add, remove and test for existence of independent attributes than for tokens in a space-separated list.
how is it easier to remove independent attributes?
I would assume 'allow' will be exposed as a TokenList to JS, same way to 'class'
OK, but that requires extra DOM API.

off the top of my head, I know how to remove attributes, but not from DOMTokenList.
element.allow.remove("fullscreen");
Sure, but it's something you have to know.

OTOH I don't care. So I just withdraw any opinion.
No longer blocks: 633602
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML

This is now in HTML and seem to be a web compat issue in bug 1588377.

smaug, do you have any opinions on this?

[0]: https://html.spec.whatwg.org/#attr-iframe-allow, https://w3c.github.io/webappsec-feature-policy/#iframe-allowfullscreen-attribute

Flags: needinfo?(bugs)
Blocks: 1588377

That's very strange, bug 1572461 should have tackled this (or more likely something before it). Thomas will have a look.

Flags: needinfo?(bugs) → needinfo?(tnguyen)

I could see in the allowfullscreen="", not sure if it is well formatted and it seems we do not support that

Flags: needinfo?(tnguyen)

Oh well, allowfullscreen is a boolean attribute and we should have supported that

I took a quick look on this web page's source and I found that the structure of the embedded player is
iframe A(container) -> document -> embedded iframe B (player)
A =
<iframe id="xdm_default6411_provider" style="display: block; margin: 0px; padding: 0px; border: 0px none;" scrolling="no" src="https://twitter.com/AAAAAAAAAAAAAAAA" allowfullscreen="" width="100%" height="406" frameborder="0"></iframe>
B =
<iframe data-src="https://www.youtube.com/AAAAAAAA" scrolling="no" allowtransparency="true" sandbox="allow-popups allow-popups-to-escape-sandbox allow-same-origin allow-scripts" src="https://www.youtube.com/embed/AAAAAAAAAAA" frameborder="0"></iframe>
Of course, that is cross-origin and fullscreen is disabled by default in B.
And we have to explicitly declare allowfullscreen in B to make B possible to request fullscreen.

Another thing I found is Chrome
A = <iframe allow="autoplay" allowfullscreen="" src="https://cards-frame.twitter.com/XXXXXXXXX" sandbox="allow-popups allow-popups-to-escape-sandbox allow-same-origin allow-scripts" class="r-1yadl64 r-16y2uox"></iframe>

B = <iframe data-src="https://www.youtube.com/embed/XXXXXXXXXX" frameborder="0" scrolling="no" allowtransparency="true" allow="autoplay; fullscreen" sandbox="allow-popups allow-popups-to-escape-sandbox allow-same-origin allow-scripts allow-presentation" src="https://www.youtube.com/XXXXXXX">

I could see chrome got allow="autoplay; fullscreen" in B so chrome fullscreen works.
Likely, the documents we get in Firefox and Chrome are diferrents (Firefox without allow fullscreen and chrome with allow fullscreen), and I have no idea about that.
I dont think it is a compat issue here, annek what do you think?

Flags: needinfo?(annevk)

allow="fullscreen" was implemented by bug 1595720 and per https://w3c.github.io/webappsec-feature-policy/#iframe-allowfullscreen-attribute we still need to keep support for the allowfullscreen attribute. (On further inspection there appears to be a problem when combining these features, filed bug 1608358 on that.)

We'll need to follow up with Twitter about them not serving us either attribute and can do so via bug 1588377.

Resolving this as INVALID therefore.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(annevk)
Resolution: --- → INVALID

There's still a problem, we supported both allowfullscreen and allow="fullscreen", however, allowfullscreen is considered as a must, particularly for the case Youtube embedded :(, so it only works with allowfullscreen.

We have to set the condition is (allowfullscreen || allow="fullscreen") for all cases

Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Status: REOPENED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1608358
You need to log in before you can comment on or make changes to this bug.