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 197339 - ActionRegistration.surviveFocusChange does not work
Summary: ActionRegistration.surviveFocusChange does not work
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Actions (show other bugs)
Version: 7.0
Hardware: All All
: P2 normal with 1 vote (vote)
Assignee: Jaroslav Tulach
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-01 19:47 UTC by Jesse Glick
Modified: 2012-05-05 10:20 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Reproducer for problems in comment 8 (zipped platform application) (22.44 KB, application/octet-stream)
2012-03-22 11:26 UTC, pjdm
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2011-04-01 19:47:00 UTC
@ActionID(category = "Build", id = "testsurvive.OtherAction")
@ActionRegistration(displayName = "Click Me Too")
@ActionReference(path = "Toolbars/File", position = 1)
public class OtherAction extends CookieAction {
    protected @Override int mode() {
        return MODE_EXACTLY_ONE;
    }
    protected @Override Class<?>[] cookieClasses() {
        return new Class<?>[] {DataObject.class};
    }
    protected @Override void performAction(Node[] nodes) {
        System.err.println(nodes[0].getLookup().lookup(DataObject.class));
    }
    public @Override String getName() {
        return "Click Me Too";
    }
    public @Override HelpCtx getHelpCtx() {
        return null;
    }
}

works as you would expect: run from a standalone module, Ctrl-3, click the root Home node or some node beneath it, and the toolbar button enables and works when clicked. Ctrl-4, button remains enabled and works when clicked, since OW has no selection. Ctrl-5, button is disabled as expected, since Services does have a selection but no DataObject in it.

@ActionID(category = "Build", id = "testsurvive.SomeAction")
@ActionRegistration(displayName = "Click Me", surviveFocusChange=true)
@ActionReference(path = "Toolbars/File", position = 0)
public final class SomeAction implements ActionListener {
    private final DataObject context;
    public SomeAction(DataObject context) {
        this.context = context;
    }
    public @Override void actionPerformed(ActionEvent ev) {
        System.err.println(context);
    }
}

does not work as documented (in fact there is no apparent difference if sFC=false): after Ctrl-4, button is disabled even though OW has no selection. Also, oddly, this action is disabled on the root "Home" node (a DataShadow), though it is enabled on the child nodes.
Comment 1 Jaroslav Tulach 2011-04-06 13:52:53 UTC
The problem with not being enabled on "Home" is that in the lookup of the node there are two DataObjects - shadow and original one. Because you are in EXACTLY_ONE mode, the action is then disabled. You can report that as another issue.
Comment 2 Jaroslav Tulach 2011-04-06 14:00:32 UTC
Otherwise I can confirm your observation. Behavior of context selection is the same regardless of the surviveFocusChange. Only ActionMap interaction honors that attribute for now.
Comment 3 Jaroslav Tulach 2011-04-06 14:40:48 UTC
ergonomics#0a63862ca972
Comment 4 Jesse Glick 2011-04-06 14:46:06 UTC
(In reply to comment #1)
> The problem with not being enabled on "Home" is that in the lookup of the node
> there are two DataObjects - shadow and original one.

Makes a certain amount of sense. Arguably an issue in the Favorites tab. Not sure if it is worth fixing, or what the fix would be; certain actions might care about the original dir, others about the shadow. (Probably most actions would fall into the first category, whereas just actions implemented in the favorites module would fall into the second.)

> Because you are in EXACTLY_ONE mode, the action is then disabled.

Only for the factory version. The CookieAction is enabled on this - bug in CookieAction, perhaps?
Comment 5 Quality Engineering 2011-04-15 08:39:06 UTC
Integrated into 'main-golden', will be available in build *201104150401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/0a63862ca972
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #197339: Support at least bit of surviveFocusChange for context actions
Comment 6 Jesse Glick 2011-04-15 11:43:09 UTC
Now it is broken in the other direction - it survives too much. A system action with sFC=true will remain enabled when you click on the Output Window (say), but will be correctly disabled if you click on some random node in the Services tab (say). That is because Services is an explorer view and provides a selection, but the selection does not match the criteria. But a factory action with sFC=true will now remain enabled *permanently* if it ever becomes enabled; even selecting the "Databases" node in Services, then clearing the selection entirely, leaves it enabled and still active on the last file you selected (if sensitive to DataObject as in the example).

(In reply to comment #4)
>> Because you are in EXACTLY_ONE mode, the action is then disabled.
> 
> Only for the factory version. The CookieAction is enabled on this

My guess is that the cause is that for either the DataShadow or its nodeDelegate, getLookup().lookupAll(DataObject.class) returns both the shadow and the original (and both are included in actionsGlobalContext), whereas (again for either the DO or its Node) getCookie(DataObject) returns just the shadow. If CookieAction is actually calling getCookie then it would be broken in this way. My impression was that it had been retrofitted to use lookup throughout, but perhaps I am wrong?
Comment 7 Jaroslav Tulach 2011-04-15 13:01:41 UTC
(In reply to comment #6)
> Now it is broken in the other direction - it survives too much.

Right. This one will be harder to fix. But I believe it is not as big problem, so making P3. 

> (In reply to comment #4)
> >> Because you are in EXACTLY_ONE mode, the action is then disabled.
> > 
> > Only for the factory version. The CookieAction is enabled on this
> 
> My guess is that the cause is that for either the DataShadow or its
> nodeDelegate, getLookup().lookupAll(DataObject.class) returns both the shadow

NodeAction and CookieAction work on Node. First of all they obtains list of all Nodes and then they query each node's lookup for particular interface.

The new context actions know nothing about nodes. As a result:
1. they don't know when to be disabled - e.g. survive too much
2. they improperly count selected objects - problems with EXACTLY_ONE & ALL
I was hoping to use Lookup.Provider as a replacement for Node, but I don't think it will really work as Node, DataObject are both providers, so there will be too many of them...
Comment 8 pjdm 2012-03-20 09:49:57 UTC
I can reproduce the "surviveFocusChange=true survives too much" problem (comment #6 and comment #7) on 7.1.1 using a String (or some other simple object) as the context object.

My TopComponent adds a context object to its lookup, and the context aware action works as expected, with the action surviving a focus change, also as expected.

However, this has the side effect that the menu item remains enabled even after the TopComponent has been closed. Executing the action shows that the action is still connected to the context object that was in the TopComponent's lookup, even if the context object is removed from the lookup in componentClosed().

Also, another action that listens on the same class that doesn't have surviveFocusChange also remains enabled if an attempt was made to execute it using a shortcut with focus in another TopComponent, although it doesn't seem to execute.

Furthermore, the menu items remain enabled after the application is closed and reopened, and before the TopComponent is opened, although they don't seem to execute. A "Clean and Build All" of the module suite disables them the next time the application is run.

Hanging on to the context object means (for my application) a potentially large amount of memory is not garbage collected and resources aren't correctly cleaned up, not to mention an action being able to execute on a context object without its TopComponent.

For us this *is* a big problem. Can I request a P1 priority?
Comment 9 Jaroslav Tulach 2012-03-22 09:42:28 UTC
You may make the problem P1, but in such case please donate a failing unit test.
Comment 10 pjdm 2012-03-22 11:26:12 UTC
Created attachment 117070 [details]
Reproducer for problems in comment 8 (zipped platform application)

I don't know if you want a specific format. The attachment is a reproducer for the problems.
Comment 11 pjdm 2012-03-22 11:27:31 UTC
PS see README.txt in the project root directory.
Comment 12 Jaroslav Tulach 2012-03-26 12:27:46 UTC
Thanks for the example. You are right, the behavior does not seem correct.
Comment 13 Jesse Glick 2012-03-28 00:01:40 UTC
Not a P1 since you can just use CookieAction if you need sFC=true until this bug is fixed. Right?
Comment 14 pjdm 2012-03-28 11:19:09 UTC
I don't know. I'm still learning the platform, so I'm unfamiliar with CookieAction, cookies, or editing layer.xml (I've used annotations for pretty much everything so far). Looking at the documentation, it's not a simple edit of the existing actions, I'd have to rewrite them now and rewrite them back again after sFC=true is fixed. 

It may be technically possible but given the above, it doesn't seem practical to use "Not the preferred solution anymore" (to quote the documentation) as a workaround when I'm still trying to figure out how the preferred solutions work.
Comment 15 Jesse Glick 2012-03-28 20:06:42 UTC
If you are interested in trying, a working example of CookieAction (with annotation-based registration) was given at the start of comment #0.
Comment 16 Jaroslav Tulach 2012-05-02 13:41:25 UTC
Observation about the sfc.zip example. The whole system works much better if you don't extend Node. E.g. "public class StringNode extends Object".
Comment 17 Jaroslav Tulach 2012-05-02 14:14:05 UTC
The "extends Node" issue was solved in ergonomics#95c221f99c03
Comment 18 pjdm 2012-05-02 14:27:36 UTC
Your comment #17 arrived as I was replying to your comment #16.

But isn't this how Node is supposed to be used?

As well as adding the Node to the TopComponent's lookup, should I also add the
instance of my data class to the lookup and use that for context-aware actions, instead of using a Node and getting the data from that?

Does comment #17 mean that the answer to the second question is "no"? :-)
Comment 19 Jaroslav Tulach 2012-05-02 15:15:39 UTC
I have not solved the DataObject vs. DataShadow issue, but I think now the system behaves much better: ergonomics#db3fac3100ca
Comment 20 Quality Engineering 2012-05-04 09:52:23 UTC
Integrated into 'main-golden', will be available in build *201205040400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/95c221f99c03
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #197339: More sane behavior of the lookup when queried about Node subclasses and superclasses. Ensure the sfc.zip example leaves both actions disabled after restart.
Comment 21 Quality Engineering 2012-05-05 10:20:44 UTC
Integrated into 'main-golden', will be available in build *201205050400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/db3fac3100ca
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #197339: Paying attention to setActivatedNodes(null) or setActivatedNodes(something).