Review Request 119243: Better OSX integration: native file dialogs and unified title/toolbar

[Ian Wadham]

On July 12, 2014, 2:11 p.m., Marko Käning wrote:
> > Other than that it looks ok. Please update the patch though.
> 
> Ian Wadham wrote:
>     I have tested the patch on Apple OS X in my kdesrc-build environment for KDE 4.13 branch. Before I did so, I removed the comment from line 316 and also the pair of braces from the line above it. Here are my results, using 3 apps for which I know the source code (I am the curent maintainer).
>     
>     MAC - Palapeli, Create Puzzle - m_imageSelector(new KUrlRequester) [1]
>     KDE - Palapeli, Import Puzzle(s) - KFileDialog::getOpenFileNames(KUrl("kfiledialog:///palapeli-import"), filter);
>     MAC - Palapeli, Export Puzzle(s) - KFileDialog::getSaveFileName(KUrl(startLoc), filter) [2]
>     
>     MAC - Kubrick, Load Puzzle - KFileDialog::getOpenFileName (KUrl(), "*.kbk", myParent, i18n("Load Puzzle"))
>     MAC - Kubrick, Save Puzzle - KFileDialog::getSaveFileName (KUrl(), "*.kbk", myParent, i18n("Save Puzzle"))
>     
>     KDE - KSudoku, Load - KFileDialog::getOpenUrl(KUrl("kfiledialog:///ksudoku"), QString(), this, i18n("Open Location"))
>     KDE - KSudoku, Save - KFileDialog::getSaveUrl(KUrl("kfiledialog:///ksudoku"))
>     
>     [1] KUrlRequester is a mini-dialog with a text-edit box and a button. The button brings up a dynamic KFileDialog window in native Mac style.
>     
>     [2] QString startLoc = QString::fromLatin1("kfiledialog:///palapeli-export/%1.puzzle").arg(cmp->metadata.name); but startLoc does not appear in the dialog's location box as a default name.
>     
>     In each case, you have the dialog style that appeared (MAC or KDE), followed by the program and action, the invocation of KFileDialog used and maybe a note.
>     
>     Without the patch, using my MacPorts installation as at KDE 4.12.5, *all* the above cases gave the KDE dialog.
>     
>     When I inserted Native=true in ~/Library/Preferences/KDE/share/config/kdeglobals (which is where app preferences are kept in Apple OS X and MacPorts), still without any patch, there were some changes in "Palapeli, Create", "Kubrick, Load" and "Kubrick, Save", but all the others stayed at KDE style. "Palapeli, Create" with just Native=true (no patch) displayed a dialog that was empty except for buttons Cancel and OK. Kubrick displayed a MAC dialog in both cases.
>     
>     This post by Boudewijn Rempt is also relevant to this somewhat confusing situation and shows some more test results on KFileDialog on various platforms --- http://mail.kde.org/pipermail/kde-mac/2014-July/001211.html --- that thread is where René originally raised this topic.
> 
> Thomas Lübking wrote:
>     The "problem" with the code is (likely) that "kfiledialog://" is not considered as "local" protocol, so actually the only thing that confuses me is that the patch turns "Palapeli, Export Puzzle" into a native dialog while the "Native" key does not.
>     
>     I assume this could be fixed (for KF5) by resolving "kfiledialog:///foobar" first and then passing the result to the native dialog (if a local protocol) - as is done for ::getSaveFileName, but not ::getSaveUrl or others.
>     The code in question is really messy in this context ;-)
>     
>     Defaulting "Native" as true on OSX (and windows?) should then be sufficient - I assume the conflicting behavior on ::getSaveFileName is due to the static flag, polluted by ::getOpenFileNames().

@Thomas: I put in some qDebug() statements to see what is happening in each case, with the patch installed but no Native=true, results were:

NB. bool condition is defined as (!startDir.isValid() || startDir.isLocalFile())

MAC - Palapeli, Create Puzzle - m_imageSelector(new KUrlRequester)
START DIR getOpenUrl KUrl("") isValid() false isLocalFile() false condition true
START DIR getOpenFileName KUrl("") isValid() false isLocalFile() false condition true

KDE - Palapeli, Import Puzzle - KFileDialog::getOpenFileNames(KUrl("kfiledialog:///palapeli-import"), filter);
START DIR getOpenFileNames KUrl("kfiledialog:/palapeli-import") isValid() true isLocalFile() false condition false

MAC - Palapeli, Export 2 Puzzles - KFileDialog::getSaveFileName(KUrl(startLoc), filter)
START DIR getSaveFileName KUrl("file:///Users/ianw/Desktop/Jigsaw Puzzles/") isValid() true isLocalFile() true condition true
START DIR getSaveFileName KUrl("file:///Users/ianw/Desktop/Jigsaw Puzzles/") isValid() true isLocalFile() true condition true

MAC - Kubrick, Load Puzzle - KFileDialog::getOpenFileName (KUrl(), ".kbk", myParent, i18n("Load Puzzle"))
START DIR getOpenFileName KUrl("") isValid() false isLocalFile() false condition true

MAC - Kubrick, Save Puzzle - KFileDialog::getSaveFileName (KUrl(), ".kbk", myParent, i18n("Save Puzzle"))
START DIR getSaveFileName KUrl("") isValid() false isLocalFile() false condition true 

KDE - KSudoku, Load - KFileDialog::getOpenUrl(KUrl("kfiledialog:///ksudoku"), QString(), this, i18n("Open Location"))
START DIR getOpenUrl KUrl("kfiledialog:/ksudoku") isValid() true isLocalFile() false condition false

KDE - KSudoku, Save - KFileDialog::getSaveUrl(KUrl("kfiledialog:///ksudoku"))
START DIR getSaveUrl KUrl("kfiledialog:/ksudoku") isValid() true isLocalFile() false condition false

I guess if "condition" is true, we get a native dialog, otherwise we get a KDE dialog.

Is what is happening any clearer?


- Ian


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


On July 14, 2014, 6:15 p.m., Marko Käning wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119243/
> -----------------------------------------------------------
> 
> (Updated July 14, 2014, 6:15 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, kdelibs, Christoph Feck, Ian Wadham, and RJVB Bertin.
> 
> 
> Bugs: 337356
>     http://bugs.kde.org/show_bug.cgi?id=337356
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This bundles both patches submitted by René J.V. Bertin in the associated issue
> 
> 
> Diffs
> -----
> 
>   kdeui/widgets/kmainwindow.cpp 85beaccdb6f123d2996b8c2b5e38435265c63ff8 
>   kio/kfile/kfiledialog.h 2b11796897ff095279e7c254a383a9e8e323ea0f 
>   kio/kfile/kfiledialog.cpp 4005ba304a18b59572cc1aece3fcd122ce05fda9 
> 
> Diff: https://git.reviewboard.kde.org/r/119243/diff/
> 
> 
> Testing
> -------
> 
> See issue for more info from René.
> 
> ---
> 
> I myself haven't yet tested this. Will report back as soon as I got to it.
> 
> 
> Thanks,
> 
> Marko Käning
> 
>

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