Merge pull request #7502 from nextcloud/backport/7238/stable-3.14

[stable-3.14] Always remove folder icon when removing folder.

Author: mgallien

src/common/utility.h

@@ -66,8 +66,19 @@ namespace Utility {
     OCSYNC_EXPORT void sleep(int sec);
     OCSYNC_EXPORT void usleep(int usec);
     OCSYNC_EXPORT QString formatFingerprint(const QByteArray &, bool colonSeparated = true);
+    /**
+     * @brief Creates the Desktop.ini file which contains the folder IconResource shown as a favorite link
+     *
+     * @param folder absolute file path to folder
+     */
     OCSYNC_EXPORT void setupFavLink(const QString &folder);
+    /**
+     * @brief Removes the Desktop.ini file which contains the folder IconResource shown as a favorite link
+     *
+     * @param folder absolute file path to folder
+     */
     OCSYNC_EXPORT void removeFavLink(const QString &folder);
+
     OCSYNC_EXPORT bool writeRandomFile(const QString &fname, int size = -1);
     OCSYNC_EXPORT QString octetsToString(const qint64 octets);
     OCSYNC_EXPORT QByteArray userAgentString();

src/common/utility_win.cpp

@@ -139,21 +139,25 @@ void Utility::setupFavLink(const QString &folder)
         SetFileAttributesW((wchar_t *)desktopIni.fileName().utf16(), FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM);
     }
 
-    // Windows Explorer: Place under "Favorites" (Links)
-    QString linkName;
-    QDir folderDir(QDir::fromNativeSeparators(folder));
-
     /* Use new WINAPI functions */
     PWSTR path;
-
-    if (SHGetKnownFolderPath(FOLDERID_Links, 0, nullptr, &path) == S_OK) {
-        QString links = QDir::fromNativeSeparators(QString::fromWCharArray(path));
-        linkName = QDir(links).filePath(folderDir.dirName() + QLatin1String(".lnk"));
-        CoTaskMemFree(path);
+    if (!SHGetKnownFolderPath(FOLDERID_Links, 0, nullptr, &path) == S_OK) {
+        qCWarning(lcUtility) << "SHGetKnownFolderPath for " << folder << "has failed.";
+        return;
     }
+
+    // Windows Explorer: Place under "Favorites" (Links)
+    const auto links = QDir::fromNativeSeparators(QString::fromWCharArray(path));
+    CoTaskMemFree(path);
+
+    const QDir folderDir(QDir::fromNativeSeparators(folder));
+    const QString filePath = folderDir.dirName() + QLatin1String(".lnk");
+    const auto linkName = QDir(links).filePath(filePath);
+
     qCInfo(lcUtility) << "Creating favorite link from" << folder << "to" << linkName;
-    if (!QFile::link(folder, linkName))
+    if (!QFile::link(folder, linkName)) {
         qCWarning(lcUtility) << "linking" << folder << "to" << linkName << "failed!";
+    }
 }
 
 void Utility::removeFavLink(const QString &folder)

src/gui/accountsettings.cpp

@@ -874,7 +874,6 @@ void AccountSettings::slotRemoveCurrentFolder()
         messageBox->addButton(tr("Cancel"), QMessageBox::NoRole);
         connect(messageBox, &QMessageBox::finished, this, [messageBox, yesButton, folder, row, this]{
             if (messageBox->clickedButton() == yesButton) {
-                Utility::removeFavLink(folder->path());
                 FolderMan::instance()->removeFolder(folder);
                 _model->removeRow(row);
 

src/gui/folderman.cpp

@@ -1327,40 +1327,43 @@ QStringList FolderMan::findFileInLocalFolders(const QString &relPath, const Acco
     return re;
 }
 
-void FolderMan::removeFolder(Folder *f)
+void FolderMan::removeFolder(Folder *folderToRemove)
 {
-    if (!f) {
+    if (!folderToRemove) {
         qCCritical(lcFolderMan) << "Can not remove null folder";
         return;
     }
 
-    qCInfo(lcFolderMan) << "Removing " << f->alias();
+    qCInfo(lcFolderMan) << "Removing " << folderToRemove->alias();
 
-    const bool currentlyRunning = f->isSyncRunning();
+    const bool currentlyRunning = folderToRemove->isSyncRunning();
     if (currentlyRunning) {
         // abort the sync now
-        f->slotTerminateSync();
+        folderToRemove->slotTerminateSync();
     }
 
-    if (_scheduledFolders.removeAll(f) > 0) {
+    if (_scheduledFolders.removeAll(folderToRemove) > 0) {
         emit scheduleQueueChanged();
     }
 
-    f->setSyncPaused(true);
-    f->wipeForRemoval();
+    folderToRemove->setSyncPaused(true);
+    folderToRemove->wipeForRemoval();
 
     // remove the folder configuration
-    f->removeFromSettings();
+    folderToRemove->removeFromSettings();
+
+    // remove Desktop.ini
+    Utility::removeFavLink(folderToRemove->path());
 
-    unloadFolder(f);
+    unloadFolder(folderToRemove);
     if (currentlyRunning) {
         // We want to schedule the next folder once this is done
-        connect(f, &Folder::syncFinished,
+        connect(folderToRemove, &Folder::syncFinished,
             this, &FolderMan::slotFolderSyncFinished);
         // Let the folder delete itself when done.
-        connect(f, &Folder::syncFinished, f, &QObject::deleteLater);
+        connect(folderToRemove, &Folder::syncFinished, folderToRemove, &QObject::deleteLater);
     } else {
-        delete f;
+        delete folderToRemove;
     }
 
     _navigationPaneHelper.scheduleUpdateCloudStorageRegistry();

src/gui/folderman.h

@@ -101,7 +101,7 @@ class FolderMan : public QObject
     Folder *addFolder(AccountState *accountState, const FolderDefinition &folderDefinition);
 
     /** Removes a folder */
-    void removeFolder(Folder *);
+    void removeFolder(Folder *folderToRemove);
 
     /** Returns the folder which the file or directory stored in path is in */
     Folder *folderForPath(const QString &path);

src/gui/navigationpanehelper.cpp

@@ -41,23 +41,26 @@ NavigationPaneHelper::NavigationPaneHelper(FolderMan *folderMan)
 
 void NavigationPaneHelper::setShowInExplorerNavigationPane(bool show)
 {
-    if (_showInExplorerNavigationPane == show)
+    if (_showInExplorerNavigationPane == show) {
         return;
+    }
 
     _showInExplorerNavigationPane = show;
     // Re-generate a new CLSID when enabling, possibly throwing away the old one.
     // updateCloudStorageRegistry will take care of removing any unknown CLSID our application owns from the registry.
-    foreach (Folder *folder, _folderMan->map())
+    for (const auto &folder : qAsConst(_folderMan->map())) {
         folder->setNavigationPaneClsid(show ? QUuid::createUuid() : QUuid());
+    }
 
     scheduleUpdateCloudStorageRegistry();
 }
 
 void NavigationPaneHelper::scheduleUpdateCloudStorageRegistry()
 {
     // Schedule the update to happen a bit later to avoid doing the update multiple times in a row.
-    if (!_updateCloudStorageRegistryTimer.isActive())
+    if (!_updateCloudStorageRegistryTimer.isActive()) {
         _updateCloudStorageRegistryTimer.start(500);
+    }
 }
 
 void NavigationPaneHelper::updateCloudStorageRegistry()
@@ -66,11 +69,12 @@ void NavigationPaneHelper::updateCloudStorageRegistry()
     // that matches ours when we saved.
     QVector<QUuid> entriesToRemove;
 #ifdef Q_OS_WIN
-    QString nameSpaceKey = QStringLiteral(R"(Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace)");
+    const auto nameSpaceKey = QStringLiteral(R"(Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace)");
     if (Utility::registryKeyExists(HKEY_CURRENT_USER, nameSpaceKey)) {
         Utility::registryWalkSubKeys(HKEY_CURRENT_USER, nameSpaceKey,
             [&entriesToRemove](HKEY key, const QString &subKey) {
-                QVariant appName = Utility::registryGetKeyValue(key, subKey, QStringLiteral("ApplicationName"));
+                const auto appName = Utility::registryGetKeyValue(key, subKey, QStringLiteral("ApplicationName"));
+                qCDebug(lcNavPane) << "Searching for user with subKey:" << subKey;
                 if (appName.toString() == QLatin1String(APPLICATION_NAME)) {
                     QUuid clsid{ subKey };
                     Q_ASSERT(!clsid.isNull());
@@ -85,23 +89,23 @@ void NavigationPaneHelper::updateCloudStorageRegistry()
         // Then re-save every folder that has a valid navigationPaneClsid to the registry.
         // We currently don't distinguish between new and existing CLSIDs, if it's there we just
         // save over it. We at least need to update the tile in case we are suddently using multiple accounts.
-        foreach (Folder *folder, _folderMan->map()) {
+        for (const auto &folder : qAsConst(_folderMan->map())) {
             if (!folder->navigationPaneClsid().isNull()) {
                 // If it already exists, unmark it for removal, this is a valid sync root.
                 entriesToRemove.removeOne(folder->navigationPaneClsid());
 
-                QString clsidStr = folder->navigationPaneClsid().toString();
-                QString clsidPath = QString() % R"(Software\Classes\CLSID\)" % clsidStr;
-                QString clsidPathWow64 = QString() % R"(Software\Classes\Wow6432Node\CLSID\)" % clsidStr;
-                QString namespacePath = QString() % R"(Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace\)" % clsidStr;
+                const auto clsidStr = folder->navigationPaneClsid().toString();
+                const QString clsidPath = QString() % R"(Software\Classes\CLSID\)" % clsidStr;
+                const QString clsidPathWow64 = QString() % R"(Software\Classes\Wow6432Node\CLSID\)" % clsidStr;
+                const QString namespacePath = QString() % R"(Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace\)" % clsidStr;
 
-                QString title = folder->shortGuiRemotePathOrAppName();
+                auto title = folder->shortGuiRemotePathOrAppName();
                 // Write the account name in the sidebar only when using more than one account.
                 if (AccountManager::instance()->accounts().size() > 1) {
                     title = title % " - " % folder->accountState()->account()->prettyName();
                 }
-                QString iconPath = QDir::toNativeSeparators(qApp->applicationFilePath());
-                QString targetFolderPath = QDir::toNativeSeparators(folder->cleanPath());
+                const auto iconPath = QDir::toNativeSeparators(qApp->applicationFilePath());
+                const auto targetFolderPath = QDir::toNativeSeparators(folder->cleanPath());
 
                 qCInfo(lcNavPane) << "Explorer Cloud storage provider: saving path" << targetFolderPath << "to CLSID" << clsidStr;
 #ifdef Q_OS_WIN
@@ -157,11 +161,11 @@ void NavigationPaneHelper::updateCloudStorageRegistry()
     }
 
     // Then remove anything that isn't in our folder list anymore.
-    foreach (auto &clsid, entriesToRemove) {
-        QString clsidStr = clsid.toString();
-        QString clsidPath = QString() % R"(Software\Classes\CLSID\)" % clsidStr;
-        QString clsidPathWow64 = QString() % R"(Software\Classes\Wow6432Node\CLSID\)" % clsidStr;
-        QString namespacePath = QString() % R"(Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace\)" % clsidStr;
+    for (const auto &clsid : qAsConst(entriesToRemove)) {
+        const auto clsidStr = clsid.toString();
+        const QString clsidPath = QString() % R"(Software\Classes\CLSID\)" % clsidStr;
+        const QString clsidPathWow64 = QString() % R"(Software\Classes\Wow6432Node\CLSID\)" % clsidStr;
+        const QString namespacePath = QString() % R"(Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace\)" % clsidStr;
 
         qCInfo(lcNavPane) << "Explorer Cloud storage provider: now unused, removing own CLSID" << clsidStr;
 #ifdef Q_OS_WIN