Comment 1 Michael Rennie 2013-01-08 10:12:41 EST

Marco, can you provide an example plug-in that produces the problem?

Comment 2 Marco Descher 2013-01-08 10:17:44 EST

Please find https://github.com/col-panic/generic-stuff/commit/fa7e5e3ab62f0220ec512f2ef473e31926b2f5f9 for an example.

Comment 3 Paul Webster 2013-01-08 13:27:22 EST

Doesn't this imply that we weren't able to generate a parameterized command for this model element?

PW

Comment 4 Marco Descher 2013-01-08 16:00:37 EST

Yes it does, using a conditional breakpoint shows that the Feature wbCommand is set to WB_COMMAND_EDEFAULT being null, but this is the case in both tooltip being set and tooltip being null.

The difference is, that in case that a tooltip is set, the following codepart is not executed where the NPE occurs.

if (text == null) {
		try {
-- NPE -->		text = parmCmd.getName();
		} catch (NotDefinedException e) {
			return null;
		}
	}

Comment 5 Marco Descher 2013-01-08 16:04:00 EST

So if no tooltip is set, it tries to add the name of the respective command as tooltip which is not applicable if there is no command set. 

Setting an arbitrary command on the HandledToolItem removes the NPE.

What way to fix this? Check for the setting of a command, i.e. the null or take the setting of a command to a HandledToolItem as a pre-condition?

Comment 6 Marco Descher 2013-01-08 16:19:03 EST

Created attachment 225355 [details]
Solution proposal

There are two solutions I see, as generateCommand() does not guarantee to create a parmCmd instance. (In the current case it fails on "if (model.getCommand()" as no command is defined:

(1) let generateCommand() return a boolean whether an instance could be created
(2) check if parmCmd != null and only in this case fetch the tooltip

solution (1) would IMHO turn the method into a function, which i am not sure of whether this is desired, hence the proposed patch uses solution (2)

Comment 7 Paul Webster 2013-01-09 08:13:03 EST

The underlying problem is a MHandledItem with no command (which is invalid).  This should be logged (probably in generateCommand()) and then yes you can guard against NPEs in other places.

PW

Comment 8 Marco Descher 2013-01-09 08:34:03 EST

Created attachment 225381 [details]
Solution proposal

I updated the patch accordingly; I could not find any other places a guard for parmCmd is missing.

Comment 9 Marco Descher 2013-01-09 16:19:01 EST

Created attachment 225402 [details]
Solution proposal (reworked)

I did some more background checks on the fix, and found out the following facts:

generateCommand() is used by MHandledToolItem and MHandledMenuItem, so the bug also occurs when defining a HandledMenuItem without defining a command.

generateCommand() is called from #setModel and #getTooltipText but getTooltipText only in MHandledToolItem, and there repeatadly to update. This leads to repeated outputs of the error message.

if (model.getCommand() != null && model.getWbCommand() == null) would also fail if there is no command but only a parameterized command available. I could not yet create such a situation.

I think the fix is good, but the repeated output of the error message in the call of #getToolTipText is not. It suffices once in #setModel.

Hence I change the patch to check for the setting of a command within #setModel only and issue a respect missing message, and care for guard in the other place.

Comment 10 Paul Webster 2013-02-05 16:15:34 EST

Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=aec2b9fa0f49ee496f8753407c924b13021893cd

Thanx Marco

PW

Comment 11 Paul Webster 2013-03-12 13:22:32 EDT

In 4.3.0.I20130311-2000

PW