From 14ddc88603bb9f3359c371cdeb8723e5fa4cea6f Mon Sep 17 00:00:00 2001 From: "James D. Smith" Date: Thu, 11 Feb 2021 21:44:49 -0700 Subject: [PATCH 1/3] Use absolute file paths instead of canonical paths. Fixes unpredictable paths in relative path saved playlists. --- src/core/commandlineoptions.cpp | 2 +- src/library/librarybackend.cpp | 8 ++++---- src/networkremote/incomingdataparser.cpp | 2 +- src/playlistparsers/parserbase.cpp | 5 ----- src/ui/mainwindow.cpp | 4 ++-- src/widgets/fileviewlist.cpp | 2 +- tests/librarybackend_test.cpp | 2 +- 7 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/core/commandlineoptions.cpp b/src/core/commandlineoptions.cpp index f1960c4c78..d82f8bc6f8 100644 --- a/src/core/commandlineoptions.cpp +++ b/src/core/commandlineoptions.cpp @@ -308,7 +308,7 @@ bool CommandlineOptions::Parse() { QString value = QFile::decodeName(argv_[i]); QFileInfo file_info(value); if (file_info.exists()) - urls_ << QUrl::fromLocalFile(file_info.canonicalFilePath()); + urls_ << QUrl::fromLocalFile(file_info.absoluteFilePath()); else urls_ << QUrl::fromUserInput(value); } diff --git a/src/library/librarybackend.cpp b/src/library/librarybackend.cpp index a40dca910a..3ed0b26edc 100644 --- a/src/library/librarybackend.cpp +++ b/src/library/librarybackend.cpp @@ -218,11 +218,11 @@ void LibraryBackend::UpdateTotalSongCount() { } void LibraryBackend::AddDirectory(const QString& path) { - QString canonical_path = QFileInfo(path).canonicalFilePath(); - QString db_path = canonical_path; + QString absolute_path = QFileInfo(path).absoluteFilePath(); + QString db_path = absolute_path; if (Application::kIsPortable && Utilities::UrlOnSameDriveAsClementine( - QUrl::fromLocalFile(canonical_path))) { + QUrl::fromLocalFile(absolute_path))) { db_path = Utilities::GetRelativePathToClementineBin(db_path); qLog(Debug) << "db_path" << db_path; } @@ -239,7 +239,7 @@ void LibraryBackend::AddDirectory(const QString& path) { if (db_->CheckErrors(q)) return; Directory dir; - dir.path = canonical_path; + dir.path = absolute_path; dir.id = q.lastInsertId().toInt(); emit DirectoryDiscovered(dir, SubdirectoryList()); diff --git a/src/networkremote/incomingdataparser.cpp b/src/networkremote/incomingdataparser.cpp index 9d9f59f8b9..1e08e36bd8 100644 --- a/src/networkremote/incomingdataparser.cpp +++ b/src/networkremote/incomingdataparser.cpp @@ -467,7 +467,7 @@ void IncomingDataParser::AppendFilesToPlaylist( QDir dir(fi_folder.absoluteFilePath()); for (const auto& file : req_append.files()) { QFileInfo fi(dir, file.c_str()); - if (fi.exists()) urls << QUrl::fromLocalFile(fi.canonicalFilePath()); + if (fi.exists()) urls << QUrl::fromLocalFile(fi.absoluteFilePath()); } if (!urls.isEmpty()) { MimeData* data = new MimeData; diff --git a/src/playlistparsers/parserbase.cpp b/src/playlistparsers/parserbase.cpp index c8a3942ef0..96cc062761 100644 --- a/src/playlistparsers/parserbase.cpp +++ b/src/playlistparsers/parserbase.cpp @@ -58,11 +58,6 @@ void ParserBase::LoadSong(const QString& filename_or_url, qint64 beginning, filename = dir.absoluteFilePath(filename); } - // Use the canonical path - if (QFile::exists(filename)) { - filename = QFileInfo(filename).canonicalFilePath(); - } - const QUrl url = QUrl::fromLocalFile(filename); // Search in the library diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index b4ab2cef33..f760c6a1c7 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -2181,7 +2181,7 @@ void MainWindow::AddFile() { // Convert to URLs QList urls; for (const QString& path : file_names) { - urls << QUrl::fromLocalFile(QFileInfo(path).canonicalFilePath()); + urls << QUrl::fromLocalFile(QFileInfo(path).absoluteFilePath()); } MimeData* data = new MimeData; @@ -2205,7 +2205,7 @@ void MainWindow::AddFolder() { // Add media MimeData* data = new MimeData; data->setUrls(QList() << QUrl::fromLocalFile( - QFileInfo(directory).canonicalFilePath())); + QFileInfo(directory).absoluteFilePath())); AddToPlaylist(data); } diff --git a/src/widgets/fileviewlist.cpp b/src/widgets/fileviewlist.cpp index 27870e4b81..2b88ccb73d 100644 --- a/src/widgets/fileviewlist.cpp +++ b/src/widgets/fileviewlist.cpp @@ -69,7 +69,7 @@ QList FileViewList::UrlListFromSelection() const { if (index.column() == 0) urls << QUrl::fromLocalFile(static_cast(model()) ->fileInfo(index) - .canonicalFilePath()); + .absoluteFilePath()); } return urls; } diff --git a/tests/librarybackend_test.cpp b/tests/librarybackend_test.cpp index 99f7cdb7d8..1e022c0d1b 100644 --- a/tests/librarybackend_test.cpp +++ b/tests/librarybackend_test.cpp @@ -74,7 +74,7 @@ TEST_F(LibraryBackendTest, AddDirectory) { // Check the signal was emitted correctly ASSERT_EQ(1, spy.count()); Directory dir = spy[0][0].value(); - EXPECT_EQ(QFileInfo("/tmp").canonicalFilePath(), dir.path); + EXPECT_EQ(QFileInfo("/tmp").absoluteFilePath(), dir.path); EXPECT_EQ(1, dir.id); EXPECT_EQ(0, spy[0][1].value().size()); } From 004c4cfe789608bf09c9de90af55e682ab1f587c Mon Sep 17 00:00:00 2001 From: "James D. Smith" Date: Tue, 12 Oct 2021 20:46:45 -0600 Subject: [PATCH 2/3] Playlist Parser: Clean the file path when loading a playlist. --- src/playlistparsers/parserbase.cpp | 4 +++- src/playlistparsers/parserbase.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/playlistparsers/parserbase.cpp b/src/playlistparsers/parserbase.cpp index 96cc062761..190d289ec7 100644 --- a/src/playlistparsers/parserbase.cpp +++ b/src/playlistparsers/parserbase.cpp @@ -53,11 +53,13 @@ void ParserBase::LoadSong(const QString& filename_or_url, qint64 beginning, // was created on/for, using replace() lets playlists work on any platform. filename = filename.replace('\\', '/'); - // Make the path absolute + // Make the path absolute and clean it if (!QDir::isAbsolutePath(filename)) { filename = dir.absoluteFilePath(filename); } + filename = QDir::cleanPath(filename); + const QUrl url = QUrl::fromLocalFile(filename); // Search in the library diff --git a/src/playlistparsers/parserbase.h b/src/playlistparsers/parserbase.h index 5670981455..d38ca3e987 100644 --- a/src/playlistparsers/parserbase.h +++ b/src/playlistparsers/parserbase.h @@ -61,7 +61,7 @@ class ParserBase : public QObject { protected: // Loads a song. If filename_or_url is a URL (with a scheme other than // "file") then it is set on the song and the song marked as a stream. - // If it is a filename or a file:// URL then it is made absolute and canonical + // If it is a filename or a file:// URL then it is made absolute and cleaned // and set as a file:// url on the song. Also sets the song's metadata by // searching in the Library, or loading from the file as a fallback. // This function should always be used when loading a playlist. From 69315833af4c21e22b038259278204ba4a324ca9 Mon Sep 17 00:00:00 2001 From: "James D. Smith" Date: Fri, 25 Mar 2022 10:10:57 -0600 Subject: [PATCH 3/3] Utilities: Resolve the path canonically for Windows same-drive check. --- src/core/utilities.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index 5541b3d571..d9c47ef4ea 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -725,8 +725,11 @@ bool UrlOnSameDriveAsClementine(const QUrl& url) { if (url.scheme() != "file") return false; #ifdef Q_OS_WIN + QUrl canUrl = + QUrl::fromLocalFile(QFileInfo(url.toLocalFile()).canonicalFilePath()); QUrl appUrl = QUrl::fromLocalFile(QCoreApplication::applicationDirPath()); - if (url.toLocalFile().left(1) == appUrl.toLocalFile().left(1)) + + if (canUrl.toLocalFile().left(1) == appUrl.toLocalFile().left(1)) return true; else return false;