<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/127944/">https://git.reviewboard.kde.org/r/127944/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 17th, 2016, 2:35 p.m. BST, <b>Martin Gräßlin</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 change looks sensible to me, but I would appreciate a test case for it. There are already some tests for the file dialog, so it should be easy to extend.</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;">There is no autotest, but have updated tests/qfiledialogtest with options '--confirmOverwrite' (on or off) and '--nativeDialog' (on or off). Checked operation of these options and overwrite warning.</pre>
<br />
<p>- Jonathan</p>
<br />
<p>On May 17th, 2016, 3:20 p.m. BST, Jonathan Marten 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-workspace and Plasma.</div>
<div>By Jonathan Marten.</div>
<p style="color: grey;"><i>Updated May 17, 2016, 3:20 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="https://bugs.kde.org/show_bug.cgi?id=360666">360666</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-integration
</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;">The referenced bug says that, by default, there is no file overwrite check when using QFileDialog to save a file. Indeed, on closer investigating it appears that there is no way to even explictly force an overwrite check when using the KDE platform theme, because of this code in plasma-integration/src/platformtheme/kdeplatformfiledialoghelper.cpp:
// overwrite option
if (options()->testOption(QFileDialogOptions::FileDialogOption::DontConfirmOverwrite)) {
dialog->m_fileWidget->setConfirmOverwrite(false);
}
The default for KFileWidget is already for no overwrite check (as set in kio/src/filewidgets/kfilewidget.cpp which initialises KFileWidgetPrivate::confirmOverwrite to false). There is no way to override this from the calling application through the platform plugin.
Suggest that the default option should be the same as that defined by Qt for QFileDialog: always perform an ovewrwrite check on saving, unless the caller has set the QFileDialog::DontConfirmOverwrite option. This is also a sensible default to have from the user's point of view. This change does that in the platform theme plugin, for all saving operations.</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;">Built plasma-intergration with this change, confirmed correct operation of file dialogues and that confirmation is requested when overwriting an existing file, unless the QFileDialog::DontConfirmOverwrite option is specified.</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>src/platformtheme/kdeplatformfiledialoghelper.cpp <span style="color: grey">(139c35d)</span></li>
<li>tests/qfiledialogtest.cpp <span style="color: grey">(1d69ea1)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127944/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>