Full-screen: Don't change the collectionBehavior of the WebView's NSWindow if not necessary
Created attachment 98293 [details] Patch
<rdar://problem/9660291>
Comment on attachment 98293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98293&action=review > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:332 > +#if !defined(BUILDING_ON_LEOPARD) > + if (![webWindow isOnActiveSpace]) { > +#endif Have you considered encapsulating this functionality into a static function, say isWindowOnActiveSpace(NSWindow*) or windowIsOnActiveSpace(NSWindow*), that is hardcoded to return false when building on Leopard and sends the specified NSWindow an isOnActiveSpace: message when we aren't building on Leopard?
WebKit2 isn’t built on Leopard so the #if’s aren't necessary.
@Daniel(In reply to comment #3) > (From update of attachment 98293 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98293&action=review > > > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:332 > > +#if !defined(BUILDING_ON_LEOPARD) > > + if (![webWindow isOnActiveSpace]) { > > +#endif > > Have you considered encapsulating this functionality into a static function, say isWindowOnActiveSpace(NSWindow*) or windowIsOnActiveSpace(NSWindow*), that is hardcoded to return false when building on Leopard and sends the specified NSWindow an isOnActiveSpace: message when we aren't building on Leopard? Apparently, since WebKit2 isn't built on Leopard, I can just remove those checks entirely. ;-)
Comment on attachment 98293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98293&action=review >>> Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:332 >>> +#endif >> >> Have you considered encapsulating this functionality into a static function, say isWindowOnActiveSpace(NSWindow*) or windowIsOnActiveSpace(NSWindow*), that is hardcoded to return false when building on Leopard and sends the specified NSWindow an isOnActiveSpace: message when we aren't building on Leopard? > > Apparently, since WebKit2 isn't built on Leopard, I can just remove those checks entirely. ;-) That sounds even better.
Created attachment 100370 [details] Patch
Comment on attachment 100370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100370&action=review > Source/WebKit2/ChangeLog:9 > + collectionBehavior. The isOnActiveSpace API was added in 10.6, so skip this check on Leopard. This comment is now out of date. > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:330 > + if (![webWindow isOnActiveSpace]) { This check needs a why comment.
(In reply to comment #8) > (From update of attachment 100370 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=100370&action=review > > > Source/WebKit2/ChangeLog:9 > > + collectionBehavior. The isOnActiveSpace API was added in 10.6, so skip this check on Leopard. > > This comment is now out of date. I thought I squashed that comment. Will remove. > > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:330 > > + if (![webWindow isOnActiveSpace]) { > > This check needs a why comment. The current comment says: // The user may have moved the fullScreen window in Spaces, so temporarily change // the collectionBehavior of the webView's window: I'll change this to: // If the user has moved the fullScreen window into a new space, temporarily change // the collectionBehavior of the webView's window so that it is pulled into the active space:
Committed r90851: <http://trac.webkit.org/changeset/90851>