A tiny typo has been fixed, a new webrev is at:
http://cr.openjdk.java.net/~anthony/x-21-popupInFullscreen-7145818.2/
--
best regards,
Anthony
On 3/2/2012 6:20 PM, Anthony Petrov wrote:
Thanks for your suggestions Mike. Here's the latest version of the fix:
http://cr.openjdk.java.net/~anthony/x-21-popupInFullscreen-7145818.1/
--
best regards,
Anthony
On 3/2/2012 3:14 AM, Mike Swingler wrote:
On Mar 1, 2012, at 1:10 PM, Anthony Petrov wrote:
Hi Mike,
Thanks for the review! Please see my comments inline.
On 3/1/2012 9:31 PM, Mike Swingler wrote:
On Feb 29, 2012, at 6:58 AM, Anthony Petrov wrote:
http://cr.openjdk.java.net/~anthony/x-21-popupInFullscreen-7145818.0/
A few points to consider:
* To protect against the unrecognized selector problem, you should
test if the AWTWindow object
-respondsToSelector:@selector(toggleFullScreen) before just calling it.
I'm aware of -respondsToSelector. But I suppose that in that case
this simply won't work on 10.6.8 at all. Since
a) presently it does work on 10.6.8 for reasons unknown, and
If OpenJDK is built on Snow Leopard, it works fine. I believe the
problem is the X11/FreeType versions in Lion are newer, and DYLD won't
load libraries linked against older versions. But that is going to
start me on my rant about how OpenJDK should bundle it's own FreeType...
b) we officially support 10.7+ only, hence the check makes little
sense in theory, and
Please think of OpenJDK, not just Oracle's proprietary binaries. There
are other users who currently compile on Snow Leopard and this is not
an inconvenience, since 10.7-only API is very rare in the JDK.
c) from ObjC perspective sending an unknown selector isn't an error,
but just a warning,
It is extremely poor form. The -respondsToSelector: check is very
cheap, and is very obvious what it is guarding against.
it seems to me that having this warning printed out to the console
(which isn't seen by 99% of users anyway) is OK when running on
10.6.8. Plus we get the full screen working there. Isn't it awesome?
We know there are developers and users who will be running OpenJDK on
Mac OS X 10.6.8, so it is perfectly reasonable to add this as an OS
guard. We should not actively damage our ability to run on 10.6 if
it's trivially avoidable.
* Also, there is already API that calls -toggleFullScreen in the
eAWT classes that you might not be aware of. You should probably
test for interactions with that, since apps can opt their window
into having a full screen widget icon and independently toggle
fullscreen.
Thanks for pointing this out. I'll rework this code to make sure
calls from EAWT update the boolean flag.
Great.
* In some cases, seeing the menubar is actually desirable, where as
in the "exclusive" mode, it's probably not. Perhaps you could
consult a client property on the window to determine if the menu bar
should be hidden?
Excellent idea! I think that by default the menu should be hidden
(for Java spec's sake). But if a client property is set, then the
menu would be visible.
Cool. There are several client property readers already on the root
AWT window that should be easily extendable.
I like this overall solution, because it uses the native platform
concept of full screen which doesn't trap the user from switching
spaces like the Java SE 6 implementation did.
We've noticed an interesting detail with -toggleFullScreen when it's
used in a multi-screen environment. In that case the window will go
full screen on the biggest monitor (actually we have a MacBook with
an external monitor connected.) The OS seems to ignore the screen
where the window were before entering the FS mode (the main notebook
display). Is this OK? Can we force it to use the same screen where
the window is originally located for the FS mode?
It's actually the monitor with the menu bar (the primary, as
designated in the Monitor's preference pane).
This is an issue with the OS, and should be filed at
<http://bugreporter.apple.com> (it's known, but if you have a specific
API suggestion to target a screen, or some sort of automatic behavior
in mind, it would be good to provide specific suggestions in the bug).
Thanks,
Mike Swingler
Apple Inc.
