This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 47857 - IndexedPropertyEditor/ComboInplaceEditor do not properly invoke row specific custom editor
Summary: IndexedPropertyEditor/ComboInplaceEditor do not properly invoke row specific ...
Status: CLOSED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Explorer (show other bugs)
Version: 3.x
Hardware: All All
: P1 blocker (vote)
Assignee: _ tboudreau
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-08-25 06:35 UTC by Michael Frisino
Modified: 2008-12-22 23:53 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
testcase Foo.java (6.61 KB, text/plain)
2004-08-25 06:38 UTC, Michael Frisino
Details
IllegalStateException trace (7.79 KB, text/plain)
2004-08-25 06:39 UTC, Michael Frisino
Details
DefaultButtonModel trace of problem (21.11 KB, text/plain)
2004-08-25 15:35 UTC, Matthew Stevens
Details
DefaultButtonModel with latency shows working trace (28.63 KB, text/plain)
2004-08-25 15:37 UTC, Matthew Stevens
Details
Some patches to try (29.65 KB, patch)
2004-08-25 17:44 UTC, _ tboudreau
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Frisino 2004-08-25 06:35:14 UTC
The bug as is exists for the user is as follows:
If the user in the course of editing an indexed
property encounters the
org.openide.explorer.propertysheet.IndexedPropertyEditor
then there is quite a bit of strange behavior when
it comes to invoking the custom editor of the in
place editor. This is most easy to see if the in
place editor is
org.openide.explorer.propertysheet.ComboInplaceEditor,
but it may also pertain to other types, I have not
exhuastively tested other types.

For instance take the following class Foo that I
have attached.
It has several properties the most interesting of
which is a
public java.awt.Color[] getC()
public void setC(java.awt.Color[] c)

Open NB 3.6 and mount the filesystem with Foo.java
Compile it.
Select the "Customize Bean" action.
This brings up the IndexedPropertyEditor.
Add a row.
The row specific in place editor is now
ComboInplaceEditor which in turn will invoke the
org.netbeans.beaninfo.editors.ColorEditor.
The initial value of row 0 in the index editor
says "null".
Click on the custom editor button for row 0.
On my Win XP system with out of the box NB 3.6, I
immediately get an IllegalStateException
java.lang.IllegalStateException: WARNING: Going
from readAccess to writeAccess, see #10778:
http://www.netbeans.org/issues/show_bug.cgi?id=10778
    at org.openide.util.Mutex.enter(Mutex.java:436)
(full stack trace at bottom)
Acknowledge the error dialog.
Observe the value of the in place editor has
changed from "null" to "white", but I have not yet
seen the custom editor, all I did was select the
custom editor button. That is oddity number one.
Now, Click on the custom editor button for row 0 a
second time.
Observe, the
org.netbeans.beaninfo.editors.ColorEditor pops up
as one would expect, though it is one click later
than one would expect.
Observe. Move the ColorEditor, aside and you will
see that while the ColorEditor is popped up,
behind it the ComboInplaceEditor tags are fully
dropped down. That is oddity number two.

NOTE - If I repeat the entire steps above in
Studio Bow build, it behaves exactly the same
except we do not see the IllegalStateException. I
have not focused on that IllegalStateException
because I have been focused on the Studio version
of this same scenario and in the Studio build the
IllegalStateException does not show up.

Ok, that is the problem that you can reproduce and
observe in NB 3.6.
Now if that were the extent of the problem I would
say it was annoying to have to click twice, and
strange that the the combo changed from null to
"white" and strange that the combo drops down
behind the pop up. But, still it is functional. It
is buggy but perhaps not P1 buggy.

However, the same scenario in Studio where instead
of a indexed property of java.awt.Color we have an
indexed property of type
com.sun.jato.tools.sunone.common.ConfiguredBeanReference
 and a type specific editor of
com.sun.jato.tools.sunone.common.ConfiguredBeanReferenceEditor
the situation is a P1. In this case the user
experience is equally odd as the one described
above, but it is more broken. In short, the custom
editor ConfiguredBeanReferenceEditor only appears
when the row value is unset and the user presses
the custom editor button. Once the value of a
given row is set, subsequent attempts to revisit
the custom editor fail. The user can click and
click and click the custom editor button on that
row, and it is entirely ignored. This is the P1
bug. You cannot reproduce it without the Jato
module. I can show it to you over VNC.

However, my analysis suggests that the underlying
problem is probably some common bug(s) in the
org.openide.explorer.propertysheet code. The root
cause of the problem is probably the same, but the
manifestation of the problem is much worse in the
Jato module.

Now to the findings.

In digging into the problem, I resorted finally to
using an instance of NB 3.6 to debug an instance
of Studio Bow.

I started an instance of Studio Bow with the
necessary debug flags.
I started an instance of NB 3.6 and attached its
debugger to the Studio Bow.
Then I repeatedly test drove the scenarios i
descibed above in the instance of Studio Bow.
I set up some breakpoints in the debugger over in
the NB 3.6.
And ... the breakpoints changed the observable
behavior of the scenarios.
That is the context of the original blurb in the
prior email concerning the debugger.

As I set various breakpoints in the
org.openide.explorer.propertysheet classes like
org.openide.explorer.propertysheet.CustomEditorAction,
and  ComboInplaceEditor and even in the
javax.swing classes like
javax.swing.DefaultButtonModel I observed all
sorts of changes to the observable behavior of the
scenario I was test driving in the Studio.

For instance:
Observation 1 - on the first click of the row
specific custom editor button, we never even reach
CustomEditorAction.actionPerformed. Not for the
ColorEditor and not for the
ConfiguredBeanReferenceEditor.  This is what drove
me to putting break points deeper in the
org.openide.explorer.propertysheet code and
ultimately javax.swing.DefaultButtonModel

Observation 2 - putting a break point in the
constructor of  ComboInplaceEditor causes BOTH the
ColorEditor and the ConfiguredBeanReferenceEditor
to work PERFECTLY, without the oddities described
in the scenario. They respond to the first click,
there is no weird modifcation of the displayed
value (e.g. "null" changed to "white"), and the
drop down list does not appear strangely behind
the pop up. It is as if that one break point has
wiped out all of the problems.

Observation 3 - putting breaks elsewhere eliminate
some of the behavior. For example, as I indicated
in the prior email, an early observation was that:
If I put a break is in
javax.swing.DefaultButtonModel.setPressed(boolean
b). This is the method that is invoked once for a
button press and once for a button release. The
second time through, on the button release it
SHOULD trigger a fireActionPerformed (see below).
 But if I do not break AT OR BEFORE the
if(!isPressed() && isArmed()) in the body of this
method then it will 100% not enter into the body
of that if test, and will therefore not invoke the
fireActionPerformed. If I do set a break point AT
OR BEFORE, anywhere earlier in the call stack, or
earlier in the body of the setPressed method, then
the if(!isPressed() && isArmed()) condition seems
to work as expected and the fireActionPerformed is
invoked as expected.

Anyway, there are so many variations on this
weirdness that I am not sure it warrants further
detail. Suffice to say that the runtime
infrastructure seems very fragile and simply
putting a breakpoint into the code dramatically
changes the behavior that one is trying to
observe. It is pretty clear from the debugging
that I have done that the modifications to the
DefaultButtonModel "statemask" (i.e. ARMED,
SELECTED, PRESSED, ENABLED and ROLLOVER) are not
being applied in the expected order. That is why
the DefaultButtonModel.setPressed code does not
invoke fireActionPerformed  when one would expect
it to, and the net effect is that the button press
appears to be ignored by the system. Now, that is
perhaps just a symptom of the problem, not a
cause, because our working assumption is that the
real cause is some sort of bug(s) in
org.openide.explorer.propertysheet.*  

Unfortunately, while we have seen many strange
things we have not identified the root cause(s).
So I hope that this is sufficient for Tim to begin.
Comment 1 Michael Frisino 2004-08-25 06:38:39 UTC
Created attachment 17096 [details]
testcase Foo.java
Comment 2 Michael Frisino 2004-08-25 06:39:18 UTC
Created attachment 17097 [details]
IllegalStateException trace
Comment 3 Michael Frisino 2004-08-25 06:49:46 UTC
Argghhh, I hate that I cannot edit previous comments:

Correction to the test case scenario. Should read as follows;

Open NB 3.6 and mount the filesystem with Foo.java
Compile it.
Select the "Customize Bean" action.
This brings up the bean customizer.
Select the custom editor for the property "c".
This brings up the IndexedPropertyEditor.
Add a row.
...

rest of instructions are correct.
Comment 4 Michael Frisino 2004-08-25 06:59:22 UTC
I have read into the report from
http://www.netbeans.org/issues/show_bug.cgi?id=10778 and I believe it
is orthoganal to the primary problem described here. That is about
deadlock issues. I am not seeing deadlock. Furthermore, I am not
seeing that warning in our Studio Bow build, perhaps we have already
backported some code in prior issuezilla bugs that eliminated the
10778 from our Studio Bow codeline.
Comment 5 Matthew Stevens 2004-08-25 15:33:02 UTC
I worked with Mike on this problem for a couple of days.  While Mike
was working in the debugger to track down problem I worked on adding
some debug printlns into classes like DefaultButtonModel to try and
track order of event processing and codepaths to the state changes in
the button.  As Mike explained, in our problem situation where the
custom editor button is useless, we realized that our
fireActionPerformed() is never called and we tracked that down to a
pathlogic stateMask value change from 13->12.  When the
'mouseReleased" event is processed as a
<DefaultButtonModel>.setPressed(false) the fireActionPerformed() is
never called because the stateMask is '12' instead of '25'.  The
button only works if the <DefaultButtonModel>.setRollover(true) run
previous to the setPressed(false).  What our debugger and log
debugging efforts shows was that introduction of some latency would
yeild a parade of events and codepaths which worked.  Without an
artificially introduced latency we see out problem.

Just like Mike found when setting breakpoints....certain combinations
of additional debug println() in DefaultButtonModel could predictably
toggle the problem.  In other words, the slight slow down causes by
some println's would make the problem go away.  The fact that we can
toggle the problem paints the issue as a Race condition.

Again, my debug logging revealed that when the problem occurs we are
reaching the <DefaultButtonModel>.setPressed(false) codepath [which
should lead to the fireActionPerformed()] WITHOUT a preceeding
<DefaultButtonModel>.setRollover(true).

Our debug logging tactic was to print the DefaultButtonModel identity,
its stateMask value change and the name of the method being entered
(e.g. setPressed, setArmed) and on occassion drop a stack trace to see
where we were coming from.  I will attach some short log file "BAD and
GOOD" which highlight the differences in the call stacks.  Our
observation was from the DefaultButtonModel perspective (which is all
I was looking at and as such I may be just looking at collateral from
the root cause issue).  The difference between the working and not
working cases was difference after the initial setPressed(true) call
on the DefaultButtonModel.  With artifical latency setPressed(true) is
followed by a setRollover(true)   AND  the event processing leading to
this mouse event is a dispatch of an AWTEvent to a Window.  Without
the latency in the case which doesn't work the setPressed(true) is
followed by a setArmed(false)   AND the event processing leading to
this focus event is a dispatch of an AWTEvent to a container....not
the Window.  A very flimsy guess is that setPressed(true) which comes
from a codepath through

TreeTable$TreeTableUI$TreeTableMouseInputHandler

is racing against something and when a latency is introduced into the
system the winner of the race changes.  I am only picking on the NB
code here because its the only non-AWT/Swing code preceeding the
problem (from the DefaultButtonModel perspective).  Again all these
are observations we found picking away at this problem.  They may or
may not help you.
Comment 6 Matthew Stevens 2004-08-25 15:35:54 UTC
Created attachment 17137 [details]
DefaultButtonModel trace of problem
Comment 7 Matthew Stevens 2004-08-25 15:37:04 UTC
Created attachment 17138 [details]
DefaultButtonModel with latency shows working trace
Comment 8 _ tboudreau 2004-08-25 17:44:28 UTC
Created attachment 17148 [details]
Some patches to try
Comment 9 _ tboudreau 2004-08-25 17:48:07 UTC
BTW, I suspect the button model stuff has to do with focus - that is, 
clicking the button will give it focus, but there is already a 
pending focus event for the combo box, which runs afterwards;  so 
losing focus could change the state of the button's model, and also 
happen between the mouse pressed and mouse released events.  Not 
sure, about that, but there is a bunch of stuff going on when a table 
cell editor is added, wrt focus, which if added to by the 
machinations to make combo boxes work properly (as in, you click one 
combo and its popup opens; click another and its popup opens [in 
reality there is only one popup in the system]).  All of which is 
easy to do if you write your own UI, but not if you want something 
that works on all look and feels and works correctly on all of them.
Comment 10 _ tboudreau 2004-08-26 18:00:48 UTC
BTW, if the patches I provided are sufficient, it would be good to 
close this issue.
Comment 11 _ tboudreau 2004-08-31 09:43:14 UTC
Closing.
Comment 12 Tomas Danek 2005-07-18 12:04:14 UTC
Really closing:-)