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
>
>
