Bug 58291 - REGRESSION: Vimeo fullscreen video displays incorrectly
Summary: REGRESSION: Vimeo fullscreen video displays incorrectly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Jer Noble
URL: http://vimeo.com/21864555
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2011-04-11 17:35 PDT by Jer Noble
Modified: 2011-04-24 18:10 PDT (History)
3 users (show)

See Also:


Attachments
Patch (7.12 KB, patch)
2011-04-11 22:58 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (15.13 KB, patch)
2011-04-12 12:16 PDT, Jer Noble
simon.fraser: review+
Details | Formatted Diff | Diff
Fullscreen works, but scrollbar fails to hide and controls are not centered. (478.47 KB, image/png)
2011-04-14 12:00 PDT, sharikov.suba
no flags Details
Screenshot with r84622 (3.45 MB, image/png)
2011-04-24 14:31 PDT, Edward Sabol
no flags Details
Fullscreen Working Properly on r84622 (3.49 MB, video/quicktime)
2011-04-24 15:29 PDT, sharikov.suba
no flags Details
document.webkitCurrentFullScreenElement Output (741.71 KB, image/png)
2011-04-24 17:51 PDT, sharikov.suba
no flags Details
Youtube fails to go fullscreen (966.66 KB, video/quicktime)
2011-04-24 18:10 PDT, sharikov.suba
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-04-11 17:35:04 PDT
4/9/11 8:36 AM Simon Fraser:
If I go to http://vimeo.com/21864555 (in HTML5 mode), start playing the video, and then take the video fullscreen, the fullscreen presentation is totally messed up. The video is unscaled, and there are bits of the page scattered around.

WebKit nightly r83321 on 10.6.7
Comment 1 Jer Noble 2011-04-11 17:37:56 PDT
<rdar://problem/9261869>
Comment 2 Jer Noble 2011-04-11 22:58:28 PDT
Created attachment 89160 [details]
Patch
Comment 3 Simon Fraser (smfr) 2011-04-12 10:09:29 PDT
I'm concerned that we're adding a lot of special magic here. Is this z-index issue discussed in the fullscreen spec?
Comment 4 Jer Noble 2011-04-12 10:57:21 PDT
(In reply to comment #3)
> I'm concerned that we're adding a lot of special magic here. Is this z-index issue discussed in the fullscreen spec?

The spec specifically suggests setting the zIndex to 2^31-1.  So there is no "special magic" here.

https://wiki.mozilla.org/Gecko:FullScreenAPI
Comment 5 Simon Fraser (smfr) 2011-04-12 11:00:52 PDT
Comment on attachment 89160 [details]
Patch

Is there a way to test the z-index issue? Do pixel tests work for the fullscreen window?
Comment 6 Jer Noble 2011-04-12 11:04:06 PDT
Pixel tests of fullscreen content should work in DRT; it doesn't use WebKit's full screen window, and it doesn't do any animation, but the test shouldn't require that.  I'll add one.

Thanks!
Comment 7 Jer Noble 2011-04-12 12:16:56 PDT
Created attachment 89242 [details]
Patch
Comment 8 Simon Fraser (smfr) 2011-04-12 15:27:38 PDT
Comment on attachment 89242 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=89242&action=review

> Source/WebCore/css/fullscreen.css:16
> +    background-color: black ! important;
> +    position: static ! important;
> +    margin: 0 ! important;
> +    height: 100% ! important;
> +    width: 100% ! important;
> +    -webkit-box-flex: 1 ! important;
> +    display: block ! important;

We don't put a space after the ! normally.

> Source/WebCore/html/HTMLMediaElement.cpp:1495
> +    // always show controls when in full screen mode.

Should use sentence case.
Comment 9 Jer Noble 2011-04-12 15:46:39 PDT
Committed r83654: <http://trac.webkit.org/changeset/83654>
Comment 10 sharikov.suba 2011-04-14 12:00:34 PDT
Created attachment 89619 [details]
Fullscreen works, but scrollbar fails to hide and controls are not centered.
Comment 11 sharikov.suba 2011-04-14 12:01:51 PDT
Attached screenshot of vimeo video fullscreen on release r83831.
Comment 12 Edward Sabol 2011-04-24 14:21:17 PDT
Should this bug be reopened due to the issues described in comment #10 and comment #11 or is there a follow-up bug?
Comment 13 Edward Sabol 2011-04-24 14:31:00 PDT
Created attachment 90888 [details]
Screenshot with r84622

Shortly after launching the latest Webkit Nightly (r84622), I went to a Vimeo page and clicked on the fullscreen button prior to playing the video. This is what I see. If I start playing the video and click the fullscreen button a couple times, it will eventually go fullscreen. (Also, the scroll bar and non-centered controls noted in comment #10 and comment #11 appear to no longer be an issue with r84622.)
Comment 14 sharikov.suba 2011-04-24 15:29:39 PDT
Created attachment 90890 [details]
Fullscreen Working Properly on r84622

Fullscreen is working properly for me ( MBPRO 13 2009 ) on  r84622.
Comment 15 Edward Sabol 2011-04-24 16:00:23 PDT
(In reply to comment #14)
> Created an attachment (id=90890) [details]
> Fullscreen Working Properly on r84622
> 
> Fullscreen is working properly for me ( MBPRO 13 2009 ) on  r84622.

Ah, but you clicked play first. I said I clicked the fullscreen button first.
Comment 16 sharikov.suba 2011-04-24 16:42:31 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > Created an attachment (id=90890) [details] [details]
> > Fullscreen Working Properly on r84622
> > 
> > Fullscreen is working properly for me ( MBPRO 13 2009 ) on  r84622.
> 
> Ah, but you clicked play first. I said I clicked the fullscreen button first.

My fullscreen issues with the visible scrollbar and player alignment have been resolved.

I tried to press the fullscreen button prior to play, and had the same results.  I can confirm your issue.
Comment 17 Jer Noble 2011-04-24 16:48:05 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > Created an attachment (id=90890) [details] [details] [details]
> > > Fullscreen Working Properly on r84622
> > > 
> > > Fullscreen is working properly for me ( MBPRO 13 2009 ) on  r84622.
> > 
> > Ah, but you clicked play first. I said I clicked the fullscreen button first.
> 
> My fullscreen issues with the visible scrollbar and player alignment have been resolved.
> 
> I tried to press the fullscreen button prior to play, and had the same results.  I can confirm your issue.

Can either of you verify that Vimeo actually entered the new full screen mode?  Both they and YouTube have a "pseudo" full screen mode which just takes up the current window.

You can verify this by using the Inspector and checking the output of document.webkitCurrentFullScreenElement.
Comment 18 sharikov.suba 2011-04-24 17:51:39 PDT
Created attachment 90893 [details]
document.webkitCurrentFullScreenElement Output

document.webkitCurrentFullScreenElement: null
document.webkitIsFullScreen: false
Comment 19 sharikov.suba 2011-04-24 18:10:06 PDT
Created attachment 90894 [details]
Youtube fails to go fullscreen

document.webkitCurrentFullScreenElement: null
document.webkitIsFullScreen: false

It looks like youtube is using the pseudo fullscreen.