From e1824c00a9cc77f74265bb520c40e360f69e7cb0 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 13 Aug 2019 12:31:53 -0700 Subject: [PATCH] feat: macOS removal fallback when moveItemToTrash fails (#19700) * feat: macOS removal fallback when moveItemToTrash fails * platform_util shouldn't know about mate::Arguments * pull full_path from args as well --- docs/api/shell.md | 5 +++-- shell/common/api/atom_api_shell.cc | 12 ++++++++++- shell/common/platform_util.h | 2 +- shell/common/platform_util_linux.cc | 2 +- shell/common/platform_util_mac.mm | 33 +++++++++++++++++++++++------ shell/common/platform_util_win.cc | 2 +- 6 files changed, 43 insertions(+), 13 deletions(-) diff --git a/docs/api/shell.md b/docs/api/shell.md index 6f6b9306fe..e5bf793380 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -43,11 +43,12 @@ Returns `Promise` Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). -### `shell.moveItemToTrash(fullPath)` +### `shell.moveItemToTrash(fullPath[, deleteOnFail])` * `fullPath` String +* `deleteOnFail` Boolean (optional) - Whether or not to unilaterally remove the item if the Trash is disabled or unsupported on the volume. _macOS_ -Returns `Boolean` - Whether the item was successfully moved to the trash. +Returns `Boolean` - Whether the item was successfully moved to the trash or otherwise deleted. Move the given file to trash and returns a boolean status for the operation. diff --git a/shell/common/api/atom_api_shell.cc b/shell/common/api/atom_api_shell.cc index 6a226a888d..14d9b98d5b 100644 --- a/shell/common/api/atom_api_shell.cc +++ b/shell/common/api/atom_api_shell.cc @@ -71,6 +71,16 @@ v8::Local OpenExternal(const GURL& url, mate::Arguments* args) { return handle; } +bool MoveItemToTrash(mate::Arguments* args) { + base::FilePath full_path; + args->GetNext(&full_path); + + bool delete_on_fail = false; + args->GetNext(&delete_on_fail); + + return platform_util::MoveItemToTrash(full_path, delete_on_fail); +} + #if defined(OS_WIN) bool WriteShortcutLink(const base::FilePath& shortcut_path, mate::Arguments* args) { @@ -134,7 +144,7 @@ void Initialize(v8::Local exports, dict.SetMethod("showItemInFolder", &platform_util::ShowItemInFolder); dict.SetMethod("openItem", &platform_util::OpenItem); dict.SetMethod("openExternal", &OpenExternal); - dict.SetMethod("moveItemToTrash", &platform_util::MoveItemToTrash); + dict.SetMethod("moveItemToTrash", &MoveItemToTrash); dict.SetMethod("beep", &platform_util::Beep); #if defined(OS_WIN) dict.SetMethod("writeShortcutLink", &WriteShortcutLink); diff --git a/shell/common/platform_util.h b/shell/common/platform_util.h index 2d196e3271..9224bfd51a 100644 --- a/shell/common/platform_util.h +++ b/shell/common/platform_util.h @@ -41,7 +41,7 @@ void OpenExternal(const GURL& url, OpenExternalCallback callback); // Move a file to trash. -bool MoveItemToTrash(const base::FilePath& full_path); +bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail); void Beep(); diff --git a/shell/common/platform_util_linux.cc b/shell/common/platform_util_linux.cc index aaba6c8651..900f90cae1 100644 --- a/shell/common/platform_util_linux.cc +++ b/shell/common/platform_util_linux.cc @@ -82,7 +82,7 @@ void OpenExternal(const GURL& url, std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open"); } -bool MoveItemToTrash(const base::FilePath& full_path) { +bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) { std::unique_ptr env(base::Environment::Create()); // find the trash method diff --git a/shell/common/platform_util_mac.mm b/shell/common/platform_util_mac.mm index cd11c050af..f141c40631 100644 --- a/shell/common/platform_util_mac.mm +++ b/shell/common/platform_util_mac.mm @@ -115,16 +115,35 @@ void OpenExternal(const GURL& url, }); } -bool MoveItemToTrash(const base::FilePath& full_path) { +bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) { NSString* path_string = base::SysUTF8ToNSString(full_path.value()); - BOOL status = [[NSFileManager defaultManager] - trashItemAtURL:[NSURL fileURLWithPath:path_string] - resultingItemURL:nil - error:nil]; - if (!path_string || !status) + if (!path_string) { + LOG(WARNING) << "Invalid file path " << full_path.value(); + return false; + } + + NSURL* url = [NSURL fileURLWithPath:path_string]; + NSError* err = nil; + BOOL did_trash = [[NSFileManager defaultManager] trashItemAtURL:url + resultingItemURL:nil + error:&err]; + + if (delete_on_fail) { + // Some volumes may not support a Trash folder or it may be disabled + // so these methods will report failure by returning NO or nil and + // an NSError with NSFeatureUnsupportedError. + // Handle this by deleting the item as a fallback. + if (!did_trash && [err code] == NSFeatureUnsupportedError) { + did_trash = [[NSFileManager defaultManager] removeItemAtURL:url + error:nil]; + } + } + + if (!did_trash) LOG(WARNING) << "NSWorkspace failed to move file " << full_path.value() << " to trash"; - return status; + + return did_trash; } void Beep() { diff --git a/shell/common/platform_util_win.cc b/shell/common/platform_util_win.cc index 3021c2e1fb..87e389ddd5 100644 --- a/shell/common/platform_util_win.cc +++ b/shell/common/platform_util_win.cc @@ -335,7 +335,7 @@ void OpenExternal(const GURL& url, std::move(callback)); } -bool MoveItemToTrash(const base::FilePath& path) { +bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) { base::win::ScopedCOMInitializer com_initializer; if (!com_initializer.Succeeded()) return false;