Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 344983

Summary:

NPE in target definition editor when opened with unknown editor input

Product:

[Eclipse Project] PDE

Reporter:

Eckart Langhuth <eckart.langhuth>

Component:

Assignee:

Ankur Sharma <ankur_sharma>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

ankur_sharma, curtis.windatt.public, daniel_megert, matthias.gradl, remy.suen

Version:

3.7

Flags:

ankur_sharma: review+
daniel_megert: review-

Target Milestone:

3.7 RC1

Hardware:

OS:

Windows 7

Whiteboard:

Bug Depends on:

Bug Blocks:

345016

Attachments:

Description	Flags
Simple Patch	none
Support StorageEditorInput patch	none
Fail with PartInitException	none

Description Eckart Langhuth

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

2011-05-06 11:04:04 EDT

Created attachment 194941 [details]
Simple Patch

Comment 2 Eckart Langhuth

2011-05-06 11:04:41 EDT

Created attachment 194942 [details]
Support StorageEditorInput patch

Comment 3 Curtis Windatt

2011-05-06 11:06:33 EDT

Consider for RC1 (at least the simple patch).

Comment 4 Curtis Windatt

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

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

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

2011-05-09 05:20:50 EDT

Created attachment 195050 [details]
Fail with PartInitException

Comment 8 Dani Megert

2011-05-09 05:25:14 EDT

Ankur, you can commit my patch if you're happy with it.

Comment 9 Ankur Sharma

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

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

2011-05-09 10:56:03 EDT

+1

Applied to HEAD

Comment 12 Curtis Windatt

2011-05-16 16:51:36 EDT

Verified in I20110514-0800