Always on top frames and dialogs BUG

koveloper commented 3 years ago

WebFrame and WebDialog with always on top function set to true loose this ability in case of any single fire of WebPopup placed on window. I found an crutch - makesetVisible(false) and setVisible(true) calls for window.

wnd.setAlwaysOnTop(true);

....

webPopup.addPopupListener(new PopupAdapter() {
    @Override
    public void popupClosed() {
        UI.ui.setVisible(false);
        UI.ui.setVisible(true);
    }
});

mgarin commented 3 years ago

I'm still not sure what exactly doesn't work:

WebFrame and WebDialog with always on top function set to true loose this ability in case of any single fire of WebPopup placed on window

What does "this ability" mean? Which ability? Of staying always on top? If so - it does seem to work correctly for me on latest WebLaF version.

Here is an example I've tried and it works properly for me:

public class Example
{
    public static void main ( final String[] args )
    {
        SwingTest.run ( () -> {
            final WebFrame<?> frame = new WebFrame<> ( "Frame" );
            frame.setAlwaysOnTop ( true );

            frame.setLayout ( new FlowLayout () );

            final JButton button = new JButton ( "Show popup" );
            button.addActionListener ( e -> {
                final WebPopup<?> popup = new WebPopup<> ();
                final JLabel label = new JLabel ( "Sample label", SwingConstants.CENTER );
                label.setBorder ( BorderFactory.createEmptyBorder ( 50, 50, 50, 50 ) );
                popup.add ( label );
                popup.setResizable ( true );
                popup.showPopup ( button, button.getWidth () / 2 - popup.getPreferredSize ().width / 2, button.getHeight () );
            } );
            frame.add ( button );

            frame.setSize ( 200, 200 );
            frame.setLocationRelativeTo ( null );
            frame.setDefaultCloseOperation ( WindowConstants.EXIT_ON_CLOSE );
            frame.setVisible ( true );
        } );
    }
}

Popup is displayed correctly above the frame:

Frame stays "always on top" even after popup being displayed/closed multiple times.

mgarin commented 3 years ago

Things that may help me to find the issue faster:

What is your WebLaF version?
What OS are you using to reproduce the bug?
What JDK/JVM version are you using to reproduce the bug?
Can you provide a SSCCE for the bug?

koveloper commented 3 years ago

Mikle, my english is not so good, so I decided to speak on russian with another russian man)). I make small video with sound. Take a look on it. My problem is that always on top window or dialog become an NON-always on top after any WebPopup open call...

https://user-images.githubusercontent.com/65438466/136666947-085f9fef-6eeb-4b45-bc00-efbba7191f5e.mp4

OS: Windows 10, JDK: 1.8.0_221 (64 bit)

mgarin commented 3 years ago

Thanks for the in-depth showcase!

I was able to reproduce and track down this issue and... it seems to be a weird interaction of non-always-on-top popup with always-on-top parent. The thing here is - even though in my example JFrame keeps always-on-top property set to true - it does indeed act like it is set to false. But it's even more complicated than that.

I'll list all the details below (mostly for myself to keep track of such nuance issues) and a TL;DR at the end.

Here are the things I've found while investigating this problem:

It seems that any Window inherits alwaysOnTop property from parent whenever possible. Here is a part of code from Window class source code (from JDK8):

private void ownedInit(Window owner) {
    this.parent = owner;
    if (owner != null) {
        owner.addOwnedWindow(weakThis);
        if (owner.isAlwaysOnTop()) {
            try {
                setAlwaysOnTop(true);
            } catch (SecurityException ignore) {
            }
        }
    }

    // WindowDisposerRecord requires a proper value of parent field.
    disposerRecord.updateOwner();
}

My WebPopup uses JWindow as a base for displaying popups, but does some adjustments, including resetting the alwaysOnTop setting during popup window initialization: https://github.com/mgarin/weblaf/blob/master/modules/ui/src/com/alee/extended/window/WebPopup.java#L802
```
// Updating window settings
window.setCloseOnFocusLoss ( isCloseOnFocusLoss () );
window.setAlwaysOnTop ( isAlwaysOnTop () );
```
As we now know from (1) - whenever you open a window (popup window in this case) with an always-on-top parent - it inherits the always-on-top property. The problem is that if that property is reset to false in the child window (popup in this case) that is already displayable (has native window peer initialized) then parent window seem to lose that property in it's own window peer, but Window component will still return that it is always-on-top. This seems like an oversight from whoever wrote the Window component and it's interaction with it's native peer.
An important detail that we can take away from (3) is that if we reset always-on-top property in the child window before it is made displayable (before it's native window peer is initialized) then issue will not appear and everything will work as expected. It is probably still incorrect since child's window native peer is probably always-on-top anyway due to parent being always-on-top, but from a user viewpoint it will work as expected.

Here is practical example in regards to (4): (I've written it on pure Swing with no WebLaF usage to highlight that this issue exists in Swing as well)

public class Example
{
    public static void main ( final String[] args )
    {
        SwingUtilities.invokeLater ( () -> {
            final JFrame frame = new JFrame ( "Frame" );
            frame.setAlwaysOnTop ( true );

            frame.setLayout ( new FlowLayout () );

            final JButton button = new JButton ( "Show popup" );
            button.addActionListener ( e -> {
                final JWindow popup = new JWindow ( frame );

                final JLabel label = new JLabel ( "Sample label", SwingConstants.CENTER );
                label.setBorder (
                        BorderFactory.createCompoundBorder (
                                BorderFactory.createLineBorder ( Color.BLACK ),
                                BorderFactory.createEmptyBorder ( 50, 50, 50, 50 )
                        )
                );
                popup.getContentPane ().add ( label );

                // Resetting it here won't reset it for JFrame
                popup.setAlwaysOnTop ( false ); // <---------- works as expected

                final Point los = CoreSwingUtils.locationOnScreen ( button );
                popup.pack ();

                // Resetting it here will reset it for JFrame because native window is now initialized via pack() call
                // popup.setAlwaysOnTop ( false ); // <---------- will cause parent window to lose always-on-top property

                popup.setLocation ( los.x + button.getWidth () / 2 - popup.getPreferredSize ().width / 2, los.y + button.getHeight () );
                popup.setVisible ( true );

                // Resetting it here will also reset it for JFrame because native window is now initialized via setVisible(true) call
                // popup.setAlwaysOnTop ( false ); // <---------- will cause parent window to lose always-on-top property

                System.out.println ( "frame.isAlwaysOnTop = " + frame.isAlwaysOnTop () );
                System.out.println ( "popup.isAlwaysOnTop = " + popup.isAlwaysOnTop () );

            } );
            frame.add ( button );

            frame.setSize ( 200, 200 );
            frame.setLocationRelativeTo ( null );
            frame.setDefaultCloseOperation ( WindowConstants.EXIT_ON_CLOSE );
            frame.setVisible ( true );
        } );
    }
}

There are 3 lines here that change popup's always-on-top property - if you uncomment and try each one of them separately (other two need to be commented out) you will see that first one doesn't cause an issue and other two do.

Displayability

I've mentioned in the notes above that updating the always-on-top property will only cause issues when Window is already "displayable" and I wanted to explain that part separately a bit more -

Window becomes "displayable" by initializing it's native WindowPeer which you can see in addNotify() method in Window:

    /**
     * Makes this Window displayable by creating the connection to its
     * native screen resource.
     * This method is called internally by the toolkit and should
     * not be called directly by programs.
     * @see Component#isDisplayable
     * @see Container#removeNotify
     * @since JDK1.0
     */
    public void addNotify() {
        synchronized (getTreeLock()) {
            Container parent = this.parent;
            if (parent != null && parent.getPeer() == null) {
                parent.addNotify();
            }
            if (peer == null) {
                peer = getToolkit().createWindow(this);
            }
            synchronized (allWindows) {
                allWindows.add(this);
            }
            super.addNotify();
        }
    }

This can normally be called only via two methods in Window class - setVisible( true ) and pack(). Each of them will initialize native window peer and make it displayable.

Why it matters in the context of this issue is because once Window is made displayable - all property changes are passed through to native window component as well to reflect the changes - like making a window resizable or making it always-on-top.

I can only guess that native code part of WindowPeer for Windows OS passes the always-on-top property change to window parent(s) as well which causes this whole issue and seemingly inconsistent property value in Window component.

I'll try to check whether this issue persists on other Java versions later, but it is definitely there on all (seemingly) Java 8 versions. If it exists in later versions as well - I will probably submit a JDK bug report based on this find.

TL;DR

This issue may happen in common Swing as well, but usually won't because you usually don't change always-on-top property for popups and it is simply inherited from parent by default.

So this is definitely a WebLaF problem, specifically with how WebPopup updates it's always-on-top property when it is shown which in its turn affects parent window always-on-top property on the native level.

I'll fix this by adjusting order of operations during WebPopup display in the closest update.

Meanwhile, the easiest fix you can do - simply set the WebPopup's always-on-top to true, example: (make sure to do that before popup is displayed or packed)

public class Example
{
    public static void main ( final String[] args )
    {
        SwingTest.run ( () -> {
            final WebFrame<?> frame = new WebFrame<> ( "Frame" );
            frame.setAlwaysOnTop ( true );

            frame.setLayout ( new FlowLayout () );

            final JButton button = new JButton ( "Show popup" );
            button.addActionListener ( e -> {
                final WebPopup<?> popup = new WebPopup<> ();
                popup.setAlwaysOnTop ( true );
                final JLabel label = new JLabel ( "Sample label", SwingConstants.CENTER );
                label.setBorder ( BorderFactory.createEmptyBorder ( 50, 50, 50, 50 ) );
                popup.add ( label );
                popup.setResizable ( true );
                popup.showPopup ( button, button.getWidth () / 2 - popup.getPreferredSize ().width / 2, button.getHeight () );
            } );
            frame.add ( button );

            frame.setSize ( 200, 200 );
            frame.setLocationRelativeTo ( null );
            frame.setDefaultCloseOperation ( WindowConstants.EXIT_ON_CLOSE );
            frame.setVisible ( true );
        } );
    }
}

mgarin / weblaf