Skip to content

Commit

Permalink
[FilesTrash] Enable dropping onto the Trash root
Browse files Browse the repository at this point in the history
The Trash root in Files app is a fake volume thus existing functionality
that enables dropping files typically bails early or fails with other
checks (e.g. verifying the fileSystem is the same as the entries being
dropped).

This updates various logic to introduce a new check that verifies the
supplied selection is on a volume that is trashable AND resides in a
location on that volume that is enabled for trash. In the event all this
is true, delegate the operation to the TrashIOTask instead of move or
copy like existing drag drop functionality as trash must always be an
intra file system operation.

Bug: b:241517469
Test: browser_tests --gtest_filter=*trashDragDrop*
Change-Id: I0f14923527c5c403548b8bce9cf8cce35bb40f65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3812712
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Ben Reich <benreich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032896}
  • Loading branch information
ben-reich authored and Chromium LUCI CQ committed Aug 9, 2022
1 parent 85e3388 commit f86f07e
Show file tree
Hide file tree
Showing 7 changed files with 326 additions and 7 deletions.
10 changes: 10 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_browsertest.cc
Expand Up @@ -2149,6 +2149,16 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
TestCase("trashNoTasksInTrashRoot").EnableTrash().FilesSwa(),
TestCase("trashDoubleClickOnFileInTrashRootShowsDialog").EnableTrash(),
TestCase("trashDoubleClickOnFileInTrashRootShowsDialog")
.EnableTrash()
.FilesSwa(),
TestCase("trashDragDropRootAcceptsEntries").EnableTrash().FilesSwa(),
TestCase("trashDragDropFromDisallowedRootsFails")
.EnableTrash()
.FilesSwa(),
TestCase("trashDragDropNonModifiableEntriesCantBeTrashed")
.EnableTrash()
.FilesSwa(),
TestCase("trashDragDropRootPerformsTrashAction")
.EnableTrash()
.FilesSwa()));

Expand Down
Expand Up @@ -418,10 +418,15 @@ export class VolumeManagerImpl extends EventTarget {
const volumeInfo = this.getVolumeInfo(entry);

if (util.isFakeEntry(entry)) {
const isReadOnly =
entry.rootType === VolumeManagerCommon.RootType.RECENT ?
!util.isRecentsFilterV2Enabled() :
true;
// Aggregated views like RECENTS and TRASH exist as fake entries but may
// actually defer their logic to some underlying implementation or
// delegate to the location filesystem.
let isReadOnly = true;
if ((entry.rootType === VolumeManagerCommon.RootType.RECENT &&
util.isRecentsFilterV2Enabled()) ||
entry.rootType === VolumeManagerCommon.RootType.TRASH) {
isReadOnly = false;
}
return new EntryLocationImpl(
volumeInfo, assert(entry.rootType),
true /* The entry points a root directory. */, isReadOnly);
Expand Down
19 changes: 19 additions & 0 deletions ui/file_manager/file_manager/common/js/trash.js
Expand Up @@ -87,6 +87,25 @@ TrashConfig.CONFIG = [
}),
];

/**
* Returns a list of strings that represent volumes that are enabled for Trash.
* Used to validate drag drop data without resolving the URLs to Entry's.
* @param {!VolumeManager} volumeManager
* @returns {!Array<!string>}
*/
export function getEnabledTrashVolumeURLs(volumeManager) {
const urls = [];
for (let i = 0; i < volumeManager.volumeInfoList.length; i++) {
const volumeInfo = volumeManager.volumeInfoList.item(i);
for (const config of TrashConfig.CONFIG) {
if (volumeInfo.volumeType === config.volumeType) {
urls.push(volumeInfo.fileSystem.root.toURL());
}
}
}
return urls;
}

/**
* Wrapper for /.Trash/files and /.Trash/info directories.
*/
Expand Down
1 change: 1 addition & 0 deletions ui/file_manager/file_manager/foreground/js/BUILD.gn
Expand Up @@ -805,6 +805,7 @@ js_library("file_transfer_controller") {
"//ui/file_manager/file_manager/common/js:api",
"//ui/file_manager/file_manager/common/js:file_type",
"//ui/file_manager/file_manager/common/js:progress_center_common",
"//ui/file_manager/file_manager/common/js:trash",
"//ui/file_manager/file_manager/common/js:util",
"//ui/file_manager/file_manager/common/js:volume_manager_types",
"//ui/file_manager/file_manager/externs:entry_location",
Expand Down
Expand Up @@ -1718,6 +1718,7 @@ export class DirectoryModel extends EventTarget {
chrome.fileManagerPrivate.IOTaskType.EMPTY_TRASH,
chrome.fileManagerPrivate.IOTaskType.MOVE,
chrome.fileManagerPrivate.IOTaskType.RESTORE,
chrome.fileManagerPrivate.IOTaskType.TRASH,
]);
/** @type {!Set<?VolumeManagerCommon.RootType>} */
const rootTypesRequireRefresh = new Set([
Expand Down
106 changes: 104 additions & 2 deletions ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
Expand Up @@ -6,9 +6,10 @@ import {assert, assertNotReached} from 'chrome://resources/js/assert.m.js';
import {Command} from 'chrome://resources/js/cr/ui/command.js';
import {List} from 'chrome://resources/js/cr/ui/list.m.js';

import {getDirectory, getDisallowedTransfers, startIOTask} from '../../common/js/api.js';
import {getDisallowedTransfers, startIOTask} from '../../common/js/api.js';
import {FileType} from '../../common/js/file_type.js';
import {ProgressCenterItem, ProgressItemState, ProgressItemType} from '../../common/js/progress_center_common.js';
import {getEnabledTrashVolumeURLs} from '../../common/js/trash.js';
import {str, strf, util} from '../../common/js/util.js';
import {VolumeManagerCommon} from '../../common/js/volume_manager_types.js';
import {FileOperationManager} from '../../externs/background/file_operation_manager.js';
Expand Down Expand Up @@ -948,12 +949,37 @@ export class FileTransferController {
* @param {!Event} event A dragleave event of DOM.
* @private
*/
onDrop_(onlyIntoDirectories, event) {
async onDrop_(onlyIntoDirectories, event) {
if (onlyIntoDirectories && !this.dropTarget_) {
return;
}
const destinationEntry =
this.destinationEntry_ || this.directoryModel_.getCurrentDirEntry();
if (destinationEntry.rootType === VolumeManagerCommon.RootType.TRASH &&
this.canTrashSelection_(destinationEntry, event.dataTransfer)) {
event.preventDefault();
const sourceURLs =
(event.dataTransfer.getData('fs/sources') || '').split('\n');
const {entries, failureUrls} =
await FileTransferController.URLsToEntriesWithAccess(sourceURLs);

// The list of entries should not be special entries (e.g. Camera, Linux
// files) and should not already exist in Trash (i.e. you can't trash
// something that's already trashed).
const isModifiableAndNotInTrashRoot = entry => {
return !util.isNonModifiable(this.volumeManager_, entry) &&
!util.isTrashEntry(entry);
};
const canTrashEntries = entries && entries.length > 0 &&
entries.every(isModifiableAndNotInTrashRoot);
if (canTrashEntries && (!failureUrls || failureUrls.length === 0)) {
startIOTask(
chrome.fileManagerPrivate.IOTaskType.TRASH, entries,
/*params=*/ {});
}
this.clearDropTarget_();
return;
}
if (!this.canPasteOrDrop_(event.dataTransfer, destinationEntry)) {
return;
}
Expand Down Expand Up @@ -1335,6 +1361,13 @@ export class FileTransferController {
return false; // Unsupported type of content.
}

// A drop on the Trash root should always perform a "Send to Trash"
// operation.
if (destinationLocationInfo.rootType ===
VolumeManagerCommon.RootType.TRASH) {
return this.canTrashSelection_(destinationLocationInfo, clipboardData);
}

const sourceUrls = (clipboardData.getData('fs/sources') || '').split('\n');
if (this.getSourceRootURL_(
clipboardData, this.getDragAndDropGlobalData_()) !==
Expand Down Expand Up @@ -1539,6 +1572,16 @@ export class FileTransferController {
DropEffectType.NONE,
strf('DROP_TARGET_FOLDER_NO_MOVE_PERMISSION', destinationEntry.name));
}
// Files can be dragged onto the TrashRootEntry, but they must reside on a
// volume that is trashable.
if (destinationLocationInfo.rootType ===
VolumeManagerCommon.RootType.TRASH) {
const effect = (this.canTrashSelection_(
destinationLocationInfo, event.dataTransfer)) ?
DropEffectType.MOVE :
DropEffectType.NONE;
return new DropEffectAndLabel(effect, null);
}
if (util.isDropEffectAllowed(event.dataTransfer.effectAllowed, 'move')) {
if (!util.isDropEffectAllowed(event.dataTransfer.effectAllowed, 'copy')) {
return new DropEffectAndLabel(DropEffectType.MOVE, null);
Expand All @@ -1557,6 +1600,65 @@ export class FileTransferController {
return new DropEffectAndLabel(DropEffectType.COPY, null);
}

/**
* Identifies if the current selection can be sent to the trash. Items can be
* dragged and dropped onto the TrashRootEntry, but they must all come from a
* valid location that supports trash.
* The URLs are compared against the volumes that are enabled for trashing.
* This is to avoid blocking the drag drop operation with resolution of the
* URLs to entries. This has the unfortunate side effect of not being able to
* identify any non modifiable entries after a directory change but prior to
* the drop event occurring.
* @param {DirectoryEntry|FilesAppDirEntry|EntryLocation|null}
* destinationEntry The destination for the current selection.
* @param {DataTransfer} clipboardData Data transfer object.
* @returns {boolean} True if the selection can be trashed, false otherwise.
* @private
*/
canTrashSelection_(destinationEntry, clipboardData) {
if (!util.isTrashEnabled() || !destinationEntry) {
return false;
}
if (destinationEntry.rootType !== VolumeManagerCommon.RootType.TRASH) {
return false;
}
if (!clipboardData) {
return false;
}
const enabledTrashURLs = getEnabledTrashVolumeURLs(this.volumeManager_);
// When the dragDrop event starts the selectionHandler_ contains the initial
// selection, this is preferable to identify whether the selection is
// available or not as the sources have resolved entries already.
const {entries} = this.selectionHandler_.selection;
if (entries && entries.length > 0) {
for (const entry of entries) {
if (util.isNonModifiable(this.volumeManager_, entry)) {
return false;
}
const entryURL = entry.toURL();
if (enabledTrashURLs.some(
volumeURL => entryURL.startsWith(volumeURL))) {
continue;
}
return false;
}
return true;
}
// If the selection is cleared the directory may have changed but the drag
// event is still active. The only way to validate if the selection is
// trashable now is to compare the `sourceRootURL` against the enabled trash
// locations.
// TODO(b/241517469): At this point the sourceRootURL may be on an enabled
// location but the entry may not be trashable (e.g. Downloads and the
// Camera folder). When the drop event occurs the URLs get resolved to
// entries to ensure the operation can occur, but this may result in a move
// operation showing as allowed when the drop doesn't accept it.
const sourceRootURL =
this.getSourceRootURL_(clipboardData, this.getDragAndDropGlobalData_());
return enabledTrashURLs.some(
volumeURL => sourceRootURL.startsWith(volumeURL));
}

/**
* Blinks the selection. Used to give feedback when copying or cutting the
* selection.
Expand Down

0 comments on commit f86f07e

Please sign in to comment.