Closed
Bug 1212299
Opened 8 years ago
Closed 8 years ago
Forbid documents inside <frame> and <object> from requesting fullscreen
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(4 files)
40 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
5.32 KB,
patch
|
ritu
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
Bug 1212299 part 1 - Forbid documents inside elements other than iframe from requesting fullscreen.
Attachment #8671328 -
Flags: review?(bugs)
Assignee | ||
Comment 2•8 years ago
|
||
Bug 1212299 part 2 - Rewrite fullscreen-denied test to have a clearer structure.
Attachment #8671329 -
Flags: review?(bugs)
Assignee | ||
Comment 3•8 years ago
|
||
Bug 1212299 part 3 - Add test for requesting fullscreen from doc inside frame/object.
Attachment #8671330 -
Flags: review?(bugs)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → quanxunzhen
Updated•8 years ago
|
Attachment #8671328 -
Flags: review?(bugs) → review+
Comment 4•8 years ago
|
||
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.
Attachment #8671329 -
Flags: review?(bugs) → review+
Comment 5•8 years ago
|
||
Comment on attachment 8671330 [details] MozReview Request: Bug 1212299 part 3 - Add test for requesting fullscreen from doc inside frame/object. rs+
Attachment #8671330 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 6•8 years ago
|
||
(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.
Assignee | ||
Comment 7•8 years ago
|
||
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
Comment 8•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d5265f27a0b9 https://hg.mozilla.org/mozilla-central/rev/3bd171d5bcb5 https://hg.mozilla.org/mozilla-central/rev/9a578edd9114
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Updated•8 years ago
|
Keywords: dev-doc-needed
Comment 9•8 years ago
|
||
Added the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/documents-in-frame-or-object-can-no-longer-request-fullscreen/
Keywords: site-compat
Comment 10•8 years ago
|
||
(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"?
Assignee | ||
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c5d3e1ee51053e15337f5fa363098a3f606adb8 Bug 1212299 followup - Fix minor grammar issue in locale text. DONTBUILD
Assignee | ||
Comment 12•8 years ago
|
||
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
Attachment #8688824 -
Flags: approval-mozilla-aurora?
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8c5d3e1ee510
Comment 14•8 years ago
|
||
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…".
Assignee | ||
Comment 15•8 years ago
|
||
(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>.
Comment 16•8 years ago
|
||
(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.
Assignee | ||
Comment 17•8 years ago
|
||
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.
Attachment #8688824 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Comment 19•8 years ago
|
||
Updated: https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen and https://developer.mozilla.org/en-US/Firefox/Releases/44#Miscellaneous
Keywords: dev-doc-needed → dev-doc-complete
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•