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.
For example, overriding getDefaultAction does not produce the intended result for getPreferredAction. In 3.5, this is a minor problem since no one typically calls getPreferredAction. In the trunk, however, Explorer views do call it, so for compatibility with existing AbstractNode classes it is desirable to implement it correctly. For example: - old node overrides just getDefault -> should be used from getPreferred - old node is a BeanNode (which calls setDefault) and also overrides getDefault (which used to cancel out the effect of setDefault) -> getDefault should be used from getPreferred Attaching possible patch which seems to work to keep full compatibility. Unfortunately it adds a bit to the storage requirements for AbstractNode.
Created attachment 9789 [details] Possible patch (includes new unit test exercising all the cases I can think of)
Marking API since the API behavior of getPreferredAction is at stake. Marking REGRESSION since in the current trunk, old nodes which used getDefaultAction and setDefaultAction do not work as expected with the new explorer views.
A priority in the trunk, I think. E.g. in dev builds the preferred action for HtmlNode is always View. HtmlNode says: public SystemAction getDefaultAction() { // ... if (/* doc fs */) { return SystemAction.get(ViewAction.class); } else { return SystemAction.get(OpenAction.class); } } However DataNode says: public Action getPreferredAction() { // ... Action action = super.getPreferredAction(); if (action != null) { return action; } Action[] arr = getActions(false); if (arr != null && arr.length > 0) { return arr[0]; } return null; } and AbstractNode says: public Action getPreferredAction() { return preferredAction; } So HtmlNode.getPreferredAction is implemented in DataNode, which gets null from the super call and then chooses the first action from the context menu - View. Properly, the super call should return View or Open as defined in HtmlNode.getDefaultAction. The patch should accomplish that. I'm sure there are other things which are broken. The root of some, but not all, evil is the fact that you can both set a default action and override the getter. This is ridiculous. I am unhappy that we repeated the mistake by introducing AbstractNode.setPreferredAction (see issue #22384).
Jesse's new test shall be part of the testsuite, sorry for not providing this, but AbstractNode.setPrefAction, was done after I stopped work on the actions branch. I was just notified about the change, I admit, I agree with it, but now maybe Jesse is right, we could delete it for 3.5. Anyway this does not solve the n2 example in Jesse's test. We need some hacks for it as well. I would rather use reflection instead of ThreadLocal. I'll attach an example.
Created attachment 9828 [details] Fix based on reflection - Jesse's test passes
Agreed, even without setPreferredAction you still need some tricks. Yarda's patch looks good. I was nervous about using reflection, but I think this is better - CPU speed of this method is not as important as avoiding wasted memory. Minor comments to the patch: - if preferredAction != null, can return that immediately, no need to check for getDefaultAction (IMO) - could avoid calling getClass() >1 time per call to getPreferredAction - clean up comment "we are subclass of FilterNode" BTW are there any tests confirming that FilterNode works sanely with getPreferredAction vs. getDefaultAction?
AN.sPA: see issue #33038.
/cvs/openide/src/org/openide/nodes/AbstractNode.java,v <-- AbstractNode.java new revision: 1.63; previous revision: 1.62 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/nodes/NodeTest.java; /cvs/openide/test/unit/src/org/openide/nodes/NodeTest.java,v <-- NodeTest.java new revision: 1.4; previous revision: 1.3 done
Integrated to 3.5 branch /cvs/openide/src/org/openide/nodes/AbstractNode.java,v <-- AbstractNode.java new revision: 1.60.6.1; previous revision: 1.60 done
Need some improvments, because of issue 33205.
Once again and better. This is the new fix (only in trunk): Checking in src/org/openide/nodes/AbstractNode.java; /cvs/openide/src/org/openide/nodes/AbstractNode.java,v <-- AbstractNode.java new revision: 1.64; previous revision: 1.63 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/nodes/NodeTest.java; /cvs/openide/test/unit/src/org/openide/nodes/NodeTest.java,v <-- NodeTest.java new revision: 1.5; previous revision: 1.4
verfied