Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
> On May 17, 2016, 4:29 p.m., Martin Gräßlin wrote:
> > The test does not verify the problem. I just pulled the patch, undid your
> > change, but the test passed nevertheless.
>
> Martin Gräßlin wrote:
> ah now I see. You adjusted the test application, but not the autotest.
>
> Jonathan Marten wrote:
> Now I see that there are autotests after all. How do I run them?
you can go to the build directory and just do:
make test
or just run the individial test binary created in build/autotests. The relevant
one would be kfiledialog_unittest and kfiledialogqml_unittest
- Martin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/#review95535
---
On May 17, 2016, 4:20 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127944/
> ---
>
> (Updated May 17, 2016, 4:20 p.m.)
>
>
> Review request for kde-workspace and Plasma.
>
>
> Bugs: 360666
> https://bugs.kde.org/show_bug.cgi?id=360666
>
>
> Repository: plasma-integration
>
>
> Description
> ---
>
> 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.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
> tests/qfiledialogtest.cpp 1d69ea1
>
> Diff: https://git.reviewboard.kde.org/r/127944/diff/
>
>
> Testing
> ---
>
> 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.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
> On May 17, 2016, 3:29 p.m., Martin Gräßlin wrote:
> > The test does not verify the problem. I just pulled the patch, undid your
> > change, but the test passed nevertheless.
>
> Martin Gräßlin wrote:
> ah now I see. You adjusted the test application, but not the autotest.
Now I see that there are autotests after all. How do I run them?
- Jonathan
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/#review95535
---
On May 17, 2016, 3:20 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127944/
> ---
>
> (Updated May 17, 2016, 3:20 p.m.)
>
>
> Review request for kde-workspace and Plasma.
>
>
> Bugs: 360666
> https://bugs.kde.org/show_bug.cgi?id=360666
>
>
> Repository: plasma-integration
>
>
> Description
> ---
>
> 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.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
> tests/qfiledialogtest.cpp 1d69ea1
>
> Diff: https://git.reviewboard.kde.org/r/127944/diff/
>
>
> Testing
> ---
>
> 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.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
> On May 17, 2016, 4:29 p.m., Martin Gräßlin wrote:
> > The test does not verify the problem. I just pulled the patch, undid your
> > change, but the test passed nevertheless.
ah now I see. You adjusted the test application, but not the autotest.
- Martin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/#review95535
---
On May 17, 2016, 4:20 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127944/
> ---
>
> (Updated May 17, 2016, 4:20 p.m.)
>
>
> Review request for kde-workspace and Plasma.
>
>
> Bugs: 360666
> https://bugs.kde.org/show_bug.cgi?id=360666
>
>
> Repository: plasma-integration
>
>
> Description
> ---
>
> 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.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
> tests/qfiledialogtest.cpp 1d69ea1
>
> Diff: https://git.reviewboard.kde.org/r/127944/diff/
>
>
> Testing
> ---
>
> 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.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/#review95535
---
The test does not verify the problem. I just pulled the patch, undid your
change, but the test passed nevertheless.
- Martin Gräßlin
On May 17, 2016, 4:20 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127944/
> ---
>
> (Updated May 17, 2016, 4:20 p.m.)
>
>
> Review request for kde-workspace and Plasma.
>
>
> Bugs: 360666
> https://bugs.kde.org/show_bug.cgi?id=360666
>
>
> Repository: plasma-integration
>
>
> Description
> ---
>
> 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.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
> tests/qfiledialogtest.cpp 1d69ea1
>
> Diff: https://git.reviewboard.kde.org/r/127944/diff/
>
>
> Testing
> ---
>
> 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.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
> On May 17, 2016, 2:35 p.m., Martin Gräßlin wrote:
> > 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.
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.
- Jonathan
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/#review95532
---
On May 17, 2016, 3:20 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127944/
> ---
>
> (Updated May 17, 2016, 3:20 p.m.)
>
>
> Review request for kde-workspace and Plasma.
>
>
> Bugs: 360666
> https://bugs.kde.org/show_bug.cgi?id=360666
>
>
> Repository: plasma-integration
>
>
> Description
> ---
>
> 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.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
> tests/qfiledialogtest.cpp 1d69ea1
>
> Diff: https://git.reviewboard.kde.org/r/127944/diff/
>
>
> Testing
> ---
>
> 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.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/
---
(Updated May 17, 2016, 3:20 p.m.)
Review request for kde-workspace and Plasma.
Changes
---
Added test
Bugs: 360666
https://bugs.kde.org/show_bug.cgi?id=360666
Repository: plasma-integration
Description
---
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.
Diffs (updated)
-
src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
tests/qfiledialogtest.cpp 1d69ea1
Diff: https://git.reviewboard.kde.org/r/127944/diff/
Testing
---
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.
Thanks,
Jonathan Marten
Re: Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/#review95532
---
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.
- Martin Gräßlin
On May 17, 2016, 3:29 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127944/
> ---
>
> (Updated May 17, 2016, 3:29 p.m.)
>
>
> Review request for kde-workspace and Plasma.
>
>
> Bugs: 360666
> https://bugs.kde.org/show_bug.cgi?id=360666
>
>
> Repository: plasma-integration
>
>
> Description
> ---
>
> 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.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
>
> Diff: https://git.reviewboard.kde.org/r/127944/diff/
>
>
> Testing
> ---
>
> 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.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Review Request 127944: KDE Platform Theme: set file dialog overwrite option appropriately for saving
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127944/
---
Review request for kde-workspace and Plasma.
Bugs: 360666
https://bugs.kde.org/show_bug.cgi?id=360666
Repository: plasma-integration
Description
---
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.
Diffs
-
src/platformtheme/kdeplatformfiledialoghelper.cpp 139c35d
Diff: https://git.reviewboard.kde.org/r/127944/diff/
Testing
---
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.
Thanks,
Jonathan Marten
File overwrite warning missing?
Hi, Users with the latest plasma/frameworks who build Krita themselves are reporting that the file dialog no longer warns about overwriting existing files: https://bugs.kde.org/show_bug.cgi?id=360666 I'm not sure what's going on here, the warning correctly appears with the GTK file dialog, the Qt file dialog, the Windows file dialog and the OSX file dialog. -- Boudewijn Rempt | http://www.krita.org, http://www.valdyas.org
