Bugzilla – Full Text Bug Listing
| Summary: | [Dialogs] promptForPassword dialog not correctly parented? | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Billy Biggs <billy.biggs> | ||||||||
| Component: | CVS | Assignee: | platform-cvs-inbox <platform-cvs-inbox> | ||||||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P5 | CC: | Michael.Valenta, mik.kersten, steffen.pingel, sxenos, Szymon.Brandys, tomasz.zarna | ||||||||
| Version: | 3.1 | Keywords: | helpwanted | ||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux-GTK | ||||||||||
| Whiteboard: | stalebug | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Billy Biggs
Copying Stefan as we are just using his parent determination mechanism. We should investigate hw to properly solve this. It would probably involve some way of associating the prompt with the window used to launch the CVS operation and only promting when that window is active or made active. Not for 3.1 though... Stefan, can you comment on this please. Billy says that the password dialog is not being parented at all. If there is no active shell and no mocal children, what do you use as the parent of the shell that the dialog uses? My code is intended to break deadlocks, not to always select the best parent. If there is no active or modal shell then there is no deadlock, and it will leave the dialog with a null parent (as requested). The suggestion in comment 1 is correct: remember the shell from which the user requested the CVS operation and open the dialogs there. Actually, comment 1 by itself would not solve the problem as deadlock could occur if another modal shell is parented by that active window. It is also possible that the launching window has been closed since the job was launched. I have opened bug 99808 against Platform UI for this issue and I will leave this open in the CVS component to track that bug. As I said in bug 99808, I feel that this issue is complicated enough that we need a Workbench level solution. "deadlock could occur if another modal shell is parented by that active window" Yes, this is a JFace bug. Window should make it impossible to pick a parent shell that creates a deadlock. It should always select a modal child if one exists. "It is also possible that the launching window has been closed since the job was launched" True, but this isn't as general as the first issue. Your situation is unique to background threads whose output is displayed in more than one window, which is quite rare. In any situation where the thread's output is being displayed in a single view or editor, it should use the IShellProvider for that part and cancel the job if the part is closed. BTW, I didn't mean to suggest caching a Shell pointer. It should cache an IShellProvider, if anything. In your situation, the best solution may be as follows: 1. The background thread never opens dialogs. Whenever it wants to display output to the user, it sends a message to all views that are currently displaying the output of the thread. 2. All views that are displaying output of the thread would simultaneously prompt the user by nonmodal means. The app would not be blocked, but the dependant views would be. 3. The background thread blocks until a response is received (just as if a modal dialog had been opened). 4. Once the user responds to the message (in ANY view that is currently displaying it), the response is sent back to the background thread, which wakes up. When the response is recieved, all views that were displaying the message hide it. 5. Any view that is currently displaying the output of the job can register itself to handle messages generated by that job. In the case where the output is only shown in one view, this will be equivalent to having a correctly-parented modal dialog. In the case where multiple views are displaying the output of the same job, all affected views will show the message. This makes sense, since all of these views are effectively blocked until the user responds to the message. Stefan, there are definitely some things to think about in comment 5. My first impression is that, no matter what is done, it must be a Workbench facility so that there is consistency in how background jobs prompt. The main question at this time is whether we think this is a big enough issue to address in 3.1. Obviously what you suggest is not appropriate for inclusion into 3.1. The way I see it, the only reasonable solution would be for me to copy the default shell provider from Window and modify it to pick the best parent. After some discussion on this, I've decided to more this bug to UI to deal with the specific issue that Billy reported. As Stefan states in comment 3, the mechanisms main purpose is to avoid deadlock. However, it seems to me that picking a null parent in the case where deadlock is not possible is not the proper behavior. The shell provider should pick a more appropriate parent in this case. UI should fix the issue that is currently permitting dialogs to be opened in a manner that creates deadlock, and this is worth considering for 3.1. UI should not (yet) offer an infrastructure for selecting the parent from a background job. Currently, the CVS dialogs are the only case where the job is associated with more than one window... and it would be premature to try to create a general infrastructure based on only one use case. CVS should fix this themselves, and if there is a demand for a similar infrastructure elsewhere, we can investigate generalizing it. Only the CVS code has the context to select the correct parent. There is no way for the workbench to solve this for free. Moving back to CVS then... Unfortunately, we do not have the cycles to address this in 3.3. What do you think about using the same mechanism to find a shell as in ProgressManagerUtil? The method getDefaultParent looks promising. This is te right method to use. However because it is UI internal method, we have to implement a similar util class in out projects. I meant "our" projects. Created attachment 84058 [details]
Patch
Patch contains the same methods used in org.eclipse.ui.internal.progress.ProgressManagerUtil.java. The only thing I changed is access level modifier (from public to private).
Created attachment 84059 [details]
mylyn/context/zip
Created attachment 84292 [details] a patch for org.eclipse.jsch.ui If a patch at comment #14 is accepted, the attached patch should be applied to org.eclipse.jsch.ui plug-in. The new methods look ok. However I would move them to the cvs ui utility class. In case, if there were other dialogs (in the cvs plugin) with wrong parenting, we could use this utility class. However before we apply the patch, we should verify if the issue still exists. AFAIK this problem was not reproducible. So we can't verify if the patch fixes this particular issue or not. Billy could you confirm that the problem still exists? We have same requirement for Mylyn, i.e. prompt the user for a password from a background job. I am currently trying to determine the recommended way to set the parent for the password dialog on 3.3 and 3.4. From a quick look at the CVS UserValidationDialog is seems that it is using null as the parent shell. This is working in most cases but I have seen it fail in two scenarios: 1. A progress dialog is showing when the user is prompted for the password. If the job that is displaying the progress dialog finishes the progress dialog is closed and the password prompt disappears because it is a child shell. 2. If a native dialog is displayed on Mac OS X (e.g. File > Open) when the modal password prompt is displayed the workbench locks up, i.e. it is neither possible to close the password prompt nor the native dialog. Is there a good example in platform that handles these scenarios? Steffen, I think that both of the cases you have mentioned would require support from the Platform to work properly. Could you log a bug against Platform/UI for these? You probably want to log two bugs since they seem to be two separate issues. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |