-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126750/
-----------------------------------------------------------

Review request for KDE Frameworks.


Bugs: 355310
    https://bugs.kde.org/show_bug.cgi?id=355310


Repository: kiconthemes


Description
-------

Currently, KIconDialog::showDialog() sets the modality to Qt::NonModal by 
calling

setModal(false);

I found that this was done in 
https://quickgit.kde.org/?p=kdelibs.git&a=commit&h=d0d2639c126f88a44c852b738550a9427c6260bb
 in order to prevent that a modal dialog locks an entire application, such as 
Plasma.

Unfortunately, this has an unwanted side effect in the "Places" of Dolphin and 
the file dialog: the KIconDialog is the child of a modal "Add Places Entry" 
dialog there. The KIconDialog itself works fine, but the "Browse..." sub 
dialog, which is a grand child of the modal dialog, is opened in the background 
and cannot be used (it could be that this worked for some reason in Qt4 times - 
I guess we would have received bug reports about this issue earlier otherwise).

This can be fixed by setting the modality to Qt::WindowModal, which ensures 
that the dialogs block their respective parents (but not the entire application 
- that would happen if the modality was set to Qt::ApplicationModal, for 
example by calling setModal(true)).

Note that there are two setModal(true) calls in the KIconDialog constructors. 
They have no effect if the dialog is opened with showDialog() (which is what 
happens if the dialog is opened by clicking a KIconButton) because the modality 
is overwritten there. I'm not sure though if there are any other uses of 
KIconDialog which would break if the apparently superfluous calls were removed. 
This might need further investigation.


Diffs
-----

  src/kicondialog.cpp cca4ed3 

Diff: https://git.reviewboard.kde.org/r/126750/diff/


Testing
-------

The "Browse..." sub dialog of the icon dialog works fine again in Dolphin and 
KWrite's file dialog when creating a new "Place".

I could not test if this affects Plasma somehow because I currently do not have 
a full self-built Plasma session running. It could probably be checked by 
opening the "Properties..." of a file in FolderView, clicking the icon, and 
then opening the "Browse..." sub dialog of the KIconDialog. This should 
hopefully not lock the entire Plasma session (because the dialogs are window 
modal, and not application modal). If anyone finds problems with that or has 
ideas for improvement, please let me know!


Thanks,

Frank Reininghaus

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to