Skip to content

[Needs testing]: Fix no warning for overwriting files in non-native dialogs

This fixes bug 412651.

Before this commit, in case a user wanted to Save As "filename", Krita would add an extension based on the selected filter. In native file dialogs, Krita will warn user if it would overwrite the file. In non-native file dialogs though (based on Qt widgets), it wouldn't happen.

This commit makes it possible to append the extension before the non-native file dialog is closed, which enables warning before overwriting the file inside the file dialog.

This commit applies the patch from Qt bug report: https://bugreports.qt.io/browse/QTBUG-27186 https://codereview.qt-project.org/c/qt/qtbase/+/109815/

Author of the patch and info from the original commit: Friedemann Kleint Friedemann.Kleint@theqtcompany.com Task-number: QTBUG-27186 Change-Id: I3ece055b328dfe361b93e68954cb0a33bd9e7d94 "The Windows native dialog prefers the name filter over the defaultSuffix (uses the same mechanism), I chose the convervative approach here (defaultSuffix takes precedence)."

Since the fix in the file dialog that was implemented is based on extensions mentioned in the filter, the filter needs to show the extensions... It's a bit of an unwanted dependency between view and logic, so it would be good to find a way to implement it wiser.

Things to do (from discussion):

  • default extension when saving should be .kra, not .exr
  • it should check for existing files for the jp(e)g extension
  • it should check for the current filename for the jp(e)g extension

Test Plan

  1. Preparation:
  • save a file as savingtest.kra, savingtest.png, savingtest.exr, savingtest.jpeg (important! it must be .jpeg for tests in 3.).
  • make sure you have non-native file dialogs selected in your Krita settings. Although for testing, it would be great if you checked both options.
  1. Bug:
  • a. open savingtest.kra. File -> Save As -> remove the extension. Expected: warning about overwrite.
  • b. open savingtest.kra. File -> Save As -> remove the extension. Change the filter to PNG files. Expected: warning about overwrite.
  • c. just as b., but click OK and make sure the timestamp of the file changed, which means Krita saved as .png file. (You can also check in krita.log)
  • d. Save something non-existing yet, for example savingtest1. Check if Krita saved with the correct extension (from the selected filter).
  • e. Save using All filter. It should save as .kra and suggest this exact extension when switching filters.
  1. Regressions:
  • a. try to save as savingtest.png, savingtest.kra and savingtest.exr. Make sure to write the extensions down. Make sure the correct filter is selected. Expected: warning about overwriting the file. Click Ok anyway.
  • b. Just as 2b., but make sure the incorrect filter is selected. Compare with krita.log to see what Krita actually did (or check timestamps). Expected: warning about overwriting the file with the correct extension. Click Ok anyway.
  • c. Try to save as savingtest2.kra, savingtest2.png etc., with extensions. Check two options, with incorrect and correct filter being selected. Expected: no overwrite warnings (the point of this subpoint is to save a new non-existing file).
  1. Jpeg/Jpg:
  • a. try to save as "savingtest" and select JPG filter. Expected: warning about overwrite.
  • b. try to save as "savingtest.jpg". No matter the filter. Expected: it won't warn you that savingtest.jpeg already exists and it just save it alongside the .jpg file.
  • c. open savingtest.jpeg. Click "Save As". It should show the .jpeg extension.
  • d. If you open Save As for savingtest.kra and change the filter to JPG, it should show the existing extension (.jpeg). (If there is no .jpeg file in the folder, it should stay .jpg).
  • e. Save savingtest.jpeg in another directory that you'll save to. Make sure the directory you'll save to doesn't have any .jp(e)g files. Open savingtest.jpeg, try to save. It should show .jpeg extension.
  • f. e but save with .jpg extension (write it down yourself); it should save as .jpg.
  1. (will fail)
  • a. [will fail] e but switch filters; after coming back to JPG, it should still show .jpeg.
  • b. [will fail] e but save without extension using the JPG filter; it should save as .jpeg.

Tests

Test Windows NN Windows N Linux N Linux NN Mac N Mac NN Android N Android NN
1a + + +
1b + + +
1c + + +
1d + + +
1e + + +
2 --- --- --- --- --- --- --- ---
2a + + +
2b + + +
2c + + +
3 --- --- --- --- --- --- --- ---
3a + - +
3b + - +
3c + - +
3d + - +
3e + -
3f + +
3 --- --- --- --- --- --- --- ---
3e - -
3f - -

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (Are there any? I've seen Qt's test, the patch actually adds some more, but... I'm not sure where are they, how to run them etc.)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Edited by Agata Cacko

Merge request reports