Bugzilla – Full Text Bug Listing
| Summary: | [editor] NPE when use openEcore action in editor | ||
|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Bouchet Stéphane <sbouchet> |
| Component: | Core | Assignee: | Ed Merks <Ed.Merks> |
| Status: | CLOSED DUPLICATE | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | john.arthorne, laurent.goubet |
| Version: | 1.2 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Bug Depends on: | 356116 | ||
| Bug Blocks: | |||
|
Description
Bouchet Stéphane
tested also in Helios ( emf 2.6 ), same issue. On the other hand, open genmodel is working correctly I can't reproduce this. What part of line 91 is getting a null pointer?
IWorkbench workbench = PlatformUI.getWorkbench();
IWorkbenchPage page = workbench.getActiveWorkbenchWindow().getActivePage();
> IEditorPart editorPart = page.openEditor(editorInput, workbench.getEditorRegistry().getDefaultEditor(uri.lastSegment()).getId());
Is there no active page? Is there no default editor? It seems to me we couldn't open an editor in such cases, so we could add guards for them, but then the function wouldn't work at all. So it seems important to understand why there is a null pointer exception for you but not for me.
Without more detail, I'll be forced to return it as works for me...
(In reply to comment #2) > I can't reproduce this. What part of line 91 is getting a null pointer? > > IWorkbench workbench = PlatformUI.getWorkbench(); > IWorkbenchPage page = workbench.getActiveWorkbenchWindow().getActivePage(); > > IEditorPart editorPart = page.openEditor(editorInput, workbench.getEditorRegistry().getDefaultEditor(uri.lastSegment()).getId()); > > Is there no active page? Is there no default editor? It seems to me we > couldn't open an editor in such cases, so we could add guards for them, but > then the function wouldn't work at all. So it seems important to understand > why there is a null pointer exception for you but not for me. > > Without more detail, I'll be forced to return it as works for me... Hi Ed, workbench.getEditorRegistry().getDefaultEditor(uri.lastSegment()) is returning null in my case. my configuration is very simple, i use Eclipse SDK + EMF, checked out EEF code. in a Runtime instance, create an ecore and associated genmodel. Does it work in the main instance? It sounds like there isn't a default editor registered for Ecore in your runtime instance. Did you include the org.eclipse.emf.ecore.editor bundle? (In reply to comment #4) > Does it work in the main instance? nope, same result. > > It sounds like there isn't a default editor registered for Ecore in your > runtime instance. Did you include the org.eclipse.emf.ecore.editor bundle? My runtime is configured to use all the target plaform plugins, including org.eclipse.emf.ecore.editor. BTW, i am digging around to see if it comes from another installed project ( i am using also acceleo, EMFcompare and OCL ) Well, it was pretty fast. debugging the "getdefaultEditor" method, i found that there is 2 contentType registered for *.ecore, and then only the first one is returned, sadly, this is not the ecore.editor one, but : org.eclipse.emf.compare.ui.contenttype.ModelContentType so..... i will move my bug to the correct guys. sorry for the noise. Aditionnal comment. Can the following method public IEditorDescriptor getDefaultEditor(String fileName, IContentType contentType) should be used in the EditUIUtil.openEditor ? Setting org.eclipse.emf.ecore.editor as contentType, this should do it. A double clic on a ".ecore" file works. What is the platform using to find the editor? That is probably what should be used in the "open ecore" case. (In reply to comment #8) > A double clic on a ".ecore" file works. What is the platform using to find the > editor? That is probably what should be used in the "open ecore" case. the double clic works for me, too i can correclty open a ecore file wit emf compare installed. this hapens only on the open ecore action in the genmodel editor. Reaffecting to EMF Core as the only action that fails is the "openEcore" action from the genmodel, which should use the same principle as a double clic on a ".ecore" in the workspace. Granted, this fails because EMF Compare adds a second content type for ".ecore" files... but that is perfectly legal as I understand it, and even if we did remove that content type, others could add their own. For the record, EditUIUtil#openEditor tries to open an editor with page.openEditor(editorInput, workbench.getEditorRegistry().getDefaultEditor(uri.lastSegment()).getId()); (relying on "IEditorRegistry.getDefaultEditor(String)") whereas a double clic on a random file in the workspace will use "IEditorRegistry.getDefaultEditor(String, String)" having searched for the content type beforehand (see IDE#getDefaultEditor(IFile, boolean)). I wonder whether someone from the platform team would comment whether it's correct for getDefaultEditor to return null when in fact there are editors registered for the file extension. If that's correct, what is the correct way to determine the editor that would be opened if the user double clicked? I imagine that EditorRegistry.getDefaultEditor should not just guess a content type but rather should consider all content types for that file and return an editor descriptor if there exists any content type with an associated editor. Currently it seems to assume that if there is a content type there must be an editor for it. Is that a valid assumption? I don't know the editor registry code very well... From the javadoc it definitely says it can return null if no associated editors are found. In this case since you are not passing in a content type, it it trying to find an editor based entirely on file extension. It does also look for any content types associated with that filename, and any editors corresponding to those. If there are no editors registered against that file extension or against any related content types, then you'll get null. I'm not sure what else it can do here based on the information it is getting. I.e., it doesn't have access to the file content, which is often needed to make an accurate content type determination. Maybe getDefaultEditor(String) should even be deprecated, because it lacks enough information to give a good answer (since it is now common that editors are registered against content types rather than purely on extension). I believe the problem here is that there is more than one content type for the extension, but the one that's guessed doesn't have editors registered for it. So while you say "It does also look for any content types" unfortunately it guesses a *single* content type of one of many possible and then only tries to determine an editor for that. When that fails, it returns null as if there is no editor whatsoever available when in fact there are one or more. I think a better implementation would iterate over all content types associated with the extension (rather than guess a single one) and would return the first editor for a content type for which there is a registered editor. It seems fundamentally incorrect to return null when there is an editor registered. After all, the Javadoc says * The default editor is determined by taking the file extension for the * file and obtaining the default editor for that extension. It also says the method "assumes an unknown content type" when in fact it assumes one specific content type. If the method can't produce reasonable behavior (i.e., one where defining a new content type doesn't result in it returning null), it really should be deprecated. I'll need to revisit all the places that EMF is currently relying on this; there are quite a few. In fact, even the generated wizards rely on this method, so changes will require clients regenerating... That makes sense. I suggest entering a bug against Platform for that... trying all content types rather than just the first one it finds makes sense to me (and javadoc to match). On deprecation.. there is always a trade-off so I'm not sure what the right answer is. Using the file contents will often find a more appropriate content type than the file name alone (xml files being a prime example). On the other hand, computing content type based on file contents is a more expensive operation, and people might not always have contents available in advance (for example creating a new file). Since the old method could still be useful in such cases I would probably err on the side of not deprecating. This is best addressed in the platform. *** This bug has been marked as a duplicate of bug 356116 *** |