<AWT Dev> [7u4] Review request for 7145818: [macosx] dialogs not showing when JFrame is in full screen mode

[Anthony Petrov]

Could I get a review for the latest version of the fix please? Here's 
the link for your convenience:

http://cr.openjdk.java.net/~anthony/x-21-popupInFullscreen-7145818.3/

--
best regards,
Anthony

On 3/5/2012 8:03 PM, Anthony Petrov wrote:
> Hi Mike,
> 
> A few points:
> 
> * When a user enters the full screen using the OS widget, the full 
> screen state isn't sync'd with Java's GraphicsDevice.getFullScreenWindow().
> 
> * Rather than calling native, I'd actually update the flag upon 
> receiving native notifications about entering/leaving the FS mode.
> 
> * If you call -toggleFullScreen: for a window w/o the 
> NSWindowCollectionBehaviorFullScreenPrimary collection behavior, then 
> the menus and the dock stay visible. Apart from that, there's no the 
> default 10.7-specific animation for such windows when entering the FS mode.
> 
> * If I mark an undecorated window as fullscreenable, its original size 
> won't be restored when exiting the FS mode. So we can't simply enable 
> the behavior automatically for all windows. An app must opt in.
> 
> * Apple JDK 1.6 won't let a user exit a FS Java application, too. So at 
> least we don't introduce a regression here. Also, the full screen is a 
> restricted functionality that's unavailable to unsigned code by default. 
> Thus we generally trust code that's able to enter the mode (in that it 
> should let user out when needed).
> 
> * The fix we're currently reviewing is supposed to fix a bug with popup 
> windows not showing up for a Java app that doesn't use EAWT at all. As 
> far as I've tested it, it does resolve the issue. And the behavior seems 
> to be quite consistent with what Apple JDK 1.6 does.
> 
> * The only thing the fix worsens is for EAWT-aware app: it hides the 
> menu bar unconditionally when entering the FS mode. I've just fixed that 
> by avoiding hiding the menu bar if a window is marked as FULLSCREENABLE. 
> I believe that a new fix at
> 
> http://cr.openjdk.java.net/~anthony/x-21-popupInFullscreen-7145818.3/
> 
> is what we need in 7u4. We may file a new change request to address 
> EAWT-specific issues in a future release.
> 
> Please review.
> 
> -- 
> best regards,
> Anthony
> 
> On 03/02/12 21:13, Mike Swingler wrote:
>> There are a few points here:
>> * If the user marks the window as "fullscreenable" using the eAWT API,
>> that puts a widget in the upper right corner of the window which allows
>> the user to toggle it into full screen themselves.
>> * When in full screen (using AWT or eAWT API), the user should be able
>> to click the blue exit fullscreen button in the menu bar at any time (if
>> they entered into full screen themselves)
>> * Generally, it is undesirable for the user to be stuck in a state where
>> they cannot return to the desktop.
>>
>> With the current diff, the isFullScreenMode boolean in Java will
>> definitely be out of sync if the user initiates either of these actions.
>>
>> Instead of trying to keep the knowledge of the full screen state in Java
>> behind a lock, I think you probably need query native to see if the
>> window is in the fullscreen state. I've used the following testcase to
>> test the existing eAWT fullscreen API, and I think you'll find it's easy
>> to get into a state where the user is stuck in windowed mode without a
>> menu bar, and that sometimes the user is locked out from returning to
>> windowed mode when they clicked on the window widget.
>>
>> After trying out your diff, I actually found it quite frustrating that
>> the menubar was suppressed and I *had* to use the four-finger swipe
>> gesture to get back to my desktop. How would you feel about just
>> dropping the whole menubar hiding stuff? Since the menu bar is
>> auto-hidden by default, users don't get to it unless they push up to the
>> top of the screen anyway.
>>
>> Try out this test case, and be sure to use the window widget, as well as
>> the button that programmatically fires the eAWT API.
>>
>> import java.awt.*;
>> import java.awt.event.ActionEvent;
>> import javax.swing.*;
>>
>> import com.apple.eawt.AppEvent.FullScreenEvent;
>> import com.apple.eawt.*;
>>
>> public class FullScreenTest {
>> public static void main(String args[]) {
>> SwingUtilities.invokeLater(new Runnable() {
>> public void run() { createFrame(); }
>> });
>> }
>>
>>
>> static void createFrame() {
>> JFrame frame = new JFrame();
>> frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>> frame.setTitle(System.getProperty("java.version"));
>>
>>
>> Container contentPane = frame.getContentPane();
>> contentPane.setLayout(new BorderLayout());
>> contentPane.add(createPanel(), BorderLayout.CENTER);
>>
>>
>> // turns on the fullscreen window titlebar widget in the upper right 
>> corner
>> FullScreenUtilities.setWindowCanFullScreen(frame, true);
>>
>>
>> // useful for re-adjusting content or hiding/showing palette windows
>> FullScreenUtilities.addFullScreenListenerTo(frame, new 
>> FullScreenAdapter() {
>> public void windowExitingFullScreen(FullScreenEvent e) {
>> System.out.println("exiting");
>> }
>> public void windowExitedFullScreen(FullScreenEvent e) {
>> System.out.println("exited");
>> }
>> public void windowEnteringFullScreen(FullScreenEvent e) {
>> System.out.println("entering");
>> }
>> public void windowEnteredFullScreen(FullScreenEvent e) {
>> System.out.println("entered");
>> }
>> });
>>
>>
>> frame.pack();
>> frame.setVisible(true);
>> }
>>
>> static Component createPanel() {
>> final JPanel panel = new JPanel(new FlowLayout());
>>
>>
>> // toggle FullScreen from a toolbar button
>> panel.add(new JButton(new AbstractAction("Full Screen Me!") {
>> public void actionPerformed(ActionEvent e) {
>> Application.getApplication().requestToggleFullScreen(
>> (Window)panel.getTopLevelAncestor());
>> }
>> }));
>>
>>
>> return panel;
>> }
>> }
>>
>> Regards,
>> Mike Swingler
>> Apple Inc.
>>
>> On Mar 2, 2012, at 6:42 AM, Anthony Petrov wrote:
>>
>>> 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).
>>