| Summary: | [StatusHandling] Still an NPE in WorkbenchStatusDialogManager.refreshSingleStatusArea(..) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||||||
| Component: | UI | Assignee: | Krzysztof Daniel <krzysztof.daniel> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | krzysztof.daniel, Olivier_Thomann, remy.suen, tomasz.zarna, utilisateur_768 | ||||||||
| Version: | 3.4 | Flags: | Kevin_McGuire:
review-
|
||||||||
| Target Milestone: | 3.5 M5 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Markus Keller
Created attachment 108053 [details]
Fix
Created attachment 108054 [details]
mylyn/context/zip
Q: Why are we getting null in this case?
The patch is straight forward but there are two other references to
statusListLabelProvider.getColumnText()
(in getViewerComparator()) and by rights these should have null guard checks too.
Another option is to never return null, although the javadoc for ITableLabelProvider suggests you can. I'm not sure if we can legally javadoc our inner class to not return it and thus be stricter than the interface. Markus do you know off hand?
In theory we also need to guard against null from getColumnImage() but I don't see any refs.
(In reply to comment #3) > Q: Why are we getting null in this case? Still would like to understand how we get into the fail state. In any case this first change is good and is the right approach (null checks on return values from getColumnText()): + String message = statusListLabelProvider.getColumnText( + statusAdapter, 0); + singleStatusLabel.setText(message != null ? message : ""); //$NON-NLS-1$ > but there are two other references to > statusListLabelProvider.getColumnText() > (in getViewerComparator()) and by rights these should have null guard checks > too. but we need these resolved too as part of this fix. Also not sure of the reason for this change: private void refreshSingleStatusArea() { - String description = statusListLabelProvider.getColumnText( - statusAdapter, 0); - if (description.equals(singleStatusLabel.getText())) - singleStatusLabel.setText(" "); //$NON-NLS-1$ - singleStatusLabel.setText(description); I mean, the code looks suspicious and is probably bad but I'd like to understand how it relates. Marking patch -1 because I need more info to release it. Summary: 1) Explain how we got null in the first place. Looking through the getColumnText() code it doesn't look like we should be getting null back from it. I want to make sure there isn't some other bug triggering this (e.g. bug in getPrimaryMessage()). 2) Other callers of statusListLabelProvider.getColumnText() should be guarded against null. Although they're not failing as part of this bug, they look like failures waiting to happen. 3) Explain removal of code from refreshSingleStatusArea(). *** Bug 244199 has been marked as a duplicate of this bug. *** *** Bug 245527 has been marked as a duplicate of this bug. *** *** Bug 257710 has been marked as a duplicate of this bug. *** Does anyone remember if progress indicator reported error just before NPE occurred? Created attachment 120302 [details]
Fix
This should solve the issue.
Fixed in 2008-12-15. The problem occured when a job reported an error that should not be presented immediately. Next status tried to create single status dialog, but there were already two statuses to display. verified (tested against scenario desribed in comment 10) |