Community
Participate
Working Groups
We have an e4 Application that still uses the legacy Workbench, mainly to use the Eclipse Help functionality. In the application, one may have two parts of the same type in a part stack, i.e. the MParts have the same elementId. When the Help should be opened for the second part, DefaultHelpUI throws a NPE beacuse WorkbenchPage#getActivePart returns null. After some investigation I found that the viewReferences handling in WorkbenchPage is the cause: In addViewReference it is checked that the viewReferences list does not already contain an entry with the same elementId od the reference's MPart. Thus, my second part is not added to the viewReferences when opened. When retrieving the ViewReferences in getViewReference(MPart), the check is for equivalence of the reference's MPart (ref.getModel() == part) instead of a check for the elementId. So, null is returned and the NPE is thrown in DefaultHelpUI. If I change the MPart elementIds to something like old.id_1 and old.id_2, then the NPE is not thrown (of course having other unwanted implications...)
Sounds like ViewReferences needs to check for the ID instead. Is this also your assessment, Benedikt?
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168308
(In reply to Lars Vogel from comment #1) > Sounds like ViewReferences needs to check for the ID instead. Is this also > your assessment, Benedikt? Well I am not too much into that Workbench handling, but I guess there should be one ViewReference for each MPart instance. If that's the case, then the contains-check in WorkbenchPage should also check for equality instead of the ID (like the getViewReference method). Otherwise there will always be only one ViewReference per ID. I added a gerrit to show what I *think* is more correct... But I cannot really see what kind of other implications this could have.
Lars, how should we proceed here? I am not really confident that we should merge the gerrit, it seems to me that this could have some unwanted implications. But on a pure logical basis, the fix seems correct to me...
(In reply to Benedikt Kuntz from comment #4) > Lars, how should we proceed here? I am not really confident that we should > merge the gerrit, it seems to me that this could have some unwanted > implications. But on a pure logical basis, the fix seems correct to me... Lets review for 4.18M1, 4.17 is in release freeze.
Mass change to 4.19 M1, please update the target if you have other plans.
Mass move 4.19 M1 bugs to M3
Removing milestone from 4.19 M3 to 4.19, please re-target accordingly.
Mass move out of 4.19
Do you think the root cause may be that the elementId is incorrect? I'm wondering whther this might be a deeper issue.
Not sure what you mean with incorrect elementId? The id (in my case) comes from the e4 model (i have a part descriptor with id "xyz") that is added to a part stack one or more times. I did not spot any other problems with the ViewReference handling except for the one mentioned before with the NPE in the DefaultHelpUI...
If I understand correctly, the problem is that 2 distinct view reference do share the same elementId, isn't it? If so, wouldn't it be instead expected that those have different elementId?
Yes, I assume that is the problem. It is expected in the contains method of the class, but not in the getViewReference method. I cannot say if they have to be different from the e4 point of view, but actually I guess not because I have never heard that you have to change the element ID manually during runtime...
Mass Move out of 4.20
