https://bugs.kde.org/show_bug.cgi?id=379615
Luigi Toscano <luigi.tosc...@tiscali.it> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Latest Commit| |https://commits.kde.org/gwe
| |nview/6ce5d2f8d82f83c5a3d72
| |6ee5807ebaf7a6e3c6c
Status|UNCONFIRMED |RESOLVED
--- Comment #2 from Luigi Toscano <luigi.tosc...@tiscali.it> ---
Git commit 6ce5d2f8d82f83c5a3d726ee5807ebaf7a6e3c6c by Luigi Toscano, on behalf
of Henrik Fehlauer.
Committed on 11/05/2017 at 20:43.
Pushed by ltoscano into branch 'Applications/17.04'.
Avoid data loss when importing pictures
Summary:
Fix porting regressions, which left users of Gwenview Importer with:
- failed import (import destination still empty)
- additionally (when choosing "Delete" instead of "Keep" after import):
pictures also removed from import source, with no way to recover
Correct additional problems remaining after fixing the import failure:
- hang on duplicate filenames
- identically named files with different content are never imported
- error dialog when deleting pictures from import source
In detail:
1st problem (introduced in 017b4fe5dc7f4b6e876cfd7b108dcebbf609ae94):
Initially, pictures are copied to a temporary folder
(e.g. "foo/.gwenview_importer-IcQqvo/"), before being moved/renamed
to the final destination (e.g. "foo/"), which is determined by
calling "cd .." on the temporary folder.
However, mistakenly this path contains a superfluous '/'
(e.g. "foo/.gwenview_importer-IcQqvo//"), resulting in the final
destination reading "foo/.gwenview_importer-IcQqvo/" instead of
"foo/". On cleanup, the temporary folder is removed, simultaneously
deleting all new pictures.
Fix: Omit '/' where appropriate.
2nd problem (broken by a3262e9b197ee97323ef8bf3a9dca1e13f61a74c):
When trying to find a unique filename, the while loop "stat"ing the
file repeats forever.
Fix: Call "KIO::stat" and "KJobWidgets::setWindow"
also inside the loop.
3rd problem (introduced in 017b4fe5dc7f4b6e876cfd7b108dcebbf609ae94):
If the destination directory already contains an identically named
file, we try to find a different name by appending a number. For
this, we need to strip the old filename from the full path.
Unfortunately, only calling "path()" is incorrect, giving
"foo/pict0001.jpg/pict0001_1.jpg", rather than "foo/pict0001_1.jpg".
Fix: Use "adjusted(QUrl::RemoveFilename)".
4th problem (broken by a3262e9b197ee97323ef8bf3a9dca1e13f61a74c):
Although deletion works, we show a warning dialog stating failure.
Fix: Invert the logic of "job->exec()", as the error handling should
be skipped by "break"ing out of the while loop.
Test Plan:
Steps to reproduce (see https://bugs.kde.org/show_bug.cgi?id=379615) no longer
result in data loss.
Autotests for importer (separate review request in D5750) pass. Hopefully, this
helps catching any future porting regressions.
Reviewers: ltoscano, sandsmark, gateau
Reviewed By: ltoscano
Differential Revision: https://phabricator.kde.org/D5749
M +4 -1 importer/fileutils.cpp
M +1 -1 importer/importdialog.cpp
M +2 -2 importer/importer.cpp
https://commits.kde.org/gwenview/6ce5d2f8d82f83c5a3d726ee5807ebaf7a6e3c6c
--
You are receiving this mail because:
You are watching all bug changes.
