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 93519

Summary: Allow intercepting of creation of action presenters
Product: platform Reporter: David Strupl <dstrupl>
Component: Window SystemAssignee: David Strupl <dstrupl>
Status: RESOLVED FIXED    
Severity: blocker CC: apireviews, jtulach
Priority: P3 Keywords: API, API_REVIEW_FAST
Version: 5.x   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Reporter:
Attachments: First attempt on a patch
Revised patch

Description David Strupl 2007-01-30 10:00:21 UTC
<quote who="David Strupl">
> We want to use it as a global place where you can disable/enable actions
> depending on the user access permissions. I mean we are currently
> writing a module that will implement per action user access permissions.
>
> If it is not changed we can provide our own menu bar (which we did at
> some point in time) but AWTBridge is called even for actions invoked
> from popups on nodes which is quite convenient.
>
> Even if the intention was different it is an extension point for us (if
> I use Eclipse terminology). You replace AWTBridge by your own impl and
> voila you are in all places where someone converts from Action to
> JMenuItem/JButton. Otherwise we are in deep sit with our idea and would
> have to provide a code in every action to enable/disable itself based on
> some condition which would effectively mean that we would have to
> mandate that all our actions depend on the access permission module. Of
> course we could use a common action superclass but that would mean
> modification to all the existing action code - not very nice.
>
> Simply can the AWTBridge be added to APIs? It is IMHO very useful
> extension point that is otherwise missing (intercepting the Action -->
> JComponent call).

Fuj! 

Ok, I do not think the AWTBridge shall be made public and stable API as it is 
in wrong package and module. But if you need this functionality, then I 
suggest you to write similar class in org-openide-awt.jar that will allow you 
to do what you need. Something like:

package org.openide.awt;

class Actions {

  public static interface ButtonUIProvider {
    public void connect(Action, AbstractButton);
  }
}

look the impls from Lookup.getDefault() and call it where needed. Just write 
some unit test to assert that the code is properly called and go thru fast 
track API review. Please make sure that if no such provider is present, the 
behaviour stays the same as right now.

-jst
Comment 1 David Strupl 2007-01-30 10:01:37 UTC
I would like to solve this for 6.0 as it has very high priority for our team.
Comment 2 David Strupl 2007-01-30 11:16:48 UTC
Hello Jarda,

Just to make sure I understand your proposal, did you mean something like:

package org.openide.awt;

class Actions {

  public static interface ButtonUIProvider {
    public JMenuItem createMenuPresenter (Action action);
    public JMenuItem createPopupPresenter(Action action);
    public Component createToolbarPresenter(Action action);
  }
}

Otherwise I don't understand how would we alter the behaviour of the menu item
from the connect method.

Thanks, David
Comment 3 Jaroslav Tulach 2007-01-30 12:50:19 UTC
I thought it is enough to change the setEnabled/Disabled state which I believe 
is handled by Actions.connect. My proposal was to create an interface and as 
first line in Actions.connect do if (Lookup.getD().lookup(TheInterface.class)) 
{ theInterface.connect(...); return }. Imho it should be enough. If it not, 
then sorry.
Comment 4 David Strupl 2007-01-30 13:11:33 UTC
And our implementation would do our checks and either call the original connect
(via looking up the default impl) or my new code? I think I should be able to
call the original connect code in order to do the old behaviour even in case my
provider is registered (for actions that are e.g. authorized for the user).
Without the original connect I am not able to properly set the text, icon,
enablement etc.
Comment 5 David Strupl 2007-02-01 08:19:28 UTC
Hello,

one last refinement before I will implement it:

package org.openide.awt;

class Actions {

  public static interface ButtonUIProvider {
    public boolean connect(Action, AbstractButton);
  }
}

The return value would be success of the connection. If false is returned the
original connect would be called after this call. Ok?

D.
Comment 6 David Strupl 2007-02-01 12:44:25 UTC
Created attachment 37915 [details]
First attempt on a patch
Comment 7 David Strupl 2007-02-01 12:47:24 UTC
Please check the patch and comment on the patch. I think that the change is
really simple.
Comment 8 Vitezslav Stejskal 2007-02-02 03:55:44 UTC
VS1: I am not sure if I should be asking, but what if there is more
ButtonActionConnectors registered in the lookup? Shouldn't you try to use all of
them? Of course the first one that 'connects' wins, but in your impl if the
first one does not connect no others get a chance.
Comment 9 David Strupl 2007-02-02 09:36:12 UTC
VS1: Of course any comments are welcome. You are right that my impl doesn't care
about more registered objects. I was thinking about our use case that will put
there just one. I have no problem in changing the patch to handle more
instances. I will try to gather all feedback first and then post refined patch.
Keep the comments coming ;-)
Comment 10 Jesse Glick 2007-02-08 21:34:50 UTC
BTW please use cvs diff -u for patches for review; the patches generated by the
IDE are hard to read. (Yes, it's filed.)

Generally looks good. Minor things:


"successfull" - typo.


'static' is meaningless on an interface, as is 'public' on interface methods.


@see org.openide.util.Lookup#getDefault();

has an extraneous semicolon.


You should have Javadoc link to the .connect(...) methods which are affected by
the SPI.


spec.version.base should end in a .0, please restore that, it is intentional; cf.

http://wiki.netbeans.org/wiki/view/DevFaqImplementationDependency


In arch.xml it is best to put the whole <p> inside <api>.
Comment 11 David Strupl 2007-02-14 10:55:59 UTC
Working on the revised patch.
Comment 12 David Strupl 2007-02-14 11:36:22 UTC
Created attachment 38478 [details]
Revised patch
Comment 13 David Strupl 2007-02-14 11:38:11 UTC
Reassigning back to apireviews with the revised patch attached to the issue. As
the changes are minor and according to all the comments so far if there are no
objections I will apply the patch tomorrow.
Comment 14 David Strupl 2007-02-15 10:27:09 UTC
As there were no complaints I will integrate the change. If some more changes
are required please file bugs against me, I will fix them.
Comment 15 David Strupl 2007-02-15 12:47:47 UTC
IDE: [15.2.07 13:49] Committing started
Checking in test/unit/src/org/openide/awt/ActionsTest.java;
/cvs/openide/awt/test/unit/src/org/openide/awt/ActionsTest.java,v  <-- 
ActionsTest.java
new revision: 1.4; previous revision: 1.3
done
Checking in src/org/openide/awt/Actions.java;
/cvs/openide/awt/src/org/openide/awt/Actions.java,v  <--  Actions.java
new revision: 1.15; previous revision: 1.14
done
Checking in nbproject/project.properties;
/cvs/openide/awt/nbproject/project.properties,v  <--  project.properties
new revision: 1.9; previous revision: 1.8
done
Checking in apichanges.xml;
/cvs/openide/awt/apichanges.xml,v  <--  apichanges.xml
new revision: 1.9; previous revision: 1.8
done
Checking in arch.xml;
/cvs/openide/awt/arch.xml,v  <--  arch.xml
new revision: 1.10; previous revision: 1.9
done
IDE: [15.2.07 13:49] Committing finished