| Summary: | Data loss when importing pictures | ||
|---|---|---|---|
| Product: | [Applications] gwenview | Reporter: | null <null> |
| Component: | importer | Assignee: | Gwenview Bugs <gwenview-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | myriam, simonandric5 |
| Priority: | NOR | ||
| Version: | 17.04.0 | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | https://commits.kde.org/gwenview/6ce5d2f8d82f83c5a3d726ee5807ebaf7a6e3c6c | Version Fixed In: | 17.04.2 |
Proposal for fix: https://phabricator.kde.org/D5749 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
|
With KDE Applications 17.04, Gwenview Importer was reintroduced into the KF5 world (yay!). However, using it to import pictures e.g. from a memory card, camera device or local directory does not work. At the same time, choosing to "delete the <N> imported documents from the device" after the import "finished", works rather well. This leaves us with no pictures on both the device and the import destination, potentially calling for a lengthy recovery session with PhotoRec. Note: This not only affects users manually calling the importer, but everyone using the import action provided by the device notifier popup when inserting external media. Steps to reproduce: - create backup copy of <folder with pictures> - $ gwenview_importer <folder with pictures> - choose <destination> - click on "Import All" - (notice how <destination> is still empty) - click on "Delete" - (notice how <folder with pictures> is now empty, too) - dismiss warning ("Failed to delete document: Unknown error code 0"), which actually means "Failed to import, succeeded to delete" Expected result: Pictures moved (or copied, when choosing "Keep" instead of "Delete") from <folder with pictures> to <destination>.