1302976 - Probably not allow user to put a video into fullscreen if the document is not allowed to

Status: NEW

(Firefox :: Menus, defect, P3)

Description

[Xidorn Quan [:xidorn] UTC+11]

Currently, chrome code has a privilege to put an element into fullscreen even if its owner document is not allowed to so. The only builtin UI for that is the "Full Screen" context menu of <video>.

I wonder whether we want to remove that privilege for chrome code, and hide "Full Screen" menu item if the document is not allowed to use fullscreen.

I don't think there is any security concern, but there is some UI consistency issue, as well as some issue which is probably hard to workaround like bug 1299795 (video in sidebar).

It seems hard to me to allow document in sidebar to enter fullscreen, so I'd prefer to just disable it.

For UI consistency, if you show controls for video, the fullscreen button wouldn't show if the document is not allowed to enter fullscreen, however the context menu would still show "Full Screen" even in that case, which seems to be inconsistent.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1302976 part 2 - Not give chrome code privilege to enter fullscreen.

Review commit: https://reviewboard.mozilla.org/r/79418/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79418/

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1302976 part 3 - Not display 'Full Screen' menu item if the document is not allowed to be in fullscreen.

Review commit: https://reviewboard.mozilla.org/r/79420/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79420/

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1302976 part 1 - Make tests not rely on chrome privilege of fullscreen.

Review commit: https://reviewboard.mozilla.org/r/79462/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79462/

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8792364 [details]
Bug 1302976 part 2 - Not give chrome code privilege to enter fullscreen.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79418/diff/1-2/

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8792365 [details]
Bug 1302976 part 3 - Not display 'Full Screen' menu item if the document is not allowed to be in fullscreen.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79420/diff/1-2/

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8792390 [details]
Bug 1302976 part 1 - Make tests not rely on chrome privilege of fullscreen.

https://reviewboard.mozilla.org/r/79462/#review78070

lgtm

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Why do we want this? I would expect videos in sandboxed document to be able to go to fullscreen if the chrome code (== user) wants that. Maybe. 
At least limiting that functionality might annoy users.

What do other browsers do?

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Olli Pettay [:smaug] (TPAC) from comment #7)
> Why do we want this? I would expect videos in sandboxed document to be able
> to go to fullscreen if the chrome code (== user) wants that. Maybe. 
> At least limiting that functionality might annoy users.

1. There are cases it seems hard to enter fullscreen, e.g. sidebar. (I'm concerned about sidebar and main browser entering fullscreen at the same time may confuse the browser state.)
2. Unexpected fullscreen change from subdocument might confuse their parent document somewhere?
3. There is UI inconsistency that the control bar doesn't show fullscreen button when in sandboxed document (because it's not chrome. But this can probably be resolved by making that allowed for ChromeOrXBL.)

> What do other browsers do?

Safari and Edge allows video to enter fullscreen for sandboxed document (with native UI e.g. control button or context menu). Chrome doesn't show fullscreen button when in sandboxed document, and it doesn't have context menu item for fullscreen.


Probably we don't want this, but I think we want to fix the UI inconsistency anyway... Probably we should also do some extra check for request from chrome to ensure fullscreen inside sidebar doesn't work.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Do Safari or Edge fire fullscreenchange event when entering to fullscreen in a document where web page can't trigger it, but it is browser chrome which does that?

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

They do fire that event.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

https://github.com/whatwg/fullscreen/issues/56

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8792364 [details]
Bug 1302976 part 2 - Not give chrome code privilege to enter fullscreen.

Clearing the request, since I think we'll need to sort this out in the spec level first.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 14

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.