> 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. > > Jonathan Marten wrote: > Now I see that there are autotests after all. How do I run them? > > Martin Gräßlin wrote: > 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 > > Jonathan Marten wrote: > Ok, found them and how - thanks. > > It doesn't appear to be possible to just check that the QFileDialog > option is passed correctly through to the file widget, because KFileWidget > has no way to read back the option set by setConfirmOverwrite. So it won't > be a simple test like the setFileMode tests - it will have to look for the > message box being shown. Is that worth doing (and reliable enough)? > > Martin Gräßlin wrote: > hmm yeah, tricky. I think it would be ok to check for the messagebox > being shown as that's kind of also how the autotest for the dialog works in > general. But I also don't have an idea on how to check whether the messagebox > got opened. So maybe just push without it. > > Btw. please push for the Plasma/5.6 branch.
Ok, have worked out an autotest. It's not pretty, but it does work - passing with the modified platform theme and failing with the original. It puts up a save box for a file which already exists, clicks the OK button and checks that a message box appears or not according to the QFileDialog option. The message box has to be checked in a QTimer slot because, if it does appear, it runs its own event loop which halts the test until the user clicks one of its buttons. Go easy with it - it's my first ever Frameworks autotest :-) - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127944/#review95535 ----------------------------------------------------------- On May 18, 2016, 11:07 a.m., Jonathan Marten wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127944/ > ----------------------------------------------------------- > > (Updated May 18, 2016, 11:07 a.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 > ----- > > autotests/kfiledialog_unittest.cpp 59915da > 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 > >
