Community
Participate
Working Groups
The e4Context of a PageSite is not activated, e.g. if a new page is created. PageSite.e4Context is only activated in PageBookView.partActivated if the view itself is activated (users changed the view). Maybe it would be appropriate to activate e4Context in PageSite.activate (like the other nested service/context?
New Gerrit change created: https://git.eclipse.org/r/58212
Today I wanted to open a bug report because for the same reason, because in our company we are using PageSite instances a lot in our own views and we needed quite a while to understand what is going wrong with our programmatically activated handlers. The observation was that for every such handler that implemented IElementUpdater the corresponding method updateElement was *never* called. This caused some serious context menu errors in all the views where we had such handlers activated within PageSite context, so PLEASE fix this bug! My analysis came to the same conclusion as the submitter that PageSite#activate and PageSite#deactivate are missing to call the corresponding activate/deactivate calls on the internally hold EclipseContext. Personally I would suggest two slightly improvements of the fix: a) The activate / deactivate calls on the context should follow the same hierarchic order as the other calls inside PageSite#(de)activate. That means that the new line e4Context.activate(); should logically be located between line active = true; and line serviceLocator.activate(); b) Not directly related to this issue, but a real minifix should be to adjust PageSite's signature of Object getAdapter(Class adapter); boolean hasService(final Class key); which don't match the recently generified IServiceLocator's methods <T> T getService(Class<T> api); boolean hasService(Class<?> api); Can we please have this interface correction applied as well to PageSite? I also want to inform those people (like us) who need a working workaround NOW. Fortunately this is possible by a rather good approximation of the ideal solution by introducing a derivative of PageSite as follows (Note the nasty and actually unnecessary type cast in the constructor because of problem (b) mentioned above): public class MyPageSite extends PageSite { private IEclipseContext e4Context; public MyPageSite(IViewSite parentViewSite) { super(parentViewSite); e4Context = (IEclipseContext) getService(IEclipseContext.class); Assert.isNotNull(e4Context); } @Override public void activate() { e4Context.activate(); super.activate(); } @Override public void deactivate() { super.deactivate(); e4Context.deactivate(); } }
New Gerrit change created: https://git.eclipse.org/r/74949
(In reply to Eclipse Genie from comment #3) > New Gerrit change created: https://git.eclipse.org/r/74949 An alternative P/R has been provided that uses the correct hierarchic order of activation and deactivation. It doesn't touch problem (b) here, because the problem is orthogonal. I will open a separate bug that requests proper generification of PageSite.
(In reply to Daniel Kruegler from comment #4) > (In reply to Eclipse Genie from comment #3) > > New Gerrit change created: https://git.eclipse.org/r/74949 > > An alternative P/R has been provided that uses the correct hierarchic order > of activation and deactivation. It doesn't touch problem (b) here, because > the problem is orthogonal. I will open a separate bug that requests proper > generification of PageSite. Stephan, I just noticed that I only cared about PageSite (which we use in our code base), but your fix (correctly) also fixes PageBookView. Would you be willing to improve the ordering problem by your patch suggestion, so that I can withdraw my presented patch?
Gerrit change https://git.eclipse.org/r/58212 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=aa8cc8120a501ef61938bd9f39c63ff26c71b1d6
Thanks, Stephan & Daniel!
New Gerrit change created: https://git.eclipse.org/r/152448
(In reply to Eclipse Genie from comment #8) > New Gerrit change created: https://git.eclipse.org/r/152448 This reverts the fix for this bug, because it causes regression described in bug 552906 comment 18 => reopening this bug again for a better fix.
Gerrit change https://git.eclipse.org/r/152448 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=b00c85436f067e43337eedae4095db9e5fd407eb
