According to the spec, only documents inside <iframe> should be able to request fullscreen, because a document which is neither topmost nor inside <iframe> should never have the "fullscreen enabled flag" set. For us, we should still allow <xul:browser>, but other elements which can include nested document should be banned.

Bug 1212299 part 1 - Forbid documents inside elements other than iframe from requesting fullscreen.

Bug 1212299 part 2 - Rewrite fullscreen-denied test to have a clearer structure.

Bug 1212299 part 3 - Add test for requesting fullscreen from doc inside frame/object.

Comment on attachment 8671329 [details] MozReview Request: Bug 1212299 part 2 - Rewrite fullscreen-denied test to have a clearer structure. We really should unprefix the API.

Comment on attachment 8671330 [details] MozReview Request: Bug 1212299 part 3 - Add test for requesting fullscreen from doc inside frame/object. rs+

(In reply to Olli Pettay [:smaug] from comment #4) > Comment on attachment 8671329 [details] > MozReview Request: Bug 1212299 part 2 - Rewrite fullscreen-denied test to > have a clearer structure. > > We really should unprefix the API. Yes, see dependencies of bug 743198. I'm going to unprefix the API when we have a reasonably up-to-date impl.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5265f27a0b92aa1c82ff5f9eb3d212991438c51 Bug 1212299 part 1 - Forbid documents inside elements other than iframe from requesting fullscreen. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/3bd171d5bcb5c7dffe5c36bfa17b509fe3fb295a Bug 1212299 part 2 - Rewrite fullscreen-denied test to have a clearer structure. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/9a578edd91140c62b6c1a555f0acf8fd7ee28536 Bug 1212299 part 3 - Add test for requesting fullscreen from doc inside frame/object. rs=smaug

https://hg.mozilla.org/mozilla-central/rev/d5265f27a0b9 https://hg.mozilla.org/mozilla-central/rev/3bd171d5bcb5 https://hg.mozilla.org/mozilla-central/rev/9a578edd9114

Added the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/documents-in-frame-or-object-can-no-longer-request-fullscreen/

(In reply to Carsten Book [:Tomcat] from comment #8) > https://hg.mozilla.org/mozilla-central/rev/d5265f27a0b9 > > +++ b/dom/locales/en-US/chrome/dom/dom.properties > -FullScreenDeniedIframeNotAllowed=Request for full-screen was denied because at least one of the document's containing iframes does not have an "allowfullscreen" attribute. > +FullScreenDeniedContainerNotAllowed=Request for full-screen was denied because at least one of the document's containing element is not iframe or does not have an "allowfullscreen" attribute. > > +++ b/webapprt/locales/en-US/webapprt/overrides/dom.properties > -FullScreenDeniedIframeNotAllowed=Request for full-screen was denied because at least one of the document's containing iframes does not have an "allowfullscreen" attribute. > +FullScreenDeniedContainerNotAllowed=Request for full-screen was denied because at least one of the document's containing element is not iframe or does not have an "allowfullscreen" attribute. I noticed this in both dom.properties files during l10n: "… one of the containing element is not iframe" Shouldn’t this be "… one of the containing elements is not an iframe"?

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c5d3e1ee51053e15337f5fa363098a3f606adb8 Bug 1212299 followup - Fix minor grammar issue in locale text. DONTBUILD

Approval Request Comment [Feature/regressing bug #]: this bug [User impact if declined]: may see warning with grammar mistakes [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: there's a risk that the sentences are still wrong :) [String/UUID change made/needed]: string changes

Personally I'd be in favor of this riding the trains with 45: it's going to create unnecessary noise in a string frozen repository, and I'm not convinced it's actually improving clarity (and for that it would need a new string ID). > Request for full-screen was denied because at least one of the document's > containing elements is not an iframe or does not have an "allowfullscreen" attribute. e.g. is it correct for an object to have multiple containers? I think it would be a lot easier to follow as "… one of the elements containing the document is not…".

(In reply to Francesco Lodolo [:flod] from comment #14) > Personally I'd be in favor of this riding the trains with 45: it's going to > create unnecessary noise in a string frozen repository, and I'm not > convinced it's actually improving clarity (and for that it would need a new > string ID). I'm not quite familiar with the rules there, and just wanted to try and see what would happen. If that would add much trouble, you can a- this request. I'm fine either way. > > Request for full-screen was denied because at least one of the document's > > containing elements is not an iframe or does not have an "allowfullscreen" attribute. > > e.g. is it correct for an object to have multiple containers? I think it > would be a lot easier to follow as "… one of the elements containing the > document is not…". There could be nested container, e.g. a document inside an <iframe> whose document is inside a <frame>.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #15) > I'm not quite familiar with the rules there, and just wanted to try and see > what would happen. If that would add much trouble, you can a- this request. > I'm fine either way. There are 2 basic rules: * All branches besides mozilla-central are string frozen. Release drivers are in charge of approving such changes, and that's why you need to specify if there string changes. * You shouldn't change existing strings without using a new ID https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings In case of doubt, ask before landing. The change you already landed on m-c is kind of border-line, but I would have been better to double check.

Well, I think this fix should be fine because the meaning is effectively unchanged, just a grammar fix :)

Comment on attachment 8688824 [details] [diff] [review] followup locale text fix As Flod pointed out, we don't accept string changes during Aurora/Beta cycles unless there is a strong marketing/PR/end-user impact/justification. Please let this one ride the train.

Updated: https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen and https://developer.mozilla.org/en-US/Firefox/Releases/44#Miscellaneous