Bug 61569 - Fullscreen content is sometimes obscured by taskbar (which even covers up the "exit fullscreen" button)
Summary: Fullscreen content is sometimes obscured by taskbar (which even covers up the...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Jeff Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-05-26 15:10 PDT by Jeff Miller
Modified: 2011-05-26 15:44 PDT (History)
3 users (show)

See Also:


Attachments
Patch to make fullscreen window a topmost window. (2.79 KB, patch)
2011-05-26 15:31 PDT, Jeff Miller
sfalken: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>