D8084: KAutoSaveFile breaks if source file name contains a space!

2017-11-05 Thread Jean-Baptiste Mardelle 
This revision was automatically updated to reflect the committed changes.
Closed by commit R244:8ed107304694: Fix KAutoSave bug on file with white space 
in it (authored by mardelle).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8084?vs=21868=21898

REVISION DETAIL
  https://phabricator.kde.org/D8084

AFFECTED FILES
  autotests/kautosavefiletest.cpp
  src/lib/io/kautosavefile.cpp

To: mardelle, #frameworks, shaforostoff, dfaure
Cc: dfaure, ngraham, cfeck, ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-11-05 Thread David Faure 
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D8084

To: mardelle, #frameworks, shaforostoff, dfaure
Cc: dfaure, ngraham, cfeck, ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-11-04 Thread Jean-Baptiste Mardelle 
mardelle marked 2 inline comments as done.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D8084

To: mardelle, #frameworks, shaforostoff
Cc: dfaure, ngraham, cfeck, ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-11-04 Thread Jean-Baptiste Mardelle 
mardelle updated this revision to Diff 21868.

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8084?vs=20216=21868

REVISION DETAIL
  https://phabricator.kde.org/D8084

AFFECTED FILES
  autotests/kautosavefiletest.cpp
  src/lib/io/kautosavefile.cpp

To: mardelle, #frameworks, shaforostoff
Cc: dfaure, ngraham, cfeck, ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-11-01 Thread David Faure 
dfaure added a comment.


  Thanks for the fix and autotest !

INLINE COMMENTS

> kautosavefiletest.cpp:83
>  {
> -// TODO
> +QUrl normalFile = QUrl::fromLocalFile(QStringLiteral("/tmp/test 
> directory/tîst me.txt"));
> +

Should use QDir::tempPath() for portability.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D8084

To: mardelle, #frameworks, shaforostoff
Cc: dfaure, ngraham, cfeck, ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-10-17 Thread Christoph Feck 
cfeck added inline comments.

INLINE COMMENTS

> kautosavefiletest.cpp:87
> +QVERIFY(saveFile.open(QIODevice::ReadWrite));
> +saveFile.write(QStringLiteral("testdata").toUtf8());
> +// Make sure the stale file is found

saveFile.write("testdata") is sufficient.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D8084

To: mardelle, #frameworks, shaforostoff
Cc: cfeck, ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-10-01 Thread Jean-Baptiste Mardelle 
mardelle updated this revision to Diff 20216.
mardelle added a comment.


  Added test to check stale file is correctly found

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8084?vs=20180=20216

REVISION DETAIL
  https://phabricator.kde.org/D8084

AFFECTED FILES
  autotests/kautosavefiletest.cpp
  src/lib/io/kautosavefile.cpp

To: mardelle, #frameworks, shaforostoff
Cc: ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-10-01 Thread Luigi Toscano 
ltoscano added a comment.


  I don't know the code too much, but would it be possible to add an autotest 
for this?

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D8084

To: mardelle, #frameworks, shaforostoff
Cc: ltoscano

D8084: KAutoSaveFile breaks if source file name contains a space!

2017-10-01 Thread Jean-Baptiste Mardelle 
mardelle created this revision.
mardelle added reviewers: Frameworks, shaforostoff.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  KAutoSaveFile does not correctly handle simple characters like spaces in file 
names!!!
  When creating a stale file (backup file) , to determine the file name it uses:
  in tempFileName(), line 83:
  return QString::fromLatin1(QUrl::toPercentEncoding(name).constData());
  
  So the filename is encoded using percent encoding. But when checking for an 
existing backup file, it does:
  in extractManagedFilePath, line 180:
  managedFileName.setPath(QUrl::fromPercentEncoding(encodedPath) + 
QLatin1Char('/') + QFileInfo(staleFileName.left(sepPos)).fileName());
  
  So the path part is correctly restored from percent encoding, but the 
filename part is not decoded.
  So we end up comparing "my file" with "my%20file" and the stale file is not 
correctly identified!

TEST PLAN
  Project files with space in their name in Kdenlive now have crash recovery 
working

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D8084

AFFECTED FILES
  src/lib/io/kautosavefile.cpp

To: mardelle, #frameworks, shaforostoff