702295 - DOM Fullscreen interacts badly with Pandora

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

STR:

1. Open Pandora in a tab, start playing music.
2. Open http://html5-demos.appspot.com/static/fullscreen.html
3. Click the full screen button

What happens:

Pandora stops playing audio, is now in a broken state.

Expected:

Pandora keeps playing music.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Weird. Doesn't happen on transition to/from F11 full-screen mode.

Comment 2

[Jim Jeffery not reading bug-mail 1/2/11]

I got a chance to test this morning and for me when doing the html5 demo full screen, Pandora stops for a couple of seconds, then resumes play.  Same for when exiting full screen demo, a pause of 2-3 seconds, then play resumes and the controls etc on Pandora are all functional. 

Lurking

System, win7 x64, 8 gig RAM, HD3200 on-board video card, HWA all enabled.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

This regressed for me between:

GOOD 2011-11-01-03-11-08
BAD  2011-11-02-03-10-56

Regression range is:

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-11-01+03%3A11%3A08&enddate=2011-11-02+03%3A10%3A56

This includes the landings to implement :-moz-full-screen-ancestor...

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

I've done a bisection, and this is a regression from bug 685779; the landings to implement :-moz-full-screen-ancestor.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

And yes, if we remove the style rules for :-moz-full-screen-ancestor from ua.css, pandora works as expected when we enter full-screen.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Which of those properties is doing it?

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

This one:

/* If there is a full-screen element that is not the root then
   we should hide the viewport scrollbar. */
*|*:root:-moz-full-screen-ancestor {
  overflow: hidden !important;
}

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, I missed that rule.  Yeah, that would do it: it's causing a reframe of the <window> element in the browser UI, I bet, which reframes all plug-ins.

Do we want to propagate the :-moz-full-screen-ancestor into the browser UI?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No!

Comment 10

[Boris Zbarsky [:bzbarsky]]

So right now we go up the GetParentDocument chain in the fullscreen code.

We should probably change that to check whether we're changing docshell type and if so stop?

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Right, we still need to propagate full-screen state into the chrome doc (currently other code relies on that) so the easiest thing to do is to simply not apply :-moz-full-screen-ancestor to the chrome doc (we should still apply the other full-screen state).

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Don't apply overflow:hidden to full-screen-ancestor chrome roots

On second thoughts, I think it makes more sense to change the rule, rather than making :-moz-full-screen-ancestor not apply to the chrome document's root. The chrome document's root *is* an ancestor of the full-screen element after all.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 575301 [details] [diff] [review]
Patch: Don't apply overflow:hidden to full-screen-ancestor chrome roots

r=me

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/974111a53365

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/974111a53365

Comment 16

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 575301 [details] [diff] [review]
Patch: Don't apply overflow:hidden to full-screen-ancestor chrome roots

It would be good to get this on Aurora because if stops entering DOM full-screen causing plugins in other tabs to re-initialize, which will for example kill music playing or farmville running in a plugin in a background tab.

Comment 18

[LegNeato]

Comment on attachment 575301 [details] [diff] [review]
Patch: Don't apply overflow:hidden to full-screen-ancestor chrome roots

We're early enough in the aurora cycle to take this.

Comment 19

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b0708d072131

Comment 20

[juan becerra [:juanb]]

Verified in the latest nightly, aurora, beta and 10.0.2 on Win7 following the steps in comment #0.