From 1a6fa2632388ffcc57ce723501a588c90b940f93 Mon Sep 17 00:00:00 2001 From: "Hong Jen Yee (PCMan)" Date: Sat, 9 Jun 2018 21:04:19 +0800 Subject: [PATCH] Fix failure to open smb:// caused by incorrect file info handling. --- src/core/basicfilelauncher.cpp | 40 ++++++++++++++++++++++++------- src/core/basicfilelauncher.h | 2 +- src/core/fileinfo.cpp | 9 ++++--- src/core/fileinfojob.cpp | 44 +++++++++++++++++++++------------- src/core/fileinfojob.h | 5 ++++ src/core/gioptrs.h | 4 ++++ src/filelauncher.cpp | 2 +- src/filelauncher.h | 2 +- 8 files changed, 77 insertions(+), 31 deletions(-) diff --git a/src/core/basicfilelauncher.cpp b/src/core/basicfilelauncher.cpp index 2c7f00e..0dc8208 100644 --- a/src/core/basicfilelauncher.cpp +++ b/src/core/basicfilelauncher.cpp @@ -29,11 +29,13 @@ bool BasicFileLauncher::launchFiles(const FileInfoList& fileInfos, GAppLaunchCon FilePathList pathsToLaunch; // classify files according to different mimetypes for(auto& fileInfo : fileInfos) { - // qDebug("path: %s, target: %s", fileInfo->path().toString().get(), fileInfo->target().c_str()); - if(fileInfo->isDir()) { - folderInfos.emplace_back(fileInfo); - } - else if(fileInfo->isMountable()) { + /* + qDebug("path: %s, type: %s, target: %s, isDir: %i, isDesktopEntry: %i", + fileInfo->path().toString().get(), fileInfo->mimeType()->name(), fileInfo->target().c_str(), + fileInfo->isDir(), fileInfo->isDesktopEntry()); + */ + + if(fileInfo->isMountable()) { if(fileInfo->target().empty()) { // the mountable is not yet mounted so we have no target URI. GErrorPtr err{G_IO_ERROR, G_IO_ERROR_NOT_MOUNTED, @@ -67,6 +69,9 @@ bool BasicFileLauncher::launchFiles(const FileInfoList& fileInfos, GAppLaunchCon pathsToLaunch.emplace_back(path); } } + else if(fileInfo->isDir()) { + folderInfos.emplace_back(fileInfo); + } else { auto& mimeType = fileInfo->mimeType(); mimeTypeToFiles[mimeType->name()].emplace_back(fileInfo); @@ -103,16 +108,27 @@ bool BasicFileLauncher::launchFiles(const FileInfoList& fileInfos, GAppLaunchCon bool BasicFileLauncher::launchPaths(FilePathList paths, GAppLaunchContext* ctx) { // FIXME: blocking with an event loop is not a good design :-( QEventLoop eventLoop; - auto job = new FileInfoJob{paths}; job->setAutoDelete(false); // do not automatically delete the job since we want its results later. GObjectPtr ctxPtr{ctx}; + + // error handling (for example: handle path not mounted error) + QObject::connect(job, &FileInfoJob::error, + &eventLoop, [this, job, ctx](const GErrorPtr & err, Job::ErrorSeverity /* severity */ , Job::ErrorAction &act) { + auto path = job->currentPath(); + if(showError(ctx, err, path, nullptr)) { + // the user handled the error and ask for retry + act = Job::ErrorAction::RETRY; + } + }, Qt::BlockingQueuedConnection); // BlockingQueuedConnection is required here to pause the job and wait for user response + QObject::connect(job, &FileInfoJob::finished, [&eventLoop]() { // exit the event loop when the job is done eventLoop.exit(); }); + // run the job in another thread to not block the UI job->runAsync(); @@ -145,7 +161,7 @@ BasicFileLauncher::ExecAction BasicFileLauncher::askExecFile(const FileInfoPtr & return ExecAction::DIRECT_EXEC; } -bool BasicFileLauncher::showError(GAppLaunchContext* /* ctx */, GErrorPtr& /* err */, const FilePath& /* path */, const FileInfoPtr& /* info */) { +bool BasicFileLauncher::showError(GAppLaunchContext* /* ctx */, const GErrorPtr & /* err */, const FilePath& /* path */, const FileInfoPtr& /* info */) { return false; } @@ -249,13 +265,21 @@ bool BasicFileLauncher::launchDesktopEntry(const char *desktopEntryName, const F FilePath BasicFileLauncher::handleShortcut(const FileInfoPtr& fileInfo, GAppLaunchContext* ctx) { auto target = fileInfo->target(); + + // if we know the target is a dir, we are not going to open it using other apps + // for example: `network:///smb-root' is a shortcut targeting `smb:///' and it's also a dir + if(fileInfo->isDir()) { + return FilePath::fromPathStr(target.c_str()); + } + auto scheme = CStrPtr{g_uri_parse_scheme(target.c_str())}; if(scheme) { // collect the uri schemes we support if(strcmp(scheme.get(), "file") == 0 || strcmp(scheme.get(), "trash") == 0 || strcmp(scheme.get(), "network") == 0 - || strcmp(scheme.get(), "computer") == 0) { + || strcmp(scheme.get(), "computer") == 0 + || strcmp(scheme.get(), "menu") == 0) { return FilePath::fromUri(target.c_str()); } else { diff --git a/src/core/basicfilelauncher.h b/src/core/basicfilelauncher.h index a28aa75..3b1545d 100644 --- a/src/core/basicfilelauncher.h +++ b/src/core/basicfilelauncher.h @@ -53,7 +53,7 @@ class LIBFM_QT_API BasicFileLauncher { virtual bool openFolder(GAppLaunchContext* ctx, const FileInfoList& folderInfos, GErrorPtr& err); - virtual bool showError(GAppLaunchContext* ctx, GErrorPtr& err, const FilePath& path = FilePath{}, const FileInfoPtr& info = FileInfoPtr{}); + virtual bool showError(GAppLaunchContext* ctx, const GErrorPtr& err, const FilePath& path = FilePath{}, const FileInfoPtr& info = FileInfoPtr{}); virtual ExecAction askExecFile(const FileInfoPtr& file); diff --git a/src/core/fileinfo.cpp b/src/core/fileinfo.cpp index 8e86f8d..b19a751 100644 --- a/src/core/fileinfo.cpp +++ b/src/core/fileinfo.cpp @@ -36,10 +36,9 @@ void FileInfo::setFromGFileInfo(const GObjectPtr& inf, const FilePath size_ = g_file_info_get_size(inf.get()); tmp = g_file_info_get_content_type(inf.get()); - if(!tmp) { - tmp = "application/octet-stream"; + if(tmp) { + mimeType_ = MimeType::fromName(tmp); } - mimeType_ = MimeType::fromName(tmp); mode_ = g_file_info_get_attribute_uint32(inf.get(), G_FILE_ATTRIBUTE_UNIX_MODE); @@ -196,6 +195,10 @@ void FileInfo::setFromGFileInfo(const GObjectPtr& inf, const FilePath } } + if(!mimeType_) { + mimeType_ = MimeType::fromName("application/octet-stream"); + } + /* if there is a custom folder icon, use it */ if(isNative() && type == G_FILE_TYPE_DIRECTORY) { auto local_path = path().localPath(); diff --git a/src/core/fileinfojob.cpp b/src/core/fileinfojob.cpp index 3c222af..7bf8bb3 100644 --- a/src/core/fileinfojob.cpp +++ b/src/core/fileinfojob.cpp @@ -13,31 +13,41 @@ FileInfoJob::FileInfoJob(FilePathList paths, FilePathList deletionPaths, FilePat void FileInfoJob::exec() { for(const auto& path: paths_) { - if(!isCancelled()) { + if(isCancelled()) { + break; + } + currentPath_ = path; + + bool retry; + do { + retry = false; GErrorPtr err; GFileInfoPtr inf{ g_file_query_info(path.gfile().get(), defaultGFileInfoQueryAttribs, G_FILE_QUERY_INFO_NONE, cancellable().get(), &err), false }; - if(!inf) { - continue; + if(inf) { + // Reuse the same dirPath object when the path remains the same (optimize for files in the same dir) + auto dirPath = commonDirPath_.isValid() ? commonDirPath_ : path.parent(); + auto fileInfoPtr = std::make_shared(inf, dirPath); + + // FIXME: this is not elegant + if(cutFilesHashSet_ + && cutFilesHashSet_->count(path.hash())) { + fileInfoPtr->bindCutFiles(cutFilesHashSet_); + } + + results_.push_back(fileInfoPtr); + Q_EMIT gotInfo(path, results_.back()); } - - // Reuse the same dirPath object when the path remains the same (optimize for files in the same dir) - auto dirPath = commonDirPath_.isValid() ? commonDirPath_ : path.parent(); - FileInfo fileInfo(inf, dirPath); - - if(cutFilesHashSet_ - && cutFilesHashSet_->count(fileInfo.path().hash())) { - fileInfo.bindCutFiles(cutFilesHashSet_); + else { + auto act = emitError(err); + if(act == Job::ErrorAction::RETRY) { + retry = true; + } } - - auto fileInfoPtr = std::make_shared(fileInfo); - - results_.push_back(fileInfoPtr); - Q_EMIT gotInfo(path, fileInfoPtr); - } + } while(retry && !isCancelled()); } } diff --git a/src/core/fileinfojob.h b/src/core/fileinfojob.h index 53a03c5..d75e88f 100644 --- a/src/core/fileinfojob.h +++ b/src/core/fileinfojob.h @@ -27,6 +27,10 @@ class LIBFM_QT_API FileInfoJob : public Job { return results_; } + const FilePath& currentPath() const { + return currentPath_; + } + Q_SIGNALS: void gotInfo(const FilePath& path, std::shared_ptr& info); @@ -39,6 +43,7 @@ class LIBFM_QT_API FileInfoJob : public Job { FileInfoList results_; FilePath commonDirPath_; const std::shared_ptr cutFilesHashSet_; + FilePath currentPath_; }; } // namespace Fm diff --git a/src/core/gioptrs.h b/src/core/gioptrs.h index 401424b..ae22602 100644 --- a/src/core/gioptrs.h +++ b/src/core/gioptrs.h @@ -112,6 +112,10 @@ class GErrorPtr { return err_; } + const GError* operator->() const { + return err_; + } + bool operator == (const GErrorPtr& other) const { return err_ == other.err_; } diff --git a/src/filelauncher.cpp b/src/filelauncher.cpp index 5f667fc..ff14533 100644 --- a/src/filelauncher.cpp +++ b/src/filelauncher.cpp @@ -76,7 +76,7 @@ bool FileLauncher::openFolder(GAppLaunchContext *ctx, const FileInfoList &folder return BasicFileLauncher::openFolder(ctx, folderInfos, err); } -bool FileLauncher::showError(GAppLaunchContext* /*ctx*/, GErrorPtr &err, const FilePath &path, const FileInfoPtr &info) { +bool FileLauncher::showError(GAppLaunchContext* /*ctx*/, const GErrorPtr &err, const FilePath &path, const FileInfoPtr &info) { /* ask for mount if trying to launch unmounted path */ if(err->domain == G_IO_ERROR) { if(path && err->code == G_IO_ERROR_NOT_MOUNTED) { diff --git a/src/filelauncher.h b/src/filelauncher.h index be5be5a..991a00a 100644 --- a/src/filelauncher.h +++ b/src/filelauncher.h @@ -43,7 +43,7 @@ class LIBFM_QT_API FileLauncher: public BasicFileLauncher { bool openFolder(GAppLaunchContext* ctx, const FileInfoList& folderInfos, GErrorPtr& err) override; - bool showError(GAppLaunchContext* ctx, GErrorPtr& err, const FilePath& path = FilePath{}, const FileInfoPtr& info = FileInfoPtr{}) override; + bool showError(GAppLaunchContext* ctx, const GErrorPtr &err, const FilePath& path = FilePath{}, const FileInfoPtr& info = FileInfoPtr{}) override; ExecAction askExecFile(const FileInfoPtr& file) override;