mardelle added a comment.


  The unittest is in another diff because it's a different author I guess. I can process the comments but looking closer I found 2 other important issues in this class:
  
  1. When using KAutoSaveFile::staleFiles(filename) to check if there is an existing stale file for a document, we use the function KAutoSaveFile::extractManagedFilePath
  
  This function compares the orginal document path one reconstructed from the lock file name. However this logic seems flawed since the lock file name uses a trimmed path in some cases it is impossible to rebuild the original document's path from it.
  Instead I would propose to replace extractManagedFilePath with a new function, comparing filename, then paths like:
  
    bool staleMatchesManaged(const QString& staleFileName, const QString managedFile)
    {
        const QStringRef sep = staleFileName.rightRef(3);
        int sepPos = staleFileName.indexOf(sep);
        if (QFileInfo(managedFile).fileName() != staleFileName.leftRef(sepPos).toString()) {
            // File names don't match
            return false;
        }
        int pathPos = staleFileName.indexOf(QChar::fromLatin1('_'), sepPos);
        const QByteArray encodedPath = staleFileName.midRef(pathPos+1, staleFileName.length()-pathPos-1-KAutoSaveFilePrivate::NamePadding).toLatin1();
        const QString sourcePathStart = QUrl::fromPercentEncoding(encodedPath);
        return QFileInfo(managedFile).absolutePath().startsWith(sourcePathStart);
    }
  
  
  
  2. The QLockFile class used in KAutoSaveFile creates a temporary file to manage locking:
  
  > QLockFile rmlock(d->fileName + QLatin1String(".rmlock"));
  
  https://github.com/qt/qtbase/blob/dev/src/corelib/io/qlockfile.cpp#L259
  
  This means that in our situation where we have a filename that already has the maximum length, trying to lock will always fail, preventing usage of the autosave file.
  Easier solution is probably to make maximum filename in KAutoSave File::tempFileName() a bit shorter.
  
  Thoughts? Should I prepare separate patches ? But changing the fileName encoding as suggested:
  
  > Simpler than toPercentEncoding: use QUrl::fileName(QUrl::FullyEncoded).
  
  Might break more the path recovery method extractManagedFilePath..

REPOSITORY
  R244 KCoreAddons

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

To: mardelle, #frameworks, dfaure, mpyne
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns