Review Request 121098: Restore filedialog functionality for modal dialogs.

[Martin Klapetek]

> On Nov. 15, 2014, 12:16 a.m., Martin Klapetek wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, lines 301-303
> > <https://git.reviewboard.kde.org/r/121098/diff/1/?file=327582#file327582line301>
> >
> >     Looking at QDialog docs, it says
> >     
> >     "modal : bool
> >     This property holds whether show() should pop up the dialog as modal or modeless."
> >     
> >     So as the modal is being set here, shouldn't the "if (windowModality == Qt::NonModal) {" just be removed instead and always call show()?
> 
> Jeremy Whiting wrote:
>     Yep, I've tried just removing the if, that makes it appear when the static QFileDialog methods are called, but you can't interact with it. Clicking anywhere in the dialog itself does nothing. As reported here: https://bugs.kde.org/show_bug.cgi?id=334963
> 
> Martin Klapetek wrote:
>     The main issue I see here is that show() is always expected to not block; if the QML dialogs wanted to use the blocking version, it would internally just call exec() rather than show(). So putting blocking exec() into show() is imho not right.
>     
>     Have you tried investigating why the dialog cannot be interacted with? I can have a look if you want
> 
> Jeremy Whiting wrote:
>     I think removing the if is the right solution, but I've no idea where to start looking to figure out why it's not interactive. If you've got some ideas I can do the digging, otherwise you can look into it. I'm fine either way, just want to get the underlying issue solved.
> 
> Martin Klapetek wrote:
>     Ok, I'll have a look and post back.

I found it. It has to do with event loops. So I looked at how the gtk2 plugin works and they call QGuiApplicationPrivate::showModalWindow(..) which does not block. I tried it and everything works correctly. BUT it's a private API (with installed header though)...anyways here's the relevant code:

    #include <private/qguiapplication_p.h>
    ...
    // ensure that window exists
    m_dialog->winId();

    if (windowModality == Qt::NonModal) {
        m_dialog->show();
    } else {
        QGuiApplicationPrivate::showModalWindow(m_dialog->windowHandle());
    }

    KSharedConfig::Ptr conf = KSharedConfig::openConfig();
    KWindowConfig::restoreWindowSize(m_dialog->windowHandle(), conf->group("FileDialogSize"));


- Martin


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


On Nov. 10, 2014, 7:17 p.m., Jeremy Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121098/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 7:17 p.m.)
> 
> 
> Review request for KDE Frameworks and Eike Hein.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> -------
> 
> Before David's astyle commit, the show method used to call m_dialog->exec
> if the window flags weren't nonmodal (i.e. modal dialogs). This got removed somehow.
> BUG:334963
> 
> 
> Diffs
> -----
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 44eca192946f0da2b357b33e93a57ef0de05135b 
> 
> Diff: https://git.reviewboard.kde.org/r/121098/diff/
> 
> 
> Testing
> -------
> 
> fifteen puzzle config with patch from https://git.reviewboard.kde.org/r/121097/ now shows the dialog and lets you choose a file.
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141117/838b4679/attachment-0001.html>