520805 - fullscreen video plays back on wrong monitor (multiple monitor setup)

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Tony Chung [:tchung]]

with a secondary monitor (desktop sharing mode), drag your browser to the 2nd screen, then go to FS.  you'll notice the FS occurs on the first monitor, while the browser video is paused.   In flash, FS applies to the screen that has focus.

Repro:
1) install Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091006 Namoroka/3.6b1pre or trunk
2) Attach a secondary monitor, enabling desktop sharing mode
3) Drag the browser to the secondary monitor
4) Play an OGG video in Full screen  (use URL example)
5) Verify OGG video plays in FS in the primary monitor, while the secondary monitor pauses the video in browser

Comment 2

[LonelyBob]

Reproduced with essentially the same steps except on OSX 10.4. Fullscreen video plays on the wrong display. Reproduced on 3.6 release: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6

Hope that this will be worked on to be fixed in next 3.6.x release.

Comment 3

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

Updating summary and component

Comment 4

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

Think this needs to be fixed for Fx4 -- we're making video a big part of that release, and this is essentially a papercut there.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch

So there are various things that contribute to fullScreen-video.xhtml opening on the wrong monitor. The main issue at play is that generic windows that don't have screenX/screenY persist always initially open at 0,0. Then window.fullScreen = true kicks in and it maximizes to the current screen where the window is (which is correct).

I think this patch fixes it in an non-intrusive way. When a window is set to "depedent,center", it is initialized with the parent's coordinates (in addition to the parent's dimensions), and thus is centered on the right screen (so this would be an existing bug regardless of fullscreen or not). Then when we make that window fullscreen, it goes to the correct placement.

Just by adding the "depedent,center" flags to openDialog (without the nsXULWindow changes), it almost works. The problem is that the centering happens after window.fullScreen is called on fullscreen-video.xhtml. This brings the window to the correct monitor, but with the wrong dimensions (because fullScreen=true already sized it to the primary screen's dimensions)

bz, are you the right person to review this patch?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Probably not if you want a quick review; I'd have to sort through all this stuff to review this sanely...  Maybe jst or roc?

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 460639 [details] [diff] [review]
Patch

Okay, I'll try jst. Johnny, please see comment #5 for an explanation of the fix.

Comment 8

[Timothy Nikkel (:tnikkel)]

So 'centerscreen' isn't enough, you need 'dependent' as well?

Comment 9

[:Felipe Gomes (needinfo for replies!)]

(In reply to comment #8)
> So 'centerscreen' isn't enough, you need 'dependent' as well?

Oh you're right. The code I read on nsAppShellService::JustCreateTopWindow made me think 'centerscreen' was dependent on 'dependent', but now I see that's not the case.

'centerscreen' alone should be enough.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 460639 [details] [diff] [review]
Patch

- In nsWebShellWindow::Initialize():

   if (base) {
     rv = base->GetPositionAndSize(&mOpenerScreenRect.x,
                                   &mOpenerScreenRect.y,
                                   &mOpenerScreenRect.width,
                                   &mOpenerScreenRect.height);
     if (NS_FAILED(rv)) {
       mOpenerScreenRect.Empty();
     }
+  } else {
+    mOpenerScreenRect.Empty();
   }

This change looks unnecessary, mOpenerScreenRect starts out empty AFAICT, and this should never be called after it has been changed, again, AFAICT.
 
r=jst with the dependent option removed, and the above looked into.

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Attachment: For check-in

Carrying forward r=jst.

Removed "dependent" from the string and looked into the above. Both are true, so emptying the rect was unecessary.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Not a blocker, but a=beltzner

Comment 13

[:Felipe Gomes (needinfo for replies!)]

http://hg.mozilla.org/mozilla-central/rev/3b59889221bc