Skip to content

Commit

Permalink
[M110] [FilesTrash] Update delete strings to say permanent deletion
Browse files Browse the repository at this point in the history
When a user presses "Delete" in Files app, the file will be permanent
deleted. The current dialog doesn't mention the permanent part which is
causing user confusion. Further the initial UI strings doc had this
update and was missed as part of the initial launch.

Demo: http://screencast/cast/NTkwNDg3NzU1MDA0MzEzNnxmMTk1ZmZjMC02OA

(cherry picked from commit 4a2b0b6)

Bug: b:265353795
Test: browser_tests --gtest_filter=*trash*:*tooltip*
Change-Id: I9f75263a0e1a4f67fd228b24143bd47974e05302
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4159738
Commit-Queue: Ben Reich <benreich@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1092908}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4186776
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#699}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
ben-reich authored and Chromium LUCI CQ committed Jan 27, 2023
1 parent 1e821c0 commit f207891
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 78 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/ash/file_manager/file_manager_browsertest.cc
Expand Up @@ -1218,7 +1218,8 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
TestCase("driveLinkOpenFileThroughTransitiveLink"),
TestCase("driveWelcomeBanner"),
TestCase("driveOfflineInfoBanner").EnableDriveDssPin(),
TestCase("driveOfflineInfoBannerWithoutFlag")
TestCase("driveOfflineInfoBannerWithoutFlag"),
TestCase("driveDeleteDialogDoesntMentionPermanentDelete")
// TODO(b/258987225): Enable
// TestCase("driveInlineSyncStatusSingleFile").EnableInlineStatusSync(),
// TestCase("driveInlineSyncStatusParentFolder").EnableInlineStatusSync()
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_string_util.cc
Expand Up @@ -349,6 +349,16 @@ void AddStringsGeneric(base::Value::Dict* dict) {
SET_STRING("TRASH_NUDGE_LABEL", IDS_FILE_BROWSER_TRASH_NUDGE_LABEL);
SET_STRING("CONFIRM_DELETE_ONE", IDS_FILE_BROWSER_CONFIRM_DELETE_ONE);
SET_STRING("CONFIRM_DELETE_SOME", IDS_FILE_BROWSER_CONFIRM_DELETE_SOME);
SET_STRING("CONFIRM_PERMANENTLY_DELETE_ONE_TITLE",
IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_ONE_TITLE);
SET_STRING("CONFIRM_PERMANENTLY_DELETE_SOME_TITLE",
IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_SOME_TITLE);
SET_STRING("CONFIRM_PERMANENTLY_DELETE_ONE_DESC",
IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_ONE_DESC);
SET_STRING("CONFIRM_PERMANENTLY_DELETE_SOME_DESC",
IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_SOME_DESC);
SET_STRING("PERMANENTLY_DELETE_FOREVER",
IDS_FILE_BROWSER_PERMANENTLY_DELETE_FOREVER);
SET_STRING("CANT_RESTORE_SINGLE_ITEM",
IDS_FILE_BROWSER_CANT_RESTORE_TRASHED_SINGLE_ITEM);
SET_STRING("CANT_RESTORE_MULTIPLE_ITEMS_SAME_PARENTS",
Expand Down
15 changes: 15 additions & 0 deletions ui/chromeos/file_manager_strings.grdp
Expand Up @@ -879,6 +879,21 @@
Move items you don't need to trash
</message>

<message name="IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_ONE_TITLE" desc="Asks the user if they are sure they want to permanently delete a single file.">
Permanently delete this file?
</message>
<message name="IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_SOME_TITLE" desc="Asks the user if they are sure they want to permanently delete multiple files/directories.">
Permanently delete these files?
</message>
<message name="IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_ONE_DESC" desc="Asks the user if they are sure they want to permanently delete a single file.">
"<ph name="FILE_NAME">$1<ex>file.txt</ex></ph>" will be deleted and you won't be able to restore it.
</message>
<message name="IDS_FILE_BROWSER_CONFIRM_PERMANENTLY_DELETE_SOME_DESC" desc="Asks the user if they are sure they want to permanently delete multiple files/directories.">
<ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items will be deleted and you won't be able to restore them.
</message>
<message name="IDS_FILE_BROWSER_PERMANENTLY_DELETE_FOREVER" desc="Button label the users confirms permanently deleting files forever">
Delete forever
</message>
<message name="IDS_FILE_BROWSER_CONFIRM_DELETE_ONE" desc="Asks the user if they are sure they want to delete a single file.">
Are you sure you want to delete "<ph name="FILE_NAME">$1<ex>file.txt</ex></ph>"?
</message>
Expand Down
@@ -0,0 +1 @@
f3f936cc1dc85814262cba809ff8213f2bbb9d6e
@@ -0,0 +1 @@
f3f936cc1dc85814262cba809ff8213f2bbb9d6e
@@ -0,0 +1 @@
4b7ab39530ef7657c5bdfba063b0404048ff7030
@@ -0,0 +1 @@
4b7ab39530ef7657c5bdfba063b0404048ff7030
@@ -0,0 +1 @@
4b7ab39530ef7657c5bdfba063b0404048ff7030
Expand Up @@ -1211,10 +1211,6 @@ CommandHandler.deleteCommand_ = new (class extends FilesCommand {
return;
}

const message = entries.length === 1 ?
strf('CONFIRM_DELETE_ONE', entries[0].name) :
strf('CONFIRM_DELETE_SOME', entries.length);

if (!dialog) {
dialog = fileManager.ui.deleteConfirmDialog;
} else if (dialog.showModalElement) {
Expand All @@ -1237,7 +1233,28 @@ CommandHandler.deleteCommand_ = new (class extends FilesCommand {
dialogDoneCallback();
};

dialog.show(message, deleteAction, cancelAction, null);
// When a user deletes a file from Drive, it gets moved to the Google Drive
// trash. This means the file is not technically permanently removed.
// Fallback to the delete text that doesn't specify permanent delete in this
// case.
if (fileManager.directoryModel.isOnDrive()) {
const deleteMessage = entries.length === 1 ?
strf('CONFIRM_DELETE_ONE', entries[0].name) :
strf('CONFIRM_DELETE_SOME', entries.length);
dialog.setOkLabel(str('DELETE_BUTTON_LABEL'));
dialog.show(deleteMessage, deleteAction, cancelAction, null);
return;
}

const title = entries.length === 1 ?
strf('CONFIRM_PERMANENTLY_DELETE_ONE_TITLE') :
strf('CONFIRM_PERMANENTLY_DELETE_SOME_TITLE');

const message = entries.length === 1 ?
strf('CONFIRM_PERMANENTLY_DELETE_ONE_DESC', entries[0].name) :
strf('CONFIRM_PERMANENTLY_DELETE_SOME_DESC', entries.length);

dialog.showWithTitle(title, message, deleteAction, cancelAction, null);
}

/**
Expand Down
Expand Up @@ -99,7 +99,7 @@ export class FileManagerUI {
* @const
*/
this.deleteConfirmDialog = new FilesConfirmDialog(this.element);
this.deleteConfirmDialog.setOkLabel(str('DELETE_BUTTON_LABEL'));
this.deleteConfirmDialog.setOkLabel(str('PERMANENTLY_DELETE_FOREVER'));
this.deleteConfirmDialog.focusCancelButton = true;

/**
Expand Down
37 changes: 37 additions & 0 deletions ui/file_manager/integration_tests/file_manager/drive_specific.js
Expand Up @@ -1170,3 +1170,40 @@ testcase.driveEnableDocsOfflineDialogDisappearsOnUnmount = async () => {
// Check: the Enable Docs Offline dialog should disappear.
await remoteCall.waitForElementLost(appId, '.cr-dialog-container.shown');
};

/**
* Tests that when deleting a file on Google Drive the dialog has no mention of
* permanent deletion (as the files aren't pemanently deleted but go to Google
* Drive trash instead).
*/
testcase.driveDeleteDialogDoesntMentionPermanentDelete = async () => {
// Open Files app on Drive.
const appId = await setupAndWaitUntilReady(RootPath.DRIVE, []);

// Wait for the "hello.txt" file to appear.
const helloTxtSelector = '#file-list [file-name="photos"]';
await remoteCall.waitAndClickElement(appId, helloTxtSelector);

// Ensure the move-to-trash command is hidden and disabled on Google Drive and
// then click the enabled delete button
await remoteCall.waitForElement(appId, '#move-to-trash[hidden][disabled]');
await remoteCall.waitAndClickElement(
appId, '#delete-button:not([hidden]):not([disabled])');

// Check: the dialog 'Cancel' button should be focused by default.
const dialogDefaultButton =
await remoteCall.waitForElement(appId, '.cr-dialog-cancel:focus');
chrome.test.assertEq('Cancel', dialogDefaultButton.text);

// Check: the dialog has no mention in the text of "permanent".
const dialogText = await remoteCall.waitForElement(appId, '.cr-dialog-text');
chrome.test.assertFalse(dialogText.text.toLowerCase().includes('permanent'));

// The dialog 'Delete' button should be only contain the text "Delete".
const dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete', dialogDeleteButton.text);

// Wait for completion of file deletion.
await remoteCall.waitForElementLost(appId, helloTxtSelector);
};
119 changes: 48 additions & 71 deletions ui/file_manager/integration_tests/file_manager/trash.js
Expand Up @@ -33,22 +33,35 @@ async function clickDeleteButton(appId) {
}

/**
* Delete files in MyFiles and ensure they are moved to /.Trash.
* Then delete items from /.Trash/files and /.Trash/info, then delete /.Trash.
* Clicks the delete button and confirms the deletion.
* @param {string} appId
*/
testcase.trashMoveToTrash = async () => {
const appId = await setupAndWaitUntilReady(
RootPath.DOWNLOADS, BASIC_LOCAL_ENTRY_SET, []);
async function clickDeleteButtonAndConfirmDeletion(appId) {
await clickDeleteButton(appId);

// Select hello.txt.
await remoteCall.waitAndClickElement(
appId, '#file-list [file-name="hello.txt"]');
// Check: the delete confirm dialog should appear.
await remoteCall.waitForElement(appId, '.cr-dialog-container.shown');

// Delete item and wait for it to be removed (no dialog).
await clickTrashButton(appId);
// Check: the dialog 'Cancel' button should be focused by default.
const dialogDefaultButton =
await remoteCall.waitForElement(appId, '.cr-dialog-cancel:focus');
chrome.test.assertEq('Cancel', dialogDefaultButton.text);

// Click the delete confirm dialog 'Delete' button.
const dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete forever', dialogDeleteButton.text);

// Wait for completion of file deletion.
await remoteCall.waitForElementLost(
appId, '#file-list [file-name="hello.txt"]');
}

/**
* Shows hidden files to facilitate tests again the .Trash directory.
* @param {string} appId
*/
async function showHiddenFiles(appId) {
// Open the gear menu by clicking the gear button.
chrome.test.assertTrue(await remoteCall.callRemoteTestUtil(
'fakeMouseClick', appId, ['#gear-button']));
Expand All @@ -63,31 +76,37 @@ testcase.trashMoveToTrash = async () => {
// Click the menu item.
await remoteCall.callRemoteTestUtil(
'fakeMouseClick', appId, ['#gear-menu-toggle-hidden-files']);
}

// Navigate to /My files/Downloads/.Trash/files.
await navigateWithDirectoryTree(appId, '/My files/Downloads/.Trash/files');
/**
* Delete files in MyFiles and ensure they are moved to /.Trash.
* Then delete items from /.Trash/files and /.Trash/info, then delete /.Trash.
*/
testcase.trashMoveToTrash = async () => {
const appId = await setupAndWaitUntilReady(
RootPath.DOWNLOADS, BASIC_LOCAL_ENTRY_SET, []);

// Select hello.txt.
await remoteCall.waitAndClickElement(
appId, '#file-list [file-name="hello.txt"]');

// Delete selected item.
await clickDeleteButton(appId);
// Delete item and wait for it to be removed (no dialog).
await clickTrashButton(appId);
await remoteCall.waitForElementLost(
appId, '#file-list [file-name="hello.txt"]');

// Check: the delete confirm dialog should appear.
await remoteCall.waitForElement(appId, '.cr-dialog-container.shown');
// Enable hidden files to be shown.
await showHiddenFiles(appId);

// Check: the dialog 'Cancel' button should be focused by default.
const dialogDefaultButton =
await remoteCall.waitForElement(appId, '.cr-dialog-cancel:focus');
chrome.test.assertEq('Cancel', dialogDefaultButton.text);
// Navigate to /My files/Downloads/.Trash/files.
await navigateWithDirectoryTree(appId, '/My files/Downloads/.Trash/files');

// Click the delete confirm dialog 'Delete' button.
let dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete', dialogDeleteButton.text);
// Select hello.txt.
await remoteCall.waitAndClickElement(
appId, '#file-list [file-name="hello.txt"]');

// Wait for completion of file deletion.
// Delete selected item.
await clickDeleteButtonAndConfirmDeletion(appId);
await remoteCall.waitForElementLost(
appId, '#file-list [file-name="hello.txt"]');

Expand All @@ -99,15 +118,7 @@ testcase.trashMoveToTrash = async () => {
appId, '#file-list [file-name="hello.txt.trashinfo"]');

// Delete selected item.
await clickDeleteButton(appId);

// Check: the delete confirm dialog should appear.
await remoteCall.waitForElement(appId, '.cr-dialog-container.shown');

// Click the delete confirm dialog 'Delete' button.
dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete', dialogDeleteButton.text);
await clickDeleteButtonAndConfirmDeletion(appId);

// Wait for completion of file deletion.
await remoteCall.waitForElementLost(
Expand All @@ -121,15 +132,7 @@ testcase.trashMoveToTrash = async () => {
appId, '#file-list [file-name=".Trash"]');

// Delete selected item.
await clickDeleteButton(appId);

// Check: the delete confirm dialog should appear.
await remoteCall.waitForElement(appId, '.cr-dialog-container.shown');

// Click the delete confirm dialog 'Delete' button.
dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete', dialogDeleteButton.text);
await clickDeleteButtonAndConfirmDeletion(appId);

// Wait for completion of file deletion.
await remoteCall.waitForElementLost(appId, '#file-list [file-name=".Trash"]');
Expand Down Expand Up @@ -205,20 +208,7 @@ testcase.trashDeleteFromTrashOriginallyFromMyFiles = async () => {
appId, '#file-list [file-name="hello.txt"]');

// Delete selected item.
await clickDeleteButton(appId);

// Check: the delete confirm dialog should appear.
await remoteCall.waitForElement(appId, '.cr-dialog-container.shown');

// Check: the dialog 'Cancel' button should be focused by default.
const dialogDefaultButton =
await remoteCall.waitForElement(appId, '.cr-dialog-cancel:focus');
chrome.test.assertEq('Cancel', dialogDefaultButton.text);

// Click the delete confirm dialog 'Delete' button.
const dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete', dialogDeleteButton.text);
await clickDeleteButtonAndConfirmDeletion(appId);

// Wait for completion of file deletion.
await remoteCall.waitForElementLost(
Expand Down Expand Up @@ -426,20 +416,7 @@ testcase.trashDeleteFromTrash = async () => {
appId, '#file-list [file-name="hello.txt"]');

// Delete selected item.
await clickDeleteButton(appId);

// Check: the delete confirm dialog should appear.
await remoteCall.waitForElement(appId, '.cr-dialog-container.shown');

// Check: the dialog 'Cancel' button should be focused by default.
const dialogDefaultButton =
await remoteCall.waitForElement(appId, '.cr-dialog-cancel:focus');
chrome.test.assertEq('Cancel', dialogDefaultButton.text);

// Click the delete confirm dialog 'Delete' button.
const dialogDeleteButton =
await remoteCall.waitAndClickElement(appId, '.cr-dialog-ok');
chrome.test.assertEq('Delete', dialogDeleteButton.text);
await clickDeleteButtonAndConfirmDeletion(appId);

// Wait for completion of file deletion.
await remoteCall.waitForElementLost(
Expand Down

0 comments on commit f207891

Please sign in to comment.