Skip to content

#5987: Use the currently active dialog as the presenter parent #5989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
neilcsmith-net merged 1 commit into from
Jul 18, 2023
Merged

#5987: Use the currently active dialog as the presenter parent #5989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 37 additions & 73 deletions platform/core.windows/src/org/netbeans/core/windows/services/DialogDisplayerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import javax.swing.SwingUtilities;
import org.netbeans.core.windows.view.ui.DefaultSeparateContainer;
import org.openide.util.Lookup;
import org.openide.util.Utilities;
import org.openide.util.lookup.ServiceProvider;

/**
Expand Down Expand Up @@ -95,53 +96,46 @@ public Dialog createDialog (final DialogDescriptor d, final Frame preferredParen
}
return Mutex.EVENT.readAccess (new Mutex.Action<Dialog> () {
public Dialog run () {
// if a modal dialog active use it as parent
// otherwise use the main window
if (NbPresenter.currentModalDialog != null) {
NbDialog dlg;
if (NbPresenter.currentModalDialog.isLeaf ()) {
dlg = new NbDialog(d, WindowManager.getDefault ().getMainWindow ());
} else {
dlg = new NbDialog(d, NbPresenter.currentModalDialog);
}
customizeDlg(dlg);
return dlg;
}
else {
Window w = preferredParent;
if( null == w ) {
w = KeyboardFocusManager.getCurrentKeyboardFocusManager ().getActiveWindow ();
if (!(w instanceof NbPresenter) || !w.isVisible()) {
// undocked window is not instanceof NbPresenter although it's NetBeans's native window
// all docked windows implements ModeUIBase interface
if (! (w instanceof DefaultSeparateContainer.ModeUIBase)) {
Container cont = SwingUtilities.getAncestorOfClass(Window.class, w);
if (cont instanceof DefaultSeparateContainer.ModeUIBase) {
w = (Window) cont;
} else {
// don't set non-ide window as parent
w = WindowManager.getDefault ().getMainWindow ();
}
Window w = preferredParent;
if( null == w ) {
w = findDialogParent();
Copy link
Member

@neilcsmith-net neilcsmith-net Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a bug here? The old code doesn't use preferredParent if there's a modal dialog. Should the preferred parent be passed through to Utilities via findDialogParent ?

Copy link
Contributor Author

@rkeen-siemens rkeen-siemens Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, if there is an active modal dialog and it is not preferredParent. How about validating preferredParent?

Window w = preferredParent;
if (w != null) {
    // Ensure the preferred parent is valid
    Component p = Utilities.findDialogParent(w, () -> null);
    if (p != w) {
        w = null;
    }
}
if (w == null) {
    // Existing logic from 101 onward
}

Copy link
Member

@neilcsmith-net neilcsmith-net Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or update and call through to findDialogParent() in this class -

private Window findDialogParent(Window preferred) {
        Component parentComponent = Utilities.findDialogParent(preferred);
    ...

I was wondering if we could get rid of the null check, and just do -

Window w = findDialogParent(preferredParent);

But I think that only makes sense if we removed the checks for NbPresenter etc. What are the chances we have an active modal dialog that is filtered out by that check?

Copy link
Contributor Author

@rkeen-siemens rkeen-siemens Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started going down that route and testParent failed because it expected the input frame as the parent which it didn't get since the frame was not visible. After overriding that in the test method, it fell through to the other logic which would have it return the main window unless I customized the input frame further. Since I don't really understand the extra logic and didn't want to change the behavior I thought it would be best to just use the preferredParent unless it was not valid.

Copy link
Member

@neilcsmith-net neilcsmith-net Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rkeen-siemens well, I've made the change as you suggested here and testParent still failed - seems to be another test leaving a modal dialog open. I noticed it was passing when run by itself.

Have opened #6216 which fixes that, changes the Utilities API, and updates a couple of docs.

if (!(w instanceof NbPresenter) || !w.isVisible()) {
// undocked window is not instanceof NbPresenter although it's NetBeans's native window
// all docked windows implements ModeUIBase interface
if (! (w instanceof DefaultSeparateContainer.ModeUIBase)) {
Container cont = SwingUtilities.getAncestorOfClass(Window.class, w);
if (cont instanceof DefaultSeparateContainer.ModeUIBase) {
w = (Window) cont;
} else {
// don't set non-ide window as parent
w = WindowManager.getDefault ().getMainWindow ();
}
} else if (w instanceof NbPresenter && ((NbPresenter) w).isLeaf ()) {
w = WindowManager.getDefault ().getMainWindow ();
}
}
NbDialog dlg;
if (w instanceof Dialog) {
dlg = new NbDialog(d, (Dialog) w);
} else {
Frame f = w instanceof Frame ? (Frame) w : WindowManager.getDefault ().getMainWindow ();
dlg = new NbDialog(d, f);
}
customizeDlg(dlg);
dlg.requestFocusInWindow ();
return dlg;
}
NbDialog dlg = new NbDialog(d, w);
customizeDlg(dlg);
dlg.requestFocusInWindow ();
return dlg;
}
});
}

private Window findDialogParent() {
Component parentComponent = Utilities.findDialogParent(null, WindowManager.getDefault()::getMainWindow);
if (parentComponent instanceof Window) {
return (Window) parentComponent;
}
Window parent = null;
if (parentComponent != null) {
parent = SwingUtilities.windowForComponent(parentComponent);
}
if (parent == null || parent instanceof NbPresenter && ((NbPresenter) parent).isLeaf ()) {
return WindowManager.getDefault().getMainWindow();
}
return parent;
}

/** Notifies user by a dialog.
* @param descriptor description that contains needed informations
* @return the option that has been choosen in the notification.
Expand Down Expand Up @@ -222,43 +216,13 @@ public void showDialog () {
while ((win != null) && (!(win instanceof Window))) win = win.getParent ();
if (win != null) focusOwner = ((Window)win).getFocusOwner ();

// if a modal dialog is active use it as parent
// otherwise use the main window

NbPresenter presenter;
Window parent = noParent ? null : findDialogParent();

if (descriptor instanceof DialogDescriptor) {
if (NbPresenter.currentModalDialog != null) {
if (NbPresenter.currentModalDialog.isLeaf ()) {
presenter = new NbDialog((DialogDescriptor) descriptor, WindowManager.getDefault ().getMainWindow ());
} else {
presenter = new NbDialog((DialogDescriptor) descriptor, NbPresenter.currentModalDialog);
}
} else {
Window w = KeyboardFocusManager.getCurrentKeyboardFocusManager ().getActiveWindow ();
if (w instanceof NbPresenter && ((NbPresenter) w).isLeaf ()) {
w = WindowManager.getDefault ().getMainWindow ();
}
Frame f = w instanceof Frame ? (Frame) w : WindowManager.getDefault().getMainWindow();
if (noParent) {
f = null;
}
presenter = new NbDialog((DialogDescriptor) descriptor, f);
}
presenter = new NbDialog((DialogDescriptor) descriptor, parent);
} else {
if (NbPresenter.currentModalDialog != null) {
if (NbPresenter.currentModalDialog.isLeaf()) {
presenter = new NbPresenter(descriptor, WindowManager.getDefault().getMainWindow(), true);
} else {
presenter = new NbPresenter(descriptor, NbPresenter.currentModalDialog, true);
}
} else {
Window w = KeyboardFocusManager.getCurrentKeyboardFocusManager ().getActiveWindow ();
Frame f = w instanceof Frame ? (Frame) w : WindowManager.getDefault().getMainWindow();
if (noParent) {
f = null;
}
presenter = new NbPresenter(descriptor, f, true);
}
presenter = new NbPresenter(descriptor, parent, Dialog.DEFAULT_MODALITY_TYPE);
}

synchronized (this) {
Expand Down
10 changes: 9 additions & 1 deletion platform/core.windows/src/org/netbeans/core/windows/services/NbDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,15 @@ public NbDialog (DialogDescriptor d, Dialog owner) {
super (d, owner, d.isModal ());
}

/** Geter for help.
/** Creates a new Dialog from specified DialogDescriptor
* @param d The DialogDescriptor to create the dialog from
* @param owner Owner of this dialog.
*/
public NbDialog(DialogDescriptor d, Window owner) {
super(d, owner, d.isModal() ? Dialog.DEFAULT_MODALITY_TYPE : ModalityType.MODELESS);
}

/** Getter for help.
*/
@Override
protected HelpCtx getHelpCtx () {
Expand Down
48 changes: 22 additions & 26 deletions platform/core.windows/src/org/netbeans/core/windows/services/NbPresenter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import java.awt.DefaultKeyboardFocusManager;
import java.awt.Dialog;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.FlowLayout;
import java.awt.Frame;
import java.awt.GraphicsDevice;
Expand Down Expand Up @@ -89,7 +88,6 @@
import org.openide.NotifyDescriptor;
import org.openide.WizardDescriptor;
import org.openide.awt.Mnemonics;
import org.openide.util.ChangeSupport;
import org.openide.util.HelpCtx;
import org.openide.util.ImageUtilities;
import org.openide.util.Mutex;
Expand All @@ -106,9 +104,6 @@
class NbPresenter extends JDialog
implements PropertyChangeListener, WindowListener, Mutex.Action<Void>, Comparator<Object> {

/** variable holding current modal dialog in the system */
public static NbPresenter currentModalDialog;
private static final ChangeSupport cs = new ChangeSupport(NbPresenter.class);
private static Boolean isJava17 = null;

protected NotifyDescriptor descriptor;
Expand Down Expand Up @@ -189,6 +184,21 @@ public NbPresenter(NotifyDescriptor d, Dialog owner, boolean modal) {
initialize(d);
}

/**
* Creates a new Dialog from the specified NotifyDescriptor and owner.
*
* @param d the non-null descriptor from which to initialize the dialog
* @param owner the owner of the dialog, must be a {@code Dialog} or
* {@code Frame} instance or {@code null} (not recommended)
* @param modality specifies whether dialog blocks input to other windows
* when shown. {@code null} value and unsupported modality types are
* equivalent to {@code MODELESS}
*/
public NbPresenter(NotifyDescriptor d, Window owner, ModalityType modality) {
super(owner, d.getTitle(), modality);
initialize(d);
}

boolean isLeaf () {
return leaf;
}
Expand Down Expand Up @@ -1092,6 +1102,7 @@ public void run () {
}
}

@Override
public Void run() {
doShow();
return null;
Expand All @@ -1108,30 +1119,21 @@ private void doShow () {
gd.setFullScreenWindow( null );
}
}
NbPresenter prev = null;
try {
MenuSelectionManager.defaultManager().clearSelectedPath();
} catch( NullPointerException npE ) {
//#216184
LOG.log( Level.FINE, null, npE );
}
if (isModal()) {
prev = currentModalDialog;
currentModalDialog = this;
fireChangeEvent();
}

superShow();

if( null != fullScreenWindow )
if( null != fullScreenWindow ) {
getGraphicsConfiguration().getDevice().setFullScreenWindow( fullScreenWindow );

if (currentModalDialog != prev) {
currentModalDialog = prev;
fireChangeEvent();
}
}

@Override
public void propertyChange(final java.beans.PropertyChangeEvent evt) {
if( !SwingUtilities.isEventDispatchThread() ) {
SwingUtilities.invokeLater(new Runnable() {
Expand Down Expand Up @@ -1274,19 +1276,13 @@ public void windowClosing(final java.awt.event.WindowEvent p1) {
public void windowActivated(final java.awt.event.WindowEvent p1) {
}

// Used by JavaHelp:
@Deprecated
public static void addChangeListener(ChangeListener l) {
cs.addChangeListener(l);
// Does nothing
}
@Deprecated
public static void removeChangeListener(ChangeListener l) {
cs.removeChangeListener(l);
}
private static void fireChangeEvent() {
EventQueue.invokeLater(new Runnable() {
public void run() {
cs.fireChange();
}
});
// Does nothing
}

private final class EscapeAction extends AbstractAction {
Expand Down
8 changes: 2 additions & 6 deletions platform/core.windows/src/org/netbeans/core/windows/services/NodeOperationImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.openide.util.Mutex;
import org.openide.util.NbBundle;
import org.openide.util.UserCancelException;
import org.openide.util.Utilities;
import org.openide.util.WeakSet;
import org.openide.util.lookup.ServiceProvider;
import org.openide.windows.Mode;
Expand Down Expand Up @@ -350,12 +351,7 @@ private static void openProperties (final NbSheet sheet, final Node[] nds) {
// Mutex.EVENT.readAccess (new Runnable () { // PENDING
javax.swing.SwingUtilities.invokeLater(new Runnable() {
public void run () {
boolean modal;
if(NbPresenter.currentModalDialog == null) {
modal = false;
} else {
modal = true;
}
boolean modal = Utilities.isModalDialogOpen();

Dialog dlg = org.openide.DialogDisplayer.getDefault().createDialog(new DialogDescriptor (
sheet,
Expand Down
30 changes: 25 additions & 5 deletions ...ore.windows/test/unit/src/org/netbeans/core/windows/services/DialogDisplayerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.awt.Dialog;
import java.awt.EventQueue;
import java.awt.Frame;
import java.awt.KeyboardFocusManager;
import java.awt.Window;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -81,10 +82,13 @@ protected void setUp() throws Exception {

@Override
protected void tearDown() throws Exception {
if (NbPresenter.currentModalDialog != null) {
NbPresenter.currentModalDialog.setVisible(false);
NbPresenter.currentModalDialog.dispose();
NbPresenter.currentModalDialog = null;
while (true) {
Window w = KeyboardFocusManager.getCurrentKeyboardFocusManager().getActiveWindow();
if (w == null) {
break;
}
w.setVisible(false);
w.dispose();
}
super.tearDown();
}
Expand Down Expand Up @@ -347,6 +351,22 @@ public void testParent() {
assertEquals(frame, dlg.getOwner());
}

@RandomlyFails
public void testNestedDialogParent() throws Exception {
Frame f = null;
Dialog owner = new Dialog(f, true);
postInAwtAndWaitOutsideAwt(() -> owner.setVisible(true));
assertShowing("Owner is invisible", true, owner);

child = new JButton();
final NotifyDescriptor nd = new NotifyDescriptor.Message(child);
postInAwtAndWaitOutsideAwt(() -> DialogDisplayer.getDefault().notify(nd));

assertShowing("Child is invisible", true, child);
Window w = SwingUtilities.windowForComponent(child);
assertSame("Window parent is not owner", owner, w.getParent());
}

static void postInAwtAndWaitOutsideAwt (final Runnable run) throws Exception {
// pendig to better implementation
SwingUtilities.invokeLater (run);
Expand Down Expand Up @@ -444,7 +464,7 @@ public void testUserCancelClosesDialog() throws Exception {
thenApply((x) -> x);

// wait for the dialog to be displayed
panel.displayed.await(10, TimeUnit.SECONDS);
assertTrue(panel.displayed.await(10, TimeUnit.SECONDS));

cf.cancel(true);

Expand Down
Loading