Last Comment Bug 702295 - DOM Fullscreen interacts badly with Pandora
: DOM Fullscreen interacts badly with Pandora
Status: VERIFIED FIXED
[qa!]
: verified-aurora, verified-beta
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla10
Assigned To: Chris Pearce (:cpearce)
:
: Andrew Overholt [:overholt]
Mentors:
: 704250 (view as bug list)
Depends on:
Blocks: 545812 685779
  Show dependency treegraph
 
Reported: 2011-11-14 09:01 PST by Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
Modified: 2012-02-28 15:39 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified
verified


Attachments
Patch: Don't apply overflow:hidden to full-screen-ancestor chrome roots (1.55 KB, patch)
2011-11-17 14:35 PST, Chris Pearce (:cpearce)
bzbarsky: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-11-14 09:01:27 PST
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) 2011-11-14 13:01:45 PST
Weird. Doesn't happen on transition to/from F11 full-screen mode.
Comment 2 Jim Jeffery not reading bug-mail 1/2/11 2011-11-15 05:05:07 PST
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) 2011-11-15 14:37:34 PST
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) 2011-11-15 19:11:00 PST
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) 2011-11-15 19:23:38 PST
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 [:bz] (still a bit busy) 2011-11-15 19:26:18 PST
Which of those properties is doing it?
Comment 7 Chris Pearce (:cpearce) 2011-11-15 19:40:42 PST
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 [:bz] (still a bit busy) 2011-11-15 19:44:02 PST
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) 2011-11-15 20:51:37 PST
No!
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2011-11-15 21:23:31 PST
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) 2011-11-16 18:22:28 PST
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) 2011-11-17 14:35:46 PST
Created attachment 575301 [details] [diff] [review]
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 [:bz] (still a bit busy) 2011-11-17 17:59:16 PST
Comment on attachment 575301 [details] [diff] [review]
Patch: Don't apply overflow:hidden to full-screen-ancestor chrome roots

r=me
Comment 15 Ed Morley [:emorley] 2011-11-18 03:02:53 PST
https://hg.mozilla.org/mozilla-central/rev/974111a53365
Comment 16 Chris Pearce (:cpearce) 2011-11-20 13:06:06 PST
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 17 Boris Zbarsky [:bz] (still a bit busy) 2011-11-21 14:41:05 PST
*** Bug 704250 has been marked as a duplicate of this bug. ***
Comment 18 christian 2011-11-28 13:15:11 PST
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) 2011-11-28 15:00:43 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/b0708d072131
Comment 20 juan becerra [:juanb] 2012-02-28 14:04:43 PST
Verified in the latest nightly, aurora, beta and 10.0.2 on Win7 following the steps in comment #0.

Note You need to log in before you can comment on or make changes to this bug.