<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119243/">https://git.reviewboard.kde.org/r/119243/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 12th, 2014, 2:11 p.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Other than that it looks ok. Please update the patch though.</p></pre>
</blockquote>
<p>On July 14th, 2014, 8:21 a.m. UTC, <b>Ian Wadham</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">MAC - Palapeli, Create Puzzle - m_imageSelector(new KUrlRequester) [1]<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
KDE - Palapeli, Import Puzzle(s) - KFileDialog::getOpenFileNames(KUrl("kfiledialog:///palapeli-import"), filter);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
MAC - Palapeli, Export Puzzle(s) - KFileDialog::getSaveFileName(KUrl(startLoc), filter) [2]</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">MAC - Kubrick, Load Puzzle - KFileDialog::getOpenFileName (KUrl(), "<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">.kbk", myParent, i18n("Load Puzzle"))<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
MAC - Kubrick, Save Puzzle - KFileDialog::getSaveFileName (KUrl(), "</em>.kbk", myParent, i18n("Save Puzzle"))</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDE - KSudoku, Load - KFileDialog::getOpenUrl(KUrl("kfiledialog:///ksudoku"), QString(), this, i18n("Open Location"))<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
KDE - KSudoku, Save - KFileDialog::getSaveUrl(KUrl("kfiledialog:///ksudoku"))</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Without the patch, using my MacPorts installation as at KDE 4.12.5, <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">all</em> the above cases gave the KDE dialog.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On July 15th, 2014, 10:37 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The code in question is really messy in this context ;-)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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().</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">@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:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">NB. bool condition is defined as (!startDir.isValid() || startDir.isLocalFile())</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">MAC - Palapeli, Create Puzzle - m_imageSelector(new KUrlRequester)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getOpenUrl KUrl("") isValid() false isLocalFile() false condition true<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getOpenFileName KUrl("") isValid() false isLocalFile() false condition true</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDE - Palapeli, Import Puzzle - KFileDialog::getOpenFileNames(KUrl("kfiledialog:///palapeli-import"), filter);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getOpenFileNames KUrl("kfiledialog:/palapeli-import") isValid() true isLocalFile() false condition false</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">MAC - Palapeli, Export 2 Puzzles - KFileDialog::getSaveFileName(KUrl(startLoc), filter)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getSaveFileName KUrl("file:///Users/ianw/Desktop/Jigsaw Puzzles/") isValid() true isLocalFile() true condition true<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getSaveFileName KUrl("file:///Users/ianw/Desktop/Jigsaw Puzzles/") isValid() true isLocalFile() true condition true</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">MAC - Kubrick, Load Puzzle - KFileDialog::getOpenFileName (KUrl(), ".kbk", myParent, i18n("Load Puzzle"))<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getOpenFileName KUrl("") isValid() false isLocalFile() false condition true</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">MAC - Kubrick, Save Puzzle - KFileDialog::getSaveFileName (KUrl(), ".kbk", myParent, i18n("Save Puzzle"))<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getSaveFileName KUrl("") isValid() false isLocalFile() false condition true </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDE - KSudoku, Load - KFileDialog::getOpenUrl(KUrl("kfiledialog:///ksudoku"), QString(), this, i18n("Open Location"))<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getOpenUrl KUrl("kfiledialog:/ksudoku") isValid() true isLocalFile() false condition false</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDE - KSudoku, Save - KFileDialog::getSaveUrl(KUrl("kfiledialog:///ksudoku"))<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
START DIR getSaveUrl KUrl("kfiledialog:/ksudoku") isValid() true isLocalFile() false condition false</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I guess if "condition" is true, we get a native dialog, otherwise we get a KDE dialog.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is what is happening any clearer?</p></pre>
<br />
<p>- Ian</p>
<br />
<p>On July 14th, 2014, 6:15 p.m. UTC, Marko Käning wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Software on Mac OS X, kdelibs, Christoph Feck, Ian Wadham, and RJVB Bertin.</div>
<div>By Marko Käning.</div>
<p style="color: grey;"><i>Updated July 14, 2014, 6:15 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=337356">337356</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This bundles both patches submitted by René J.V. Bertin in the associated issue</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">See issue for more info from René.</p>
<hr style="text-rendering: inherit;margin: 0;padding: 0;white-space: normal;border: 1px solid #ddd;line-height: inherit;" />
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I myself haven't yet tested this. Will report back as soon as I got to it.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdeui/widgets/kmainwindow.cpp <span style="color: grey">(85beaccdb6f123d2996b8c2b5e38435265c63ff8)</span></li>
<li>kio/kfile/kfiledialog.h <span style="color: grey">(2b11796897ff095279e7c254a383a9e8e323ea0f)</span></li>
<li>kio/kfile/kfiledialog.cpp <span style="color: grey">(4005ba304a18b59572cc1aece3fcd122ce05fda9)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119243/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>