Community
Participate
Working Groups
At line 720. If the selection is null this will result in an NPE. Here is the offending code snippet: 714: // get the new selection 715: TreeItem[] sel = new TreeItem[] { selection }; 716: if (sel.length == 0) { 717: setMessage(null); 718: setErrorMessage(null); 719: } else { 720: Object object = sel[0].getData(); // assume single selection It would be better replaced with the following (sorry, no patch, I'm not set up for editing the platform!): // get the new selection TreeItem sel = new TreeItem[] { selection; if (selection == null) { setMessage(null); setErrorMessage(null); } else { Object object = selection.getData(); // assume single selection if (object instanceof IPropertySheetEntry) { // get the entry for this item IPropertySheetEntry activeEntry = (IPropertySheetEntry) object; // display the description for the item setMessage(activeEntry.getDescription()); // activate a cell editor on the selection activateCellEditor(selection); } } In fact, I'm not at all sure why the selection was being wrapped in a single element array at all, I'm guessing a holdover from old code?
Hi Ian, how did you get the NPE? What actions were you performing?
This was sent back to the bugzilla daemon by mistake! So, err, sorry for the delay... I was using an Eclipse RCP based product called Monkey World 3D (mw3d.org). In that I carried out the following steps: 1. select an object in the editor (this loads values into the property sheet, mostly represented by custom IPropertySource classes); 2. select a property to edit (in this case it is a custom CellEditor used to edit quaternion values); 3. hit enter. It doesn't seem to matter if I make any changes to the editor or not. I would expect this to apply the editor changes and then leave the property cell in the selected state. I will try some different combinations of steps to see if I can reproduce this any other way, but the code snippet I highlighted does seem clearly incorrect, the as sel.length will ALWAYS be 1 so that test will always be false;
Ian, I could safe up the code but I suspect that this won't actually solve your issue. According to your steps there should have been a non-null value in 'e.item' when the 'widgetDefaultSelected' fired (since you had selected an entry in the tree). I'm pretty sure (but not positive) that the various OS's will not send this event when there's no selected tree item. The only other way I can find into this code already checks to ensure that null is not passed. If true then you shouldn't ever be able to get this code to throw an NPE... Can you attach a stack trace of the NPE (just to make sure that there isn't a third way in...;-).
Created attachment 121075 [details] Patch that safes up the code This will safe up the code but I'm yet to be convinced that it'll help the issue. Ian, if you can please try this patch out but as previously stated I'm not sure that it'll actually fix the issue (but would make a good negative test).
Created attachment 125234 [details] Patch that removes copy/paste artefacts as well as improving code safety. Hi Eric, Sorry for taking so long to get back to you on this one. That looks OK to me. I've tried it out and it seems to work although I think you are right in that it is possibly hiding another issue (very possibly in my code though, not Eclipse) rather than solving the root problem. I'm also going to attach a different patch that I'd like to propose in place of yours. It's functionally the same but cleans up some of the copy-and-paste artefacts (as mentioned in my initial issue report). Cheers, Ian.
Thanks for the patch Ian! Nice one, removing the arcanity of turning a single element into an array... Committed in >20090210. Applied Ian's patch.
Verified (visually) in I20090310-0100.
