Bug 56534

CCing Tim as author this problem, defect caused by
changes in NbPresenter:

date: 2004/08/21 14:59:40;  author: tboudreau;  state: Exp;  lines: +15 -2
Also sort options when checking result, or buttons are ordered correctly but do
the wrong things;
eliminate left side buttons on Aqua - mac dialogs don't ever put buttons there.
 Makes the find dialog
slightly taller, but much more natural seeming on Aqua.
----------------------------
revision 1.12
date: 2004/08/21 14:38:05;  author: tboudreau;  state: Exp;  lines: +57 -11
Button order in dialogs on the mac should always be with the default button
rightmost

Not sure my commit caused the problem;  until recently, buttons worked correctly, but I've noticed this 
problem in the last few weeks in, e.g., the Editor's find dialog.  I specifically tested that when I did the 
original commit and it worked fine.

Some other change in NbPresenter?

tim, they work the first time, when the dialog is reused, they get messed up
somehow. I've experienced that especially in the find&replace dialog for quite
some time (months not weeks). Clicking repeatedly on the find button caused "1
replace followed by 1 find" effect.
For a long time I thought it's a problem of editor, but I was able to reproduce
the problem in the Autoupdate's license confirmation dialog. If you decline the
first time, then any accept afterwards means decline and pushing decline lets
you install th software.

I can believe that.

But what does anybody think they're optimizing by *reusing* an instance of DialogDescriptor, or 
DialogDisplayer's return value, et. al. - the whole point of the API is one-shot use.  Granted it should 
work the same way if reused, and I might have caused it not to, but WTF?  Nobody should be holding 
onto a reference to a dialog created this way - one of the big reasons we have this API is so such 
dialogs can be GC'd if they JVM needs the space.  Use it, then lose it - that's the whole point.

There actually are a few interesting things we could do for performance with dialog caching, just 
because creating a window context is a fairly expensive operation on most OSs - I prototyped some 
stuff around that a year or so ago.  But under no circumstances should it ever be up to the application 
layer to cache dialogs for some imagined performance benefit - if there's a benefit to be had, the 
framework should do it transparently.

If you really can't find all the instances of DD reuse, okay, it's fixable, but I think the bug is trying to 
reuse them in the first place - that's not what they're designed for.

>But what does anybody think they're optimizing by *reusing* an instance of
>DialogDescriptor, or 
>DialogDisplayer's return value, et. al. - the whole point of the API is
one-shot >use.

given that we have a createDialog() method on the DialogDescriptor, your
assumption is not 100% correct. there's a note in the javadoc of the method that
one should not cache modal dialogs, but find for example is not modal.

plus in the find&replace dialogs there some other issue than dialog reuse,
because the buttons switch places during one invokation of the dialog.

Hmm. Well, there's an array of buttons and an array of command return values that go with them.  My 
patch which Jirka cites was the second of two - the first to order them appropriately for the mac, the 
second to ensure the array of commands was similarly reordered.  There must be some place where the 
original array/order gets restored.

Added the test which demonstrates this problem. Good start for fixing :-)


Checking in windows/src/org/netbeans/core/windows/services/NbPresenter.java;
/cvs/core/windows/src/org/netbeans/core/windows/services/NbPresenter.java,v  <--
 NbPresenter.java
new revision: 1.20; previous revision: 1.19
done
RCS file:
/cvs/core/windows/test/unit/src/org/netbeans/core/windows/services/NbPresenterTest.java,v
done
Checking in
windows/test/unit/src/org/netbeans/core/windows/services/NbPresenterTest.java;
/cvs/core/windows/test/unit/src/org/netbeans/core/windows/services/NbPresenterTest.java,v
 <--  NbPresenterTest.java
initial revision: 1.1
done

Waiting for review issue 56878.

Fixed by new methods ND.getDefaultValue() which has been introduced in patch 56878.


Checking in core/windows/src/org/netbeans/core/windows/services/NbPresenter.java;
/cvs/core/windows/src/org/netbeans/core/windows/services/NbPresenter.java,v  <--
 NbPresenter.java
new revision: 1.22; previous revision: 1.21
done
Checking in
core/windows/test/unit/src/org/netbeans/core/windows/services/NbPresenterTest.java;
/cvs/core/windows/test/unit/src/org/netbeans/core/windows/services/NbPresenterTest.java,v
 <--  NbPresenterTest.java
new revision: 1.5; previous revision: 1.4
done

v/c