Bug 61569

Summary: Fullscreen content is sometimes obscured by taskbar (which even covers up the "exit fullscreen" button)
Product: WebKit Reporter: Jeff Miller <jeffm>
Component: MediaAssignee: Jeff Miller <jeffm>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, jer.noble, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch to make fullscreen window a topmost window. sfalken: review+

Description Jeff Miller 2011-05-26 15:10:46 PDT
From Adam Roben:

To reproduce:

1. Go to http://flexibits.com/fantastical
2. Click on "Watch the screencast"
3. Click the fullscreen button in the video controls to enter fullscreen

Sometimes, the taskbar will appear in front of the fullscreen window. This obscures the video controls, including the "exit fullscreen" button.
Comment 1 Jeff Miller 2011-05-26 15:11:39 PDT
This appears to only happen the first time you take a video fullscreen.  If I exit full screen and enter it again, the taskbar is hidden.
Comment 2 Jeff Miller 2011-05-26 15:12:03 PDT
Going to fullscreen involves:

1) Creating two initially hidden fullscreen windows, specifically MediaPlayerPrivateFullscreenWindow, one for the black "background" and one that contains the content.
2) Calling AnimateWindow() on the background window to animate showing it.
3) Forcing the WebView to repaint (which is synchronous in WK1, async in WK2).
4) When FullScreenController is notified that the WebView has repainted, it uses AnimateWindow() on the real fullscreen window to animate showing it.

When the taskbar is hidden correctly, it appears to hide during step 2, which makes sense since showing the background window is the first time we show a fullscreen window.  This would also seem to rule out any differences between WK1 and WK2, since the first call to AnimateWindow() happens before any web process messaging.

I verified that after the calls to AnimateWindow() in step 2 and 4, the fullscreen window we just animated showing is the active window, which in theory should force Windows to hide the taskbar.
Comment 3 Jeff Miller 2011-05-26 15:12:15 PDT
I tried replacing the calls to AnimateWindow() with SetWindowPos(), but this bug still occurs.
Comment 4 Jeff Miller 2011-05-26 15:12:30 PDT
Jer and I found a kind of ugly workaround for this that involves making the fullscreen window a topmost window before animating it. The task bar is still visible when the background window animates in, but it disappears when the real fullscreen window animates in.
Comment 5 Jeff Miller 2011-05-26 15:12:55 PDT
<rdar://problem/9454315>
Comment 6 Jeff Miller 2011-05-26 15:31:40 PDT
Created attachment 95056 [details]
Patch to make fullscreen window a topmost window.
Comment 7 WebKit Review Bot 2011-05-26 15:34:01 PDT
Attachment 95056 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/win/FullScreenController.cpp:89:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Steve Falkenburg 2011-05-26 15:36:11 PDT
Comment on attachment 95056 [details]
Patch to make fullscreen window a topmost window.

You should change wParam == 0 to !wParam
Comment 9 Jeff Miller 2011-05-26 15:41:06 PDT
I'll fix this before landing the patch.
Comment 10 Jeff Miller 2011-05-26 15:44:37 PDT
Committed <http://trac.webkit.org/changeset/87442>