1036597 - Extend MakeFullScreen to take a nsIScreen for selecting full screen display

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Right now, MakeFullScreen makes the window full screen on the monitor that it's on.  It would be useful if it could make the window full screen on a specific monitor.

The API should be extended to take an optional nsIScreen pointer that chooses the screen on which the window is to be made full screen.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: add optional nsIScreen arg to MakeFullScreen

This currently only works on Windows right now, because the other platforms have different ways of making things full screen that I don't fully understand.

Gtk - Karl, right now the gtk code just calls gtk_window_fullscreen(win).  Can I get it to go on the right screen by moving the window to that screen first (gtk_window_move/gtk_window_resize) and then calling gtk_window_fullscreen?

Cocoa - Steven, I don't quite understand nsCocoaWindow's MakeFullScreen, specifically mUsesNativeFullScreen.  If I wanted to make the window full screen on a specific screen, how would I go about it?  Same as with Gtk, move it to the right screen first?

This doesn't need to work on any mobile devices at the moment.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8468600 [details] [diff] [review]
add optional nsIScreen arg to MakeFullScreen

> Cocoa - Steven, I don't quite understand nsCocoaWindow's
> MakeFullScreen, specifically mUsesNativeFullScreen.  If I wanted to
> make the window full screen on a specific screen, how would I go
> about it?  Same as with Gtk, move it to the right screen first?

OS X has only had "native" support for a fullscreen mode since OS X
10.7, so we only support it on 10.7 and up.  On OS X 10.6 we use a
fullscreen mode that looks very much like the fullscreen mode on other
platforms.

The "native" fullscreen mode was a real bitch to get working, and
frankly it still doesn't work entirely correctly.  For example a lot
of the fullscreen mode mochitests fail, probably because the
transition to fullscreen is deliberately slow (several seconds, to
give Apple a chance to show off its animation), and at least some of
our code assumes the transition has finished before it really has.

For several years I've wanted to give this code a thorough overview,
and at least partial rewrite.  But I'm not sure I'll ever have the
time for it.

So firstly, you need to be careful about making changes to this code,
at least wrt the native fullscreen mode.  Here be dragons.

Secondly, I don't know of a documented way to make a given Cocoa
window fullscreen ("natively") on a particular screen.  The standard
call is -[NSWindow toggleFullScreen:(id)sender].  (And yes, there's a
way to discover an NSWindow's fullscreen "state" -- by looking at the
value of the NSFullScreenWindowMask bit in its "styleMask".)

We (you or I) could probably figure out an undocumented way to do
this, but it'd be time consuming and possibly risky.  It'd also go
against Apple's idea of how native fullscreen should work.  In a
multiscreen system, the OS expects to be able to choose which screen
to make a Cocoa window fullscreen on.  See for example:

http://support.apple.com/kb/HT5079?viewlocale=en_US

So I'd really prefer not to do this, at least with the native
fullscreen mode.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8468600 [details] [diff] [review]
add optional nsIScreen arg to MakeFullScreen

> Same as with Gtk, move it to the right screen first?

Oops, missed this.  That would probably "work", but I still really don't like the idea of doing this.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Note that this is specifically for the case of when the screen has specific characteristics that are required by the fullscreen app -- in the immediate case, when the "screen" is a VR head-mounted display and we are entering VR mode.  Going fullscreen on the current display in that case will actually be wrong and annoying to the user, because they will be seeing a distorted split view.

I can see other use cases with different Web APIs, e.g. if the screen is known to be a projector, and we want to have a window show up on the projector.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

It's possible that OS X would choose the "right" screen anyway.  And if not, it'd be best to find some way of "encouraging" it to do so -- i.e. to twist its arm but let the OS do the actual work, in its black-boxy way.  If nothing else, this would be less likely to break the transition animation.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Do you know if OS X has any native support for VR displays?

Comment 7

[Steven Michaud [:smichaud] (Retired)]

One alternative is to ditch the "native fullscreen" mode altogether when working with a VR device, and use the older, cross-platform fullscreen mode that we use on OS X 10.6.

Comment 8

[Karl Tomlinson (:karlt)]

Comment on attachment 8468600 [details] [diff] [review]
add optional nsIScreen arg to MakeFullScreen

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #1)
> Gtk - Karl, right now the gtk code just calls gtk_window_fullscreen(win). 
> Can I get it to go on the right screen by moving the window to that screen
> first (gtk_window_move/gtk_window_resize) and then calling
> gtk_window_fullscreen?

That would put it on the right monitor with most window managers.
Unfortunately though, exiting fullscreen would not restore to the intended
monitor/position.  That might be OK for a dedicated fullscreen window but not
for a typical browser window.

_NET_WM_FULLSCREEN_MONITORS [1] provides a means to request a particular
screen from the Xinerama index.  The message/hint was designed for fullscreen
presentation across multiple monitors, but I assume the same monitor can be
specified for all edges.

GDK doesn't provide an API for this, but Xlib can be used.

[1] http://standards.freedesktop.org/wm-spec/1.5/ar01s06.html#idm140251367957040

>      * Put the toplevel window into or out of fullscreen mode.
>-     *
>+     * If aTargetScreen is given, go fullscreen on that screen.
>      */
>-    NS_IMETHOD MakeFullScreen(bool aFullScreen) = 0;
>+    NS_IMETHOD MakeFullScreen(bool aFullScreen, nsIScreen* aTargetScreen = nullptr) = 0;

Please spell out explicitly that aTargetScreen is ignored if !aFullScreen (assuming that is the intention).

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: add optional nsIScreen arg to MakeFullScreen (v2, xpwidgets/win only)

I'd like to get the framework for this in and the Windows implementation (well, the xpwidgets implementation, which is all that Windows needs).  There is no VR support on Linux yet, so that can wait; and OSX has enough complexity that I'd like to do that as a separate followup patch.

Comment 10

[Karl Tomlinson (:karlt)]

Comment on attachment 8514422 [details] [diff] [review]
add optional nsIScreen arg to MakeFullScreen (v2, xpwidgets/win only)

Can you note in the nsIWidget doc that this is currently only implemented on
WINNT, or whatever is appropriate, please?

+++ b/widget/gonk/nsWindow.cpp
@@ -451,7 +451,7 @@ nsWindow::ReparentNativeWidget(nsIWidget* aNewParent)
 }
 
 NS_IMETHODIMP
-nsWindow::MakeFullScreen(bool aFullScreen)
+nsWindow::MakeFullScreen(bool aFullScreen, nsIntRect*)

nsIScreen*

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to Karl Tomlinson (:karlt) from comment #10)
> Comment on attachment 8514422 [details] [diff] [review]
> add optional nsIScreen arg to MakeFullScreen (v2, xpwidgets/win only)
> 
> Can you note in the nsIWidget doc that this is currently only implemented on
> WINNT, or whatever is appropriate, please?

Done: 

+     * If aTargetScreen is given, attempt to go fullscreen on that screen,
+     * if possible.  (If not, it behaves as if aTargetScreen is null.)
+     * If !aFullScreen, aTargetScreen is ignored.
+     * aTargetScreen support is currently only implemented on Windows.


> +++ b/widget/gonk/nsWindow.cpp
> @@ -451,7 +451,7 @@ nsWindow::ReparentNativeWidget(nsIWidget* aNewParent)
>  }
>  
>  NS_IMETHODIMP
> -nsWindow::MakeFullScreen(bool aFullScreen)
> +nsWindow::MakeFullScreen(bool aFullScreen, nsIntRect*)
> 
> nsIScreen*

Whoops!  Tryservering the lot of this right now :)

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d8aab26a3af

Will file followup bugs for OSX & Linux.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/66f63e8af1ca

Comment 14

[Eric Shepherd [:sheppy]]

I think my addition of dev-doc-needed here was unnecessary; this method is internal and is never mentioned on MDN. I'm removing dev-doc-needed; please re-add if I'm mistaken and let me know why.