Bug 40933 – Playing movie full screen on second monitor hides menu bar and title bar on main monitor

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jer Noble]

Click the 'expand' button in the lower right, while window is on second monitor. The movie expands to fill the screen, but the dock and title bar on the main monitor hide themselves for no reason.

[Jer Noble]

<rdar://problem/7858452>

[Jer Noble]

Created attachment 59281 [details]
Patch

[Eric Seidel (no email)]

Attachment 59281 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/3340575

[Jer Noble]

Created attachment 59293 [details]
Patch

[Darin Adler]

Comment on attachment 59293 [details]
Patch

> -    SetSystemUIMode(_savedUIMode, _savedUIOptions);
> +    [self updateMenuAndDockForFullscreen];   

You have some extra trailing spaces on this line of code.

> +    // NSApplicationPresentationOptions is available on > 10.6 only:

This comment says > 10.6, but the code says >= 10.6.

> +        if ([[NSScreen screens] objectAtIndex:0] == fullscreenScreen)
> +            options |= (NSApplicationPresentationAutoHideMenuBar | NSApplicationPresentationAutoHideDock);

I don't think you need those parentheses.

> +    if ([NSApp respondsToSelector:@selector(setPresentationOptions:)])
> +        [NSApp setPresentationOptions:options];
> +    else
> +#endif
> +        SetSystemUIMode(_isEndingFullscreen ? kUIModeNormal : kUIModeAllHidden, 0);

I recommend using a "return" here to avoid the need for the else and the ugly need to have code outside the #if indented based on code inside the #if.

Is the respondsToSelector: call needed or helpful?

r=me

[Jer Noble]

(In reply to comment #5)
> (From update of attachment 59293 [details])
> > -    SetSystemUIMode(_savedUIMode, _savedUIOptions);
> > +    [self updateMenuAndDockForFullscreen];   
> 
> You have some extra trailing spaces on this line of code.

Drat.  Will remove.

> > +    // NSApplicationPresentationOptions is available on > 10.6 only:
> 
> This comment says > 10.6, but the code says >= 10.6.

I'll update the comment to match the code.

> > +        if ([[NSScreen screens] objectAtIndex:0] == fullscreenScreen)
> > +            options |= (NSApplicationPresentationAutoHideMenuBar | NSApplicationPresentationAutoHideDock);
> 
> I don't think you need those parentheses.

Will remove.

> > +    if ([NSApp respondsToSelector:@selector(setPresentationOptions:)])
> > +        [NSApp setPresentationOptions:options];
> > +    else
> > +#endif
> > +        SetSystemUIMode(_isEndingFullscreen ? kUIModeNormal : kUIModeAllHidden, 0);
> 
> I recommend using a "return" here to avoid the need for the else and the ugly need to have code outside the #if indented based on code inside the #if.

That's a good idea.  I've seen the "else on the other side of a #endif" idiom used elsewhere, but the early return works much better.

> Is the respondsToSelector: call needed or helpful?

Only if this code is built on Snow Leopard but run on Leopard.  

Thanks!

[Jer Noble]

Committed r61786: <http://trac.webkit.org/changeset/61786>