Bugzilla – Full Text Bug Listing
| Summary: | [ErrorHandling] "Multiple Problems have occurred" dialog should not be modal | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||||
| Component: | UI | Assignee: | Kevin McGuire <Kevin_McGuire> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | P3 | CC: | bokowski, krzysztof.daniel, Mike_Wilson, Szymon.Brandys | ||||||
| Version: | 3.4 | Flags: | Mike_Wilson:
pmc_approved+
bokowski: review+ Szymon.Brandys: review+ |
||||||
| Target Milestone: | 3.4 RC2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
Steps to reproduce can be found in bug 232037 comment 1. ASTView: http://www.eclipse.org/jdt/ui/astview/index.php that dialog is modal if status is reported with BLOCK flag. we have two possibilities here: * completely remove modality from StatusDialog - I would not do that * remove BLOCK flag - recommended by me Who is passing a BLOCK flag? Markus, does the ASTView pass a BLOCK flag? Note that in some (rare) cases, the BLOCK flag might be appropriate. We cannot remove this flag, or change its behaviour. Clients should not use it though - maybe we should improve the documentation? (In reply to comment #4) > Markus, does the ASTView pass a BLOCK flag? No, the error dialog comes from the Platform. Szymon, you fixed bug 168301 recently, using StatusManager.BLOCK in the default JFace status handler. Why did you not use StatusManager.SHOW? (See JFaceUtil.java, line 69) I can't see in the comments for bug 168301 and in the code any reason why I used BLOCK instead of SHOW. The only reason I can see is that ErrorDialog is modal by default (isn't it) and I was influenced by this. One important thing though is, that we have one error dialog now for all errors. So if the dialog is already modal (somewhere BLOCK was used), all statuses from SafeRunnables will be added to this modal dialog. So this will be still an issue in some cases. We can only hope that whoever used BLOCK knew well what they were doing. As Markus pointed out, this is a regression - we were not using ErrorDialog before but instead used SafeRunnableDialog for the very reason that bad things can happen if you open a modal dialog from deep down somewhere. Kevin, do you have the time to make sure line 69 gets changes to SHOW instead of BLOCK for RC2, or should I take over this bug? I have just one question about view activation related to the initial comment. (In reply to comment #0) > The "Multiple Problems have occurred" dialog should not be modal. I just got > into a situation where an NullPointerException occurred whenever a certain view > was activated. Now, each time I close the "Multiple Problems have occurred" > dialog, the view is activated again, and I get the error dialog again. Why do we try to activate a view over and over, if we know that there is a problem (NPE) already? Activating the view is a side effect of focus being transferred back to the workbench window from the modal dialog. (In reply to comment #9) > Kevin, do you have the time to make sure line 69 gets changes to SHOW instead > of BLOCK for RC2, or should I take over this bug? Will do. Created attachment 102205 [details]
patch changing BLOCK to SHOW to avoid modal lockup
Patch as suggested to change BLOCK to SHOW.
Released as JFaceUtil 1.20 into HEAD for build I20080528-2000 |
Created attachment 100143 [details] Screenshot I20080513-2000 The "Multiple Problems have occurred" dialog should not be modal. I just got into a situation where an NullPointerException occurred whenever a certain view was activated. Now, each time I close the "Multiple Problems have occurred" dialog, the view is activated again, and I get the error dialog again. To break the endless circle, I remember that previous versions of such dialogs used to be non-modal, giving me a chance to activate another view or to even close the view. Major, because the only way to get out is to kill Eclipse.