Comment 1 Beat Schaller 2011-11-18 04:34:53 EST

Created attachment 207207 [details]
OpenHandler.java

Comment 2 Beat Schaller 2011-11-18 04:35:25 EST

Created attachment 207208 [details]
AboutHandler.java

Comment 3 Paul Webster 2011-11-18 08:23:32 EST

While I'm not that familiar with that code, you shouldn't set the context in your model.  The renderers will create the context when they render that part.

Eric?

PW

Comment 4 Beat Schaller 2011-11-18 09:19:27 EST

(In reply to comment #3)
> While I'm not that familiar with that code, you shouldn't set the context in
> your model.  The renderers will create the context when they render that part.
> 
> Eric?
> 
> PW

In this simple example I agree. But in my real application I have to extend the context with my own Objects to inject it in the part.

Comment 5 Remy Suen 2011-11-18 09:41:40 EST

(In reply to comment #3)
> While I'm not that familiar with that code, you shouldn't set the context in
> your model.  The renderers will create the context when they render that part.

Correct, the renderers will create the context for when an MContext is rendered (an MPart is an MContext).

(In reply to comment #4)
> In this simple example I agree. But in my real application I have to extend the
> context with my own Objects to inject it in the part.

You could potentially use DS to contribute IContextFunction implementations for your parts.

Comment 6 Thomas Schindl 2011-11-18 09:45:47 EST

[...]

> (In reply to comment #4)
> > In this simple example I agree. But in my real application I have to extend the
> > context with my own Objects to inject it in the part.
> 
> You could potentially use DS to contribute IContextFunction implementations for
> your parts.

Yes IContextFunction is the way to go! I've given a talk at EclipseCon Europe on how to create Context - the slides are online at http://tomsondev.bestsolution.at/2011/11/07/slides-from-latest-talks/ and a recording of the talk is at http://www.fosslc.org/drupal/content/eclipse-4-application-platform-not-commonly-known-features-new-platform

Comment 7 Paul Webster 2011-11-18 09:50:07 EST

(In reply to comment #4)

> 
> In this simple example I agree. But in my real application I have to extend the
> context with my own Objects to inject it in the part.


If adding an IContextFunction for each piece of your data doesn't provide you with the correct behaviour, then you would have add a UIEvent handler so you can respond when the Renderer sets the IEclipseContext on your MPart.

If they're String properties, you can use the MPart.getProperties() to set String,String tuples that will be set in the IEclipseContext when it is created.

PW

Comment 8 Beat Schaller 2011-11-18 10:28:34 EST

(In reply to comment #7)
> (In reply to comment #4)
> 
> > 
> > In this simple example I agree. But in my real application I have to extend the
> > context with my own Objects to inject it in the part.
> 
> 
> If adding an IContextFunction for each piece of your data doesn't provide you
> with the correct behaviour, then you would have add a UIEvent handler so you
> can respond when the Renderer sets the IEclipseContext on your MPart.
> 
> If they're String properties, you can use the MPart.getProperties() to set
> String,String tuples that will be set in the IEclipseContext when it is
> created.
> 
> PW

I will have a look to the IContextFunction. In my real app it works with simply extending the context. All values are injected and they are correct.

But the problem is still the same. In my simple example, even without setting the context I have this behaviour of getting the exception when closing the part(s).

Comment 9 Remy Suen 2011-11-18 10:32:44 EST

(In reply to comment #8)
> But the problem is still the same. In my simple example, even without setting
> the context I have this behaviour of getting the exception when closing the
> part(s).

Please zip your project(s) and attach the zip to this bug.

Comment 10 Beat Schaller 2011-11-18 10:42:34 EST

(In reply to comment #7)
> (In reply to comment #4)
> 
> > 
> > In this simple example I agree. But in my real application I have to extend the
> > context with my own Objects to inject it in the part.
> 
> 
> If adding an IContextFunction for each piece of your data doesn't provide you
> with the correct behaviour, then you would have add a UIEvent handler so you
> can respond when the Renderer sets the IEclipseContext on your MPart.
> 
> If they're String properties, you can use the MPart.getProperties() to set
> String,String tuples that will be set in the IEclipseContext when it is
> created.
> 
> PW

I will have a look to the IContextFunction. In my real app it works with simply extending the context. All values are injected and they are correct.

But the problem is still the same. In my simple example, even without setting the context I have this behaviour of getting the exception when closing the part(s).

Comment 11 Beat Schaller 2011-11-18 10:45:53 EST

Created attachment 207226 [details]
zip project testmodel

Comment 12 Remy Suen 2011-11-18 10:58:06 EST

(In reply to comment #0)
> First error:
> 1. Start Application testmodel
> 2. Menu File->Open
> Error endless loop
> Caused by: java.lang.StackOverflowError
>     at java.util.Collections$SynchronizedMap.get(Collections.java:747)
>     at
> org.eclipse.e4.core.internal.contexts.EclipseContext.internalGet(EclipseContext.java:210)

Cannot reproduce this at all.

> java.lang.NullPointerException
>     at
> org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(StackRenderer.java:700)
>     at
> org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.access$7(StackRenderer.java:686)
>     at
> org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$6.close(StackRenderer.java:653)

I can reproduce this problem with your project but cannot reproduce it if I comment out the setContext(IEclipseContext) call in OpenHandler.

> Third error/question:
> (Testform is defined as part like in Second error)
> 1. Start Application testmodel (view now ok)
> 2. Add new view File->Open (view 100)
> 3. Add new view File->Open (view 101)
> 4. Add new view File->Open (view 102)
> 5. view Help->About => view 100 is activated
> 5. close view 100 (click on x) => view 102 is Active
> 6. view Help->About => view 100 is activated
> But - I get the message from my handler that the view 100 is just activated and
> not created. 
> Is this correct? Do I have to keep in mind that the view is already
> initialized?

By default, parts stay in the model when they are closed by pressing the 'X'. That's why your find(*) call in AboutHandler returns a non-null value. If you want them removed from the model, then tag the part with the EPartService.REMOVE_ON_HIDE_TAG tag.

Comment 13 Beat Schaller 2011-11-18 11:08:44 EST

(In reply to comment #12)
> (In reply to comment #0)
> > First error:
> > 1. Start Application testmodel
> > 2. Menu File->Open
> > Error endless loop
> > Caused by: java.lang.StackOverflowError
> >     at java.util.Collections$SynchronizedMap.get(Collections.java:747)
> >     at
> > org.eclipse.e4.core.internal.contexts.EclipseContext.internalGet(EclipseContext.java:210)
> 
> Cannot reproduce this at all.
> 
> > java.lang.NullPointerException
> >     at
> > org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(StackRenderer.java:700)
> >     at
> > org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.access$7(StackRenderer.java:686)
> >     at
> > org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$6.close(StackRenderer.java:653)
> 
> I can reproduce this problem with your project but cannot reproduce it if I
> comment out the setContext(IEclipseContext) call in OpenHandler.
> 
> > Third error/question:
> > (Testform is defined as part like in Second error)
> > 1. Start Application testmodel (view now ok)
> > 2. Add new view File->Open (view 100)
> > 3. Add new view File->Open (view 101)
> > 4. Add new view File->Open (view 102)
> > 5. view Help->About => view 100 is activated
> > 5. close view 100 (click on x) => view 102 is Active
> > 6. view Help->About => view 100 is activated
> > But - I get the message from my handler that the view 100 is just activated and
> > not created. 
> > Is this correct? Do I have to keep in mind that the view is already
> > initialized?
> 
> By default, parts stay in the model when they are closed by pressing the 'X'.
> That's why your find(*) call in AboutHandler returns a non-null value. If you
> want them removed from the model, then tag the part with the
> EPartService.REMOVE_ON_HIDE_TAG tag.

For the first error please remove the Part "Testform" in the Application.e4xmi, the retry

Comment 14 Remy Suen 2011-11-18 11:18:30 EST

(In reply to comment #13)
> For the first error please remove the Part "Testform" in the Application.e4xmi,
> the retry

Yes, I get it now. But like the second error, removing the erroneous setContext(IEclipseContext) call also fixes the problem for me.

Comment 15 Beat Schaller 2011-11-21 02:55:51 EST

(In reply to comment #14)
> (In reply to comment #13)
> > For the first error please remove the Part "Testform" in the Application.e4xmi,
> > the retry
> 
> Yes, I get it now. But like the second error, removing the erroneous
> setContext(IEclipseContext) call also fixes the problem for me.

Ok, yes you're right. Without the setContext it works. 

Hmmm, so I don't have to set this context, otherwise it doesn't work. But why this method exists? Just for internal reason?

Comment 16 Beat Schaller 2011-11-21 03:20:49 EST

(In reply to comment #14)
> (In reply to comment #13)
> > For the first error please remove the Part "Testform" in the Application.e4xmi,
> > the retry
> 
> Yes, I get it now. But like the second error, removing the erroneous
> setContext(IEclipseContext) call also fixes the problem for me.

Ok, yes you're right. Without the setContext it works. 

Hmmm, so I don't have to set this context, otherwise it doesn't work. But why this method exists? Just for internal reason?

Comment 17 Beat Schaller 2011-11-21 03:33:04 EST

(In reply to comment #14)
> (In reply to comment #13)
> > For the first error please remove the Part "Testform" in the Application.e4xmi,
> > the retry
> 
> Yes, I get it now. But like the second error, removing the erroneous
> setContext(IEclipseContext) call also fixes the problem for me.

I add the modified simple project to show you my problem of injection when using a specific model:

In my testmodel2 project I have two models (Objects) I want to inject. If I just use the SimpleModel all is okay. I have nothing to do - it is injected and I can use it.
If I want to use the SpecModel number 4 in a list (in OpenEditor) I have to set the context. 
The result is:
Get first an exception, because in the static Testform I haven't set the a SpecModel. That's okay. The I press Open to open a new editor and yes, setting the context to the right object it works and opens the editor. The Object is injected like I would. But now I have the problem when closing the editors...

So I resume - it works when opening, it doesn't work when closing. Is this behaviour really correct? As Tom stated - I will check it with IContextFunction, but it seems to be strange....

Comment 18 Beat Schaller 2011-11-21 03:33:48 EST

Created attachment 207287 [details]
Testmodel2 with injected models

Comment 19 Thomas Schindl 2011-11-21 03:54:50 EST

I guess the problem is that because you are the one setting the context it doesn't get disposed appropriately (which is done for the context created by the framework).

Another problem with your code is that is very likley not in the right hiearchical position (it should be a child of the Perspective IIRC). 

The only solution without IContextFunction is that you set your new value after the call to showPart() by retrieving the context created by the framework. The draw back there would be that you can't open the part through other means but only through this very handler implementation.

Do not set the context unlike you really really understand what its supposed to do. We should maybe javadoc that this API is not to be called by clients but considered internal to the framework.

Comment 20 Beat Schaller 2011-11-21 06:31:41 EST

(In reply to comment #19)
> I guess the problem is that because you are the one setting the context it
> doesn't get disposed appropriately (which is done for the context created by
> the framework).
> 
> Another problem with your code is that is very likley not in the right
> hiearchical position (it should be a child of the Perspective IIRC). 
> 
> The only solution without IContextFunction is that you set your new value after
> the call to showPart() by retrieving the context created by the framework. The
> draw back there would be that you can't open the part through other means but
> only through this very handler implementation.
> 
> Do not set the context unlike you really really understand what its supposed to
> do. We should maybe javadoc that this API is not to be called by clients but
> considered internal to the framework.

Thx Tom. Yes I think it would clarify to add some comments in the API. If it's only a porblem of disposing, I don't know. It seems also to manipulate the model... but ok, I try to implement IContextFunction. And thanks, the problem (setContext) is located.

Comment 21 Beat Schaller 2011-11-21 10:59:39 EST

(In reply to comment #19)
> I guess the problem is that because you are the one setting the context it
> doesn't get disposed appropriately (which is done for the context created by
> the framework).
> 
> Another problem with your code is that is very likley not in the right
> hiearchical position (it should be a child of the Perspective IIRC). 
> 
> The only solution without IContextFunction is that you set your new value after
> the call to showPart() by retrieving the context created by the framework. The
> draw back there would be that you can't open the part through other means but
> only through this very handler implementation.
> 
> Do not set the context unlike you really really understand what its supposed to
> do. We should maybe javadoc that this API is not to be called by clients but
> considered internal to the framework.

I tried it with the IContextFunction and it seems to work. I'am just wondering why I can't get the last selected element in a list. In this list I set each change with the SelectionService to the value. But in my ContextFunction no chance to get the actual value... Is this correct or do I missunderstand something?

@SuppressWarnings("restriction")
public class VmServiceFunction extends ContextFunction {
    @Override
    public Object compute(final IEclipseContext context) {
        // doesn't work!!
        // (the first selected Object is always the selected one)
        // final GenericModelObject<VmIdentifikation> gmo = (GenericModelObject<VmIdentifikation>) context.get(IServiceConstants.ACTIVE_SELECTION);
        // System.err.println("selection = " + gmo.getObject().getVmId());
        final MInputPart part = context.get(MInputPart.class);
        if (part != null) {
....

Comment 22 Paul Webster 2011-11-21 12:54:46 EST

(In reply to comment #21)
> I tried it with the IContextFunction and it seems to work. I'am just wondering
> why I can't get the last selected element in a list. In this list I set each
> change with the SelectionService to the value.

You set a value from a part using the SelectionService?  And then when you use your handler to open your view, your view sees the second last selection you provided?  Or your GenericModelObject<VmIdentifikation> are not showing up at all?

Where did you install your IContextFunction, and what variable does it provide?  i.e. what do you expect it to inject into your MPart when your part is rendered/instantiated?

Or

Comment 23 Beat Schaller 2011-11-22 02:06:22 EST

(In reply to comment #22)
> (In reply to comment #21)
> > I tried it with the IContextFunction and it seems to work. I'am just wondering
> > why I can't get the last selected element in a list. In this list I set each
> > change with the SelectionService to the value.
> 
> You set a value from a part using the SelectionService?  And then when you use
> your handler to open your view, your view sees the second last selection you
> provided?  Or your GenericModelObject<VmIdentifikation> are not showing up at
> all?
> 
> Where did you install your IContextFunction, and what variable does it provide?
>  i.e. what do you expect it to inject into your MPart when your part is
> rendered/instantiated?
> 
> Or

So, in short - I have table in a resultview where a list of GenericModelObject<VmIdentifikation> are diplayed. In the ISelectionChangedListener I update the value for the ESelectionService. In the IOpenListener I execute my handler to open the part dynamically to see all details. In this detail view in inject the corresponding GenericModelObject<VmIdentifikation>. In this bug I reported my problem. I tried to extend the IEclipseContext in my IOpenListener with my object and propagate it to let it inject in this detail view. 
As I have seen I can't (but I can't understand why this simple approach doesn't work, other own objects are also created by extending the context and then ContextInjectionFactory.make...).
So I have implemented the command handler to misuse the part.setInputURI to set just the id of my detail object. In the ContextFunction (OSGI Service) I read the hole object and with this function I can inject the detail view with my object and that is now my working solution.
I think I understand now, why I can't get the selected element in the ContextFunction. It is because it's an OSGI service and not inject over the e4 model, right?
But the strange behaviour is - when I try to get the selected element by injecting the handler with:

@Execute
public void open(@Named(IServiceConstants.ACTIVE_SELECTION) GenericModelObject<VmIdentifikation> vmIdentifikation, ...

the first time I execute all is okay. I get the selected element. All further calls the object doesn't refresh????? So it opens the part allways with the same data, even other list entries are selected.

Comment 24 Beat Schaller 2011-11-22 09:53:47 EST

Created attachment 207362 [details]
Shows the problem when injecting in handler

The project included show the problem of injecting the handler. DoubleClick on first person shows the selected person, in openListener and in handler. The second time no refresh in the handler.

Comment 25 Paul Webster 2011-11-22 11:44:33 EST

OK, with the example project I get success if I click the list part between invocations, but if I just select in the list and then double-click I get the problem behaviour.

Remy, are we not catching an SWT.Activate somewhere?

PW

Comment 26 Remy Suen 2011-11-22 14:33:41 EST

(In reply to comment #25)
> Remy, are we not catching an SWT.Activate somewhere?

The problem is that the opened details part doesn't have a) a control that can take focus and b) a @Focus method. Hence, while the system thinks the other part has been activated, it hasn't actually been activated according to the windowing system.

Comment 27 Paul Webster 2011-11-22 14:44:01 EST

(In reply to comment #26)
> (In reply to comment #25)
> > Remy, are we not catching an SWT.Activate somewhere?
> 
> The problem is that the opened details part doesn't have a) a control that can
> take focus and b) a @Focus method. Hence, while the system thinks the other
> part has been activated, it hasn't actually been activated according to the
> windowing system.

Ah, OK.  I added @Focus to the table.  So adding @Focus to the detail part fixes the problem?

PW

Comment 28 Remy Suen 2011-11-22 14:47:39 EST

(In reply to comment #27)
> Ah, OK.  I added @Focus to the table.  So adding @Focus to the detail part
> fixes the problem?

Almost. You'd still need a control that can take focus. Right now there's only a label there, which cannot take focus.

Comment 29 Paul Webster 2011-11-22 14:55:16 EST

OK, Beat, the long and the short is that each of your parts need an @Focus method that can assign focus to a widget, similar to:

    @Focus
    public void setFocus() {
    	viewer.getControl().setFocus();
    }

With that, I correctly get the same selection in the list view and in the handler every time.

We're looking into documentation for this in the Eclipser 4 world, as it is the same restriction we have in 3.x, check out the javadoc for org.eclipse.ui.IWorkbenchPart.setFocus()

PW

Comment 30 Paul Webster 2011-11-22 14:55:46 EST

org.eclipse.e4.ui.di.Focus is in org.eclipse.e4.ui.di

PW

Comment 31 Remy Suen 2011-11-22 15:00:57 EST

Please see this link for more information, Beat.
http://wiki.eclipse.org/Eclipse4/RCP/Dependency_Injection#.40Focus_.28org.eclipse.e4.ui.di.29

Comment 32 Beat Schaller 2011-11-23 02:33:39 EST

(In reply to comment #31)
> Please see this link for more information, Beat.
> http://wiki.eclipse.org/Eclipse4/RCP/Dependency_Injection#.40Focus_.28org.eclipse.e4.ui.di.29

Thank you! I didn't see the relation between the handler and the detail part...
Now it works as I wished...

Comment 33 Thomas Schindl 2011-11-23 03:03:51 EST

Aren't we in a better position than in 3.x where we blindly had to call setFocus() not known what people are doing with it. 

Can't we:
a) in absence of @Focus deliver the SWT.Activate on our own?
b) make @Focus method allow to return true/false and in case of false we deliver 
   the SWT.Activate as well

This setFocus() behavior already bugged me a lot in 3.x because it is extremly hard to track down why the hell something is not working as expected.

Comment 34 Thomas Schindl 2011-11-23 13:09:44 EST

(In reply to comment #33)
> Aren't we in a better position than in 3.x where we blindly had to call
> setFocus() not known what people are doing with it. 
> 
> Can't we:
> a) in absence of @Focus deliver the SWT.Activate on our own?
> b) make @Focus method allow to return true/false and in case of false we
> deliver 
>    the SWT.Activate as well

Well reading you FAQ-Entry we could be the one who call composite.setFocus() in case a and b - I think without us doing this in e4 we'll get into a worse situation than we've been in 3.x.

Comment 35 Paul Webster 2011-11-23 13:17:25 EST

(In reply to comment #34)
> 
> Well reading you FAQ-Entry we could be the one who call composite.setFocus() in
> case a and b - I think without us doing this in e4 we'll get into a worse
> situation than we've been in 3.x.

right now our renderer creates a parent composite with a hack in the setFocus() call that tries to activate the part.  It causes recursive activation warnings in the usecase presented here.  We'd have to re-work how we handle the parent composite.

PW

Comment 36 Lars Vogel 2019-11-14 03:14:33 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

If the bug is still relevant, please remove the "stalebug" whiteboard tag.