Review Request 114436: Set WindowModality of all KIO message box to Qt::WindowModal

[Dawit Alemayehu]

> On Dec. 13, 2013, 2:27 p.m., Frank Reininghaus wrote:
> > Thanks for looking into this, Dawit! I greatly appreciate this effort.
> > 
> > 
> > Two questions come to my mind:
> > 
> > 1. Why should these dialogs be modal at all? Everything that KIO does is asynchronous, so it could very well be that the window isn't even showing the directory (where the action took place that triggered the dialog) any more.
> > 
> > 2. Since every little change can have unexpected side effects, and the "modality" issue is not causing a lot of trouble for users right now (please correct me if I'm wrong), maybe this change should better be done in master? Currently, the only situation in which a single process can have multiple windows that can perform file management actions is that there are two Konqueror windows, one of which was opened from the other one with "Open New Window", I think (but I might be overlooking some other possibilities).
> > 
> > 
> > Some background for people who have not followed the "modality" discussion in the past: some time ago, Thomas raised the question why Dolphin is not a KUniqueApplication any more. This was done mostly because Strigi made Dolphin crash a lot, and it was quite annoying for users to see all their Dolphin windows disappear if one of them crashed (this is not a big problem any more), but also because it was a bit irritating that KIO dialogs would freeze all Dolphin windows. Some more information can be found in these threads:
> > 
> > http://lists.kde.org/?t=137529683100002&r=1&w=2
> > http://lists.kde.org/?t=137537235900004&r=1&w=2
> 
> Thomas Lübking wrote:
>     > 1. Why should these dialogs be modal at all?
>     
>     Unless anybody has a better explanation I assume it was done because of either
>     a) the wrong assumption that "modal" equals "transient"
>     b) the assumption of (a) actually may hold on some systems?
>     
>     A modal window is used if sequential action is mandatory, eg. if you open a file you can not edit it before you opened it - the modal dialog makes sense to enforce the workflow and assert() it in the code.
>     
>     This is obviously not the case here:
>     the system MUST be prepared to filesystem changes during the nested eventloop of the modal dilaog, eg. while the dialog asks "do you really want to delete foo/bar.txt" this could just happen in another dolphin window, in konsole, VT1 or through a script or cron job.
>     
>     
>     On top of this, I do not even think the dialog should be transient.
>     
>     Eg. I often start a longer (network, crap USB stick) copy job and close dolphin immediately.
>     Popping up questions (override) arrive for the copy job and not the causing process (long closed dolphin)
>     
>     The user must get aware that this action is currently halted and requires interaction to continue, but that isn't related to a particular other window.
>     Some "system interaction spot" would be nice but it had to significantly differ from the common "i don't care" notification that pops up because phonon thinks it lost a resource or so.
>     Alternatively the process indicator in the notification area could just start blinking or show a "interaction required!" message/icon/whatsoever.
>     
>     This is however probably beyond KDE4.
>     
>     The fallback (non-plasma context) solution could simply be a "keep above on all desktops" dialog (it doesn't have to get the focus, but must show up visible) what might be a usable approach even for KDE4
> 
> Kai Uwe Broulik wrote:
>     Unfortunately that [1] never made it to a final state :-/
>     
>     [1] http://en.munknex.net/2012/06/new-kde-copy-dialog-first-preview.html

> 1. Why should these dialogs be modal at all? Everything that KIO does is asynchronous, so it could very well be that the window isn't even 
> showing the directory (where the action took place that triggered the dialog) any more.

Which dialogs? There are dialogs requested by the jobs and those that originate from the ioslaves themselves. The prompts from the jobs and ioslaves are distinctly different and cannot be lumped together.

Though I am not the original creator of these dialogs, I can think of at least one reason why it might have been done this way. Making the dialogs modal is the simplest way to avoid the problem of multiple "File Already Exists" dialog boxes from popping up when copying/moving several files and more than one of those files exist in the destination. Another reason is the jobs themselves might not originally have been written in such a way that they are capable of accommodating asynch responses from prompts.

As far as prompts from the ioslaves, the user almost always needs to respond to them in order for the ioslaves to proceed. Almost all are mandatory prompts. For example, when a user visits a site and gets prompted with untrusted SSL certificate warning dialog, that prompt needs to be answered before the site will load. It makes no sense to make such prompts non-modal! Otherwise, the user can do whatever including going to a new site by entering another address in the address bar. If the user then goes back and chose to accept the untrusted SSL certificate the browser will to go back to the previous URL.

> 2. Since every little change can have unexpected side effects, and the "modality" issue is not causing a lot of trouble for users right now 
> please correct me if I'm wrong), maybe this change should better be done in master? Currently, the only situation in which a single process
> can have multiple windows that can perform file management actions is that there are two Konqueror windows, one of which was opened from the 
> other one with "Open New Window", I think (but I might be overlooking some other possibilities).

Well I have no particular preference to what branch the patch is applied. I just did not see a problem with applying it to the current stable branch because the issue is actually a bug and the change itself does not get rid of modality. It simply restricts it to a given window.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114436/#review45646
-----------------------------------------------------------


On Dec. 13, 2013, 1:53 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114436/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2013, 1:53 p.m.)
> 
> 
> Review request for kdelibs, David Faure and Frank Reininghaus.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> The attached patch changes the WindowModality of all the message/information boxes displayed by KIO::JobUiDelegate to Qt::WindowModal instead of Qt::ApplicationModal. This prevents a message box in one window from blocking all other windows.
> 
> 
> Diffs
> -----
> 
>   kio/kio/jobuidelegate.cpp 8534863 
> 
> Diff: http://git.reviewboard.kde.org/r/114436/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20131214/31254a09/attachment.htm>