Community
Participate
Working Groups
This bug is related to 92601. I'm creating a new one because 92601 is marked FIXED but I don't think the soltion really addresses the stated problem. I am using CommonNavigator with an EMF model. So, the EMF objects do not implement IAdaptable. Thus, when I select content in my navigator so properties are displayed in the properties sheet. The fix for 92601 seems to just address the perferences pages. Based on comment #11 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=92601#c11) I made the following change to PropertySheetEntry. The patch is attached. if (propertySourceProvider == null && object != null) { propertySourceProvider = (IPropertySourceProvider) Platform.getAdapterManager().getAdapter(object, IPropertySourceProvider.class); } if (propertySourceProvider != null) { result = propertySourceProvider.getPropertySource(object); } else if (object instanceof IPropertySource) { result = (IPropertySource) object; } else if (object instanceof IAdaptable) { result = (IPropertySource) ((IAdaptable) object) .getAdapter(IPropertySource.class); } else { if (object != null) { result = (IPropertySource) Platform.getAdapterManager().getAdapter(object, IPropertySource.class); } } I first just added the last call to Platform.getAdapterManager() to get an IPropertySource, but EMF apparently requires its own IPropertySourceProvider. So, I added the first call to to Platform.getAdapterManager() to see if one exists. I imagine an IPropertySourceProvider extension point might be better.
Created attachment 37252 [details] Patch to PropertySheetEntry
Tod, let's go over the patch...
Thanks for the patch John. As our API for setPropertySourceProvider says that we will use a default provider this supplies some more flexibility for the default nicely. One concern I had was that you are setting the propertySourceProvider field in your version of the code which I think is dangerous because it is the provider for a specific object. If we make it a field we should be fine. Here is the code - I'll attach a patch once I hear from you. IPropertySource result = null; IPropertySourceProvider provider = propertySourceProvider; if (provider == null && object != null) provider = (IPropertySourceProvider) Platform.getAdapterManager() .getAdapter(object, IPropertySourceProvider.class); if (provider != null) { result = provider.getPropertySource(object); } else if (object instanceof IPropertySource) { result = (IPropertySource) object; } else if (object instanceof IAdaptable) { result = (IPropertySource) ((IAdaptable) object) .getAdapter(IPropertySource.class); } else { if (object != null) result = (IPropertySource) Platform.getAdapterManager() .getAdapter(object, IPropertySource.class); } sources.put(object, result); return result;
Created attachment 37327 [details] Tods patch
The tweak to the patch looks good to me!! Any idea what milestone this would be targeted for?
It's too late for M6 - I'll get it into RC1. Thanks for the quick response. When I get it in I would appreciate it if you could verify for us.
I will be glad to veryify it!
Fixed in build >20060403
Marking verified. John please reopen if this is not correct.
Tod, I just tried this out in 3.2 RC2 and it doesn't look like it made it in.
We looked at the code that is currently released, and it appears to match the text shown in comment #3. Oddly, this does not match the contents of "Tods patch", which looks like an exact duplicate of John's. John, does this match what you're seeing?
I tried out my plugin with 3.2RC2 and the bug was still there. So, in 3.2RC2 SDK I did Open Type for PropertySheetEntry and the code looked unchanged.
OK. I just retested this with the code from cvs and it does not work. This is the line that seems to make the difference: IPropertySourceProvider provider = propertySourceProvider; Removing this line and doing all the conditions based on propertySourceProvider does work.
John, sorry but we can't risk this change for 3.2. While it may be working in the scenario you see it may introduce regressions in other areas/products that we simply won't have time to pick up on... Changing target to reflect this...
Ok. I will package a patch with my plugin. If I figure out why the current fix didn't work I will update the bug.
John Looking at the code removing that line seems to defeat the purpose of having propertySourceProvider cached at all. I am going to remove the milestone for this until we have more information from you as to why you think this is a bad idea.
Thanks for the update. I will take a deeper look at it. Based on my memory, in my case, I don't think propertySourceProvider was getting cached at all.
OK. Here is another patch that might be exceptable. It adds the following method: public IPropertySourceProvider getPropertySourceProvider(Object object) { if (propertySourceProvider != null) { return propertySourceProvider; } else { return (IPropertySourceProvider) Platform.getAdapterManager() .getAdapter(object, IPropertySourceProvider.class); } }
Created attachment 45255 [details] getPropertySourceProvider()
Created attachment 45256 [details] better getPropertySourceProvider()
Upping priority to keep it on my radar
Has this been fixed by the fix for bug 158581?
Created attachment 51551 [details] Merged patch Here is a patch merged with what is in HEAD. Jon could you see if this (or our existing code) solves your issue please.
Looks good to me. In the mean time I have gone with using the tabbed property feature as a work around.
(In reply to comment #22) > Has this been fixed by the fix for bug 158581? Sorry Tod - this is a TPTP bug number...?
The patch or the current code? I am going to close this as REMIND - please reopen if you think there is any work for us to do here
Sorry should be Bug 156581
(In reply to comment #23) > Created an attachment (id=51551) [edit] > Merged patch The merged patch reintroduces manual checking for instanceof and IAdaptable, which is done in a helper method (ViewsPlugin.getAdapter). We should only use the helper methods when getting adapters (this is what bug 156581 was about). Note that currently, these helper methods are duplicated in a number of plugins, but not exposed as API, and we are pushing for this being offered as API on AdapterManager.
As of now 'LATER' and 'REMIND' resolutions are no longer supported. Please reopen this bug if it is still valid for you.
