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 35334 - Indexed property editing is broken
Summary: Indexed property editing is broken
Status: VERIFIED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Explorer (show other bugs)
Version: 3.x
Hardware: All All
: P1 blocker (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: REGRESSION
Depends on:
Blocks:
 
Reported: 2003-08-08 00:44 UTC by Todd Fast
Modified: 2008-12-23 11:14 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Patch for [release35] (put into lib/patches dir) (9.11 KB, application/zip)
2003-08-13 16:01 UTC, Peter Zavadsky
Details
diff of patch for [release35] (14.91 KB, patch)
2003-08-13 16:02 UTC, Peter Zavadsky
Details | Diff
Better fix for main trunk (4.87 KB, patch)
2003-08-20 15:13 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Todd Fast 2003-08-08 00:44:51 UTC
Indexed property editing support is broken in OpenAPI 
3.5/Sun ONE Studio 5.  Specifically, the index property 
editor and dialog combination,

org.openide.explorer.propertysheet.IndexedPropertyEditor
org.openide.explorer.propertysheet.IndexedEditorPanel

do not function to allow users to create an array to fill 
in a property value.  Instead, they actually invoke 
incorrect functions on the node that owns the property 
sheet.  This bug affects a core IDE feature and has a 
massive impact on ANY module that uses Nodes with indexed 
properties.  The result is that there is currently no way 
to fill in an indexed property value on a node.

To see what I mean, follow these steps:

1. In a filesystem, create a new empty Java file

2. Expand the file's Java class node and right-click on the 
Bean Patterns node.  Select Add -> Indexed Property.

3. In the dialog that appears, name the 
property "intArray", and set its type to "Integer".  Select 
all checkboxes on the dialog.  This results in a total of 
three checkboxes checked in the "Options" category, and 
four checkboxes checked in the "Non-Indexed Options" 
category.  Press OK.

4. Compile the bean.  When done, right-click on the bean 
class and select "Customize Bean...".

5. In the bean instance property dialog, select the "..." 
button on the intArray property.  The indexed property 
editor dialog appears.

6. Press the "New" button.  Instead of a new element in the 
array, a "New Class" dialog will appear.  This indicates 
that NewAction has been (incorectly) invoked on the bean 
class node because the first NewType on that node 
is "Class...".  We have verified this conclusion, see more 
below.

7. Press the "Move Up"/"Move Down" buttons, which are 
incorrectly enabled.  Note that the bean class node in the 
Explorer moves up and down in the file list!

We first saw this behavior in a copy of the 
IndexedPropertyEditor/IndexedEditorPanel classes that we 
copied from the OpenAPI 3.4 codebase into our module (to 
correct bugs).  When invoked in an Open API 3.5 
environment, our module's version of the editor operates 
exactly the same way as described above.  It invokes 
whatever the first NewType of the parent node is, and moves 
the original node up and down in its parent container.  In 
tracking down the problem, we tried to verify this behavior 
was NOT present in the standard Open API 3.5 codebase.  
Unfortunatley, we found that the core IDE's support for 
indexed property editing was indeed also broken.

The implementations of IndexedPropertyEditor and 
IndexedEditorPanel don't appear to have changed much 
between 3.4 and 3.5.  One primary suspect of the problem 
may be changes to the NewAction implementation, which now 
uses some "context aware" code to initialize actions and do 
other things we don't really understand.  We believe the 
root of the confusion lies in the fact that the indexed 
property editor/panel co-opts Nodes and Actions for editing 
an array, and that something in the new OpenAPI version is 
causing the editor to use the wrong Node(s).
Comment 1 Todd Fast 2003-08-08 10:27:45 UTC
Please note that the code in IndexedPropertyEditor and 
IndexedEditorPanel is identical in the OpenAPI 3.5 and 
3.5.1 releases.  I have not been able to test directly, but 
unless changes were made elsewhere in codebase, this bug 
also affects the 3.5.1 release.
Comment 2 Jesse Glick 2003-08-08 11:16:14 UTC
I can reproduce in 3.5.1. Probably not property sheet as such, rather
changes in how actions get their selection and context, which is
pzavadsky's area I think. The implementation of IndexedPropertyEditor
looks rather bizarre - for some reason it is using the Nodes API to
just move some entries around; I am not surprised it broke so easily.
Could just be rewritten to work the obvious way, I guess.

Tim - cannot reproduce in dev builds with the new property sheet,
because there is *no property editor* at all. This is also a serious,
unrelated regression you need to fix.
Comment 3 _ tboudreau 2003-08-08 18:00:14 UTC
I added some logging to the code that iterates all the
property sets and populates the model that drives the
properties tree:

PROP: blue - org.openide.nodes.PropertySupport$Reflection@9ac5ba26
PROP: green - org.openide.nodes.PropertySupport$Reflection@165467bd
PROP: red - org.openide.nodes.PropertySupport$Reflection@ccbe5daf

There simply is no Node.Property for intArray.  Maybe
problem in the Beans module or the IndexedPropertyEditor?

Here is the code that is doing the iteration - I don't 
see how it could miss a property:

    private void initPlain () {
        if (sets == null) return;
        int pcount=0;
        
        for (int i=0; i < sets.length; i++) {
            pcount += sets[i].getProperties().length;
        }
        Property[] props = new Property[pcount];
        int l = 0;
        for (int i=0; i < sets.length; i++) {
            Property[] p = sets[i].getProperties();
            System.arraycopy(p, 0, props, l, p.length);
            l += p.length;
        }
        Arrays.sort (props, comparator);
        fds.addAll (Arrays.asList (props));
    }
Comment 4 Todd Fast 2003-08-08 23:30:13 UTC
Jesse: Our team originally copied the IndexedPropertyEditor 
to fix a couple of bugs in it, as well as improve its 
usability a bit (I can send you the fixed sources if you 
like).  My take on the implementation, which is largely 
inscrutable to me, is that it was easier for the author to 
leverage the property sheet/property editor support of a 
Node than it was to try to manage the property editor/combo 
box + button feature manually.  We were originally going to 
write our own indexed property editor replacement, but it 
was exactly that complexity that drive us to try and fix 
the one available to us.  

Maybe it's actually easy to do this manually, and if so, we 
would deeply appreciate any guidance on how to do so.  That 
would get us back up and running without waiting for 
changes from NetBeans.  Or, if someone were to rewrite the 
IndexedPropertyEditor in the next few days, we could make a 
copy of the new version and embed it into our module for 
private use without requiring a release cycle on the 
Studio. We have to make a module release before the next 
version of NetBeans/Studio, so we're going to have to do 
something drastic about this problem anyway.
Comment 5 Peter Zavadsky 2003-08-13 10:12:20 UTC
I'm trying to go trhu those steps, but I'm don't see the property
editor at all in prop sheet (step No 5).  There is just a (No Property
Editor) message in the sheet.

I'm using [trunk] build... is it wrong?
Comment 6 Todd Fast 2003-08-13 10:29:23 UTC
You should also be able to reproduce the problem if you 
create a Node that has an indexed property on its property 
sheet.  The type can generally be anything that has a 
registered property editor (note that String won't work 
because it has a special editor).
Comment 7 _ ttran 2003-08-13 10:46:14 UTC
> I'm using [trunk] build... is it wrong?

Jesse wrote above that there is another bug in the new prop sheet
which prevents you from reproducing this bug.  Please try release35
build instead
Comment 8 Peter Zavadsky 2003-08-13 12:52:50 UTC
I just looked at the code.

I doesn't work due to missing the correct context.. always the global
action is performed.

But I would say IndexedEditorPanel misuses actions (new, move up and
move down) in its action handlers... I guess actions may not be
invoked that way.. there should be put plain handling code. 
Comment 9 _ tboudreau 2003-08-13 14:58:52 UTC
As I mentioned earlier, there is no indication that there
is a problem in the property sheet relating to this bug - 
there simply *is* no Node.Property corresponding to the
indexed property in the property sets of the node when the
property sheet is instantiated.

Conceivably it could create the node *after* the property
sheet is instantiated, and never fire an event - I don't
know, but when I logged the entire contents of the property
sets, there was no Property for the indexed property.
Comment 10 Peter Zavadsky 2003-08-13 15:28:07 UTC
Well I'm going to fix it the way it is working (NewAction
performAction uses inaccessible code)... so the TreeTableView will be
put inside ExplorerPanel, which will provide context for the context
aware action instances.
Comment 11 Peter Zavadsky 2003-08-13 15:56:57 UTC
Fixed in trunk

openide/../explorer/propertysheet/IndexedEditorPanel.form 1.4
                                 /IndexedEditorPanel.java 1.11

Note: It is not possible to see, since the propery editor isn't shown
there. I think this should be a separate bug.
I'm going to prepare patch for [release35] branch.
Comment 12 Peter Zavadsky 2003-08-13 16:01:39 UTC
Created attachment 11307 [details]
Patch for [release35] (put into lib/patches dir)
Comment 13 Peter Zavadsky 2003-08-13 16:02:35 UTC
Created attachment 11308 [details]
diff of patch for [release35]
Comment 14 Todd Fast 2003-08-14 14:03:42 UTC
Thank you for the patch, Peter.  Since we had originally 
copied the indexed property editor to our module to address 
some problems with it, I was able to take your patched 
version and update our copy so that it now works properly 
within our module.  This bug is not longer a problem for 
us.  I will submit the other editor problems we know about 
so that they can also be fixed.  This should allow us to 
eventually remove our local copy.
Comment 15 Peter Zavadsky 2003-08-14 14:27:54 UTC
Thanks for info todd.

What to do with the patch... is it necessary to integrate into
[release35] branch?
Comment 16 Todd Fast 2003-08-14 14:31:02 UTC
At this time, it is not necessary to integrate the patch 
into Studio 5/Open API 3.5 for use by our module.  However, 
since the bug affects all Studio 5 users, it may still be 
beneficial to integrate the patch.
Comment 17 _ ttran 2003-08-14 14:34:56 UTC
Todd> This bug is not longer a problem for us.

good to hear!

Peter: don't integrate into release35 (yet), although we may.  I need
to check the schedules. For now fix in trunk only and mark target
milestone properly

Thanks!
Comment 18 Peter Zavadsky 2003-08-14 14:55:06 UTC
OK, in trunk it is fixed. Thus marking as fixed.

But you can't experience it since the property editor isn't shown (for
the indexed properties). 

It is another issue and Tim already knows about it.. (we found out
that IndexedPropertyEditor was hacked inside PropertyPanel...)
Comment 19 Jaroslav Tulach 2003-08-20 15:12:52 UTC
The fix is not too good as it introduces dependency from
openide-explorer.jar to ExplorerPanel (deprecated). I'll attache
better fix that uses special lookup instead of the deprecated API.
Comment 20 Jaroslav Tulach 2003-08-20 15:13:26 UTC
Created attachment 11382 [details]
Better fix for main trunk
Comment 21 Peter Zavadsky 2003-08-20 16:04:42 UTC
I reviewed it..It seems fine.. please put it in.
Comment 22 Jaroslav Tulach 2003-08-20 16:34:25 UTC
Going to put the fix in...
Comment 23 Jaroslav Tulach 2003-08-20 16:38:06 UTC
openide/src/org/openide/explorer/propertysheet/IndexedEditorPanel.java,v
 <--  IndexedEditorPanel.java
new revision: 1.12
Comment 24 Marian Mirilovic 2003-09-05 15:35:03 UTC
verified in [nb_dev](20030905)