[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request 119243: Better OSX integration: native file dialogs and unified title/toolbar
From: Thomas_Lübking <thomas.luebking () gmail ! com>
Date: 2014-07-21 11:52:19
Message-ID: 20140721115219.14617.36643 () probe ! kde ! org
[Download RAW message or body]
--===============4981845826008580319==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Juli 21, 2014, 5:16 vorm., Ian Wadham wrote:
> > So where are we leaving this? Any conclusions? Any solutions or further patches?
> >
> > Should we report the inconsistent handling of Native file dialogs as a bug on \
> > bugs.kde.org (without a patch this time)?
> > If so, against what "product" should we report it (i.e. where a KDE developer \
> > will be listening)?
> > I agree with Thomas that "kfiledialog://" should be evaluated before deciding \
> > whether it is a local file.
> > I notice also, that in the "condition" that keeps occurring ((!startDir.isValid() \
> > || startDir.isLocalFile()), the isValid() method seems to be inherited from QUrl \
> > class and maybe isLocalFile() is too.
> > It seems odd that a possible condition for invoking QFileDialog methods should be \
> > that the QUrl is invalid. Perhaps there used to be an isValid() in KUrl with a \
> > different meaning.
> > It's all very confusing. Who can sort it out?
dfaure (assined to the kio framework)
kfiledialog.cpp would require a general cleanup, but it's in kde4support now, so the \
predominant question is whether it makes any sense to spend time on this at all.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119243/#review62755
-----------------------------------------------------------
On Juli 14, 2014, 6:15 nachm., Marko Käning wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119243/
> -----------------------------------------------------------
>
> (Updated Juli 14, 2014, 6:15 nachm.)
>
>
> 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
>
>
--===============4981845826008580319==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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 Juli 21st, 2014, 5:16 vorm. 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;">So where are we leaving this? Any conclusions? Any \
solutions or further patches?</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Should we report the \
inconsistent handling of Native file dialogs as a bug on bugs.kde.org (without a \
patch this time)?</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">If so, against what "product" should we \
report it (i.e. where a KDE developer will be listening)?</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
agree with Thomas that "kfiledialog://" should be evaluated before deciding whether \
it is a local file.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I notice also, that in the "condition" \
that keeps occurring ((!startDir.isValid() || startDir.isLocalFile()), the isValid() \
method seems to be inherited from QUrl class and maybe isLocalFile() is too.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">It seems odd that a possible condition for invoking QFileDialog methods \
should be that the QUrl is invalid. Perhaps there used to be an isValid() in KUrl \
with a different meaning.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">It's all very confusing. Who can sort \
it out?</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;">dfaure (assined to the kio framework)</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">kfiledialog.cpp would require a general cleanup, but it's in kde4support \
now, so the predominant question is whether it makes any sense to spend time on this \
at all.</p></pre> <br />
<p>- Thomas</p>
<br />
<p>On Juli 14th, 2014, 6:15 nachm. 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 Juli 14, 2014, 6:15 nachm.</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>
--===============4981845826008580319==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic