Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 344983 - NPE in target definition editor when opened with unknown editor input
Summary: NPE in target definition editor when opened with unknown editor input
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.7 RC1   Edit
Assignee: Ankur Sharma CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 345016
  Show dependency tree
 
Reported: 2011-05-06 11:02 EDT by Eckart Langhuth CLA
Modified: 2011-05-16 16:51 EDT (History)
5 users (show)

See Also:
ankur_sharma: review+
daniel_megert: review-

Attachments
Simple Patch (3.14 KB, patch)
2011-05-06 11:04 EDT, Eckart Langhuth CLA 		no flags Details | Diff
Support StorageEditorInput patch (7.53 KB, patch)
2011-05-06 11:04 EDT, Eckart Langhuth CLA 		no flags Details | Diff
Fail with PartInitException (3.36 KB, patch)
2011-05-09 05:20 EDT, Dani Megert CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eckart Langhuth CLA 2011-05-06 11:02:27 EDT 
Build Identifier: I20110310-1119

When opened with unknown editor input the target definition editor will fail to open and display a NPE.
Currently IFileEditorInput and IURIEditorInput are supported.
e.g. Team history views will use other editor inputs. Probably sub types of IStorageEditorInput.
I'll attach two patch proposals.
The first very simple one will just prevent the NPE and inform the user.
The second patch proposal introduce support for any kind of IStorageEditorInput.
(incl. save behavior as described in IStorageEditorInput documentation)

Reproducible: Always

Steps to Reproduce:
1. open a target file from EGit history view
Comment 1 Eckart Langhuth CLA 2011-05-06 11:04:04 EDT 
Created attachment 194941 [details]
Simple Patch
Comment 2 Eckart Langhuth CLA 2011-05-06 11:04:41 EDT 
Created attachment 194942 [details]
Support StorageEditorInput patch
Comment 3 Curtis Windatt CLA 2011-05-06 11:06:33 EDT 
Consider for RC1 (at least the simple patch).
Comment 4 Curtis Windatt CLA 2011-05-06 14:56:50 EDT 
I'm not confident enough with the storage editor input patch to put it in for RC1.  Still looking at the simple patch for RC1 inclusion.  We'll clone the bug for consideration in 3.8.

Once you have used the save option (runs save as), I get the following NPE because the target doesn't have a proper backing handle.

java.lang.NullPointerException
	at org.eclipse.pde.internal.core.target.TargetPlatformService.saveTargetDefinition(TargetPlatformService.java:244)
	at org.eclipse.pde.internal.ui.editor.targetdefinition.TargetEditor$InputHandler.saveTargetDefinition(TargetEditor.java:400)
	at org.eclipse.pde.internal.ui.editor.targetdefinition.TargetEditor.doSave(TargetEditor.java:94)
	at org.eclipse.ui.internal.SaveableHelper$2.run(SaveableHelper.java:151)
	at org.eclipse.ui.internal.SaveableHelper$5.run(SaveableHelper.java:277)
	at org.eclipse.jface.operation.ModalContext.runInCurrentThread(ModalContext.java:464)
	at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:372)
Comment 5 Curtis Windatt CLA 2011-05-06 15:05:11 EDT 
The simple patch gets a +1 from me.  Only change I would suggest is not having the \n in TargetEditor_6 string.

Ankur, please review and commit for RC1.

This bug was cloned as bug 345016 to consider expanding editor support in 3.8.
Comment 6 Dani Megert CLA 2011-05-09 05:19:41 EDT 
-1 for the patch for two reasons:
- Opening an editor should never show a dialog.
- The correct fix is to throw a PartInitException which will then result in an
  exception in the log but on the other hand it will show the file inside a
  text editor (this is the default fallback code in Platform Team).
Comment 7 Dani Megert CLA 2011-05-09 05:20:50 EDT 
Created attachment 195050 [details]
Fail with PartInitException
Comment 8 Dani Megert CLA 2011-05-09 05:25:14 EDT 
Ankur, you can commit my patch if you're happy with it.
Comment 9 Ankur Sharma CLA 2011-05-09 10:30:07 EDT 
I didn't commit it because I want sure it shall get counted as contributed. Will commit after verifying.
Comment 10 Ankur Sharma CLA 2011-05-09 10:35:08 EDT 
I was referring to the 'simple patch'. Testing Dani's patch now and will commit it asap.
Comment 11 Ankur Sharma CLA 2011-05-09 10:56:03 EDT 
+1

Applied to HEAD
Comment 12 Curtis Windatt CLA 2011-05-16 16:51:36 EDT 
Verified in I20110514-0800