From 528ee1ae3daabe30c1307cf9dcd6e77eb96094bc Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 25 May 2022 18:29:28 -0700 Subject: [PATCH] Allow multiple entries with the same mimetype in dataTransfer (#150425) Currently our data transfer implementation only allows a single entry of each mimeType. There can only be a single `image/gif` file for example. However this doesn't match how the DOM apis work. If you drop multiple gifs into VS Code for example, the DataTransfer you get contains entries for each of the gifs. This change allows us to also support DataTransfers that have multiple entries with the same mime type. Just like with the DOM, we support constructing these duplicate mime data transfers internally, but do not allow extensions to create them As part of this change, I've also made a few clean ups: - Add helpers for creating dataTransfer items - Clarify when adding a data transfer item should `append` or `replace` - Adopt some helper functions in a few more places --- src/vs/base/common/dataTransfer.ts | 75 ++++++++++++------- src/vs/editor/browser/dnd.ts | 19 +++-- .../copyPaste/browser/copyPasteController.ts | 12 +-- .../browser/dropIntoEditorContribution.ts | 27 ++----- .../api/browser/mainThreadLanguageFeatures.ts | 4 +- .../api/browser/mainThreadTreeViews.ts | 4 +- .../api/common/extHostTypeConverters.ts | 10 +-- src/vs/workbench/api/common/extHostTypes.ts | 23 +++++- .../workbench/browser/parts/views/treeView.ts | 16 ++-- 9 files changed, 104 insertions(+), 86 deletions(-) diff --git a/src/vs/base/common/dataTransfer.ts b/src/vs/base/common/dataTransfer.ts index d5d99bd283b..5074f30c75e 100644 --- a/src/vs/base/common/dataTransfer.ts +++ b/src/vs/base/common/dataTransfer.ts @@ -17,55 +17,74 @@ export interface IDataTransferItem { value: any; } +export function createStringDataTransferItem(stringOrPromise: string | Promise): IDataTransferItem { + return { + asString: async () => stringOrPromise, + asFile: () => undefined, + value: typeof stringOrPromise === 'string' ? stringOrPromise : undefined, + }; +} + +export function createFileDataTransferItem(fileName: string, uri: URI | undefined, data: () => Promise): IDataTransferItem { + return { + asString: async () => '', + asFile: () => ({ name: fileName, uri, data }), + value: undefined, + }; +} + export class VSDataTransfer { - private readonly _data = new Map(); + private readonly _entries = new Map(); public get size(): number { - return this._data.size; + return this._entries.size; } public has(mimeType: string): boolean { - return this._data.has(mimeType); + return this._entries.has(this.toKey(mimeType)); } public get(mimeType: string): IDataTransferItem | undefined { - return this._data.get(mimeType); + return this._entries.get(this.toKey(mimeType))?.[0]; } - public set(mimeType: string, value: IDataTransferItem): void { - this._data.set(mimeType, value); + public append(mimeType: string, value: IDataTransferItem): void { + const existing = this._entries.get(mimeType); + if (existing) { + existing.push(value); + } else { + this._entries.set(this.toKey(mimeType), [value]); + } + } + + public replace(mimeType: string, value: IDataTransferItem): void { + this._entries.set(this.toKey(mimeType), [value]); } public delete(mimeType: string) { - this._data.delete(mimeType); + this._entries.delete(this.toKey(mimeType)); } - public setString(mimeType: string, stringOrPromise: string | Promise) { - this.set(mimeType, { - asString: async () => stringOrPromise, - asFile: () => undefined, - value: typeof stringOrPromise === 'string' ? stringOrPromise : undefined, - }); + public *entries(): Iterable<[string, IDataTransferItem]> { + for (const [mine, items] of this._entries.entries()) { + for (const item of items) { + yield [mine, item]; + } + } } - public setFile(mimeType: string, fileName: string, uri: URI | undefined, data: () => Promise) { - this.set(mimeType, { - asString: async () => '', - asFile: () => ({ name: fileName, uri, data }), - value: undefined, - }); - } - - public entries(): IterableIterator<[string, IDataTransferItem]> { - return this._data.entries(); - } - - public values(): IterableIterator { - return this._data.values(); + public values(): Iterable { + return Array.from(this._entries.values()).flat(); } public forEach(f: (value: IDataTransferItem, key: string) => void) { - this._data.forEach(f); + for (const [mime, item] of this.entries()) { + f(item, mime); + } + } + + private toKey(mimeType: string): string { + return mimeType.toLowerCase(); } } diff --git a/src/vs/editor/browser/dnd.ts b/src/vs/editor/browser/dnd.ts index 08a3f558786..80a5958504d 100644 --- a/src/vs/editor/browser/dnd.ts +++ b/src/vs/editor/browser/dnd.ts @@ -3,8 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { VSDataTransfer } from 'vs/base/common/dataTransfer'; +import { createFileDataTransferItem, createStringDataTransferItem, IDataTransferItem, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { URI } from 'vs/base/common/uri'; +import { FileAdditionalNativeProperties } from 'vs/platform/dnd/browser/dnd'; export function toVSDataTransfer(dataTransfer: DataTransfer) { @@ -13,16 +14,20 @@ export function toVSDataTransfer(dataTransfer: DataTransfer) { const type = item.type; if (item.kind === 'string') { const asStringValue = new Promise(resolve => item.getAsString(resolve)); - vsDataTransfer.setString(type, asStringValue); + vsDataTransfer.append(type, createStringDataTransferItem(asStringValue)); } else if (item.kind === 'file') { - const file = item.getAsFile() as null | (File & { path?: string }); + const file = item.getAsFile(); if (file) { - const uri = file.path ? URI.parse(file.path) : undefined; - vsDataTransfer.setFile(type, file.name, uri, async () => { - return new Uint8Array(await file.arrayBuffer()); - }); + vsDataTransfer.append(type, createFileDataTransferItemFromFile(file)); } } } return vsDataTransfer; } + +export function createFileDataTransferItemFromFile(file: File): IDataTransferItem { + const uri = (file as FileAdditionalNativeProperties).path ? URI.parse((file as FileAdditionalNativeProperties).path!) : undefined; + return createFileDataTransferItem(file.name, uri, async () => { + return new Uint8Array(await file.arrayBuffer()); + }); +} diff --git a/src/vs/editor/contrib/copyPaste/browser/copyPasteController.ts b/src/vs/editor/contrib/copyPaste/browser/copyPasteController.ts index 6c203af56cd..74ddf65d392 100644 --- a/src/vs/editor/contrib/copyPaste/browser/copyPasteController.ts +++ b/src/vs/editor/contrib/copyPaste/browser/copyPasteController.ts @@ -6,7 +6,7 @@ import { addDisposableListener } from 'vs/base/browser/dom'; import { CancelablePromise, createCancelablePromise } from 'vs/base/common/async'; import { CancellationToken } from 'vs/base/common/cancellation'; -import { VSDataTransfer } from 'vs/base/common/dataTransfer'; +import { createStringDataTransferItem, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { Disposable } from 'vs/base/common/lifecycle'; import { Mimes } from 'vs/base/common/mime'; import { generateUuid } from 'vs/base/common/uuid'; @@ -103,7 +103,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi for (const result of results) { result?.forEach((value, key) => { - dataTransfer.set(key, value); + dataTransfer.replace(key, value); }); } @@ -148,7 +148,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi if (handle && this._currentClipboardItem?.handle === handle) { const toMergeDataTransfer = await this._currentClipboardItem.dataTransferPromise; toMergeDataTransfer.forEach((value, key) => { - dataTransfer.set(key, value); + dataTransfer.append(key, value); }); } @@ -156,11 +156,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi const resources = await this._clipboardService.readResources(); if (resources.length) { const value = resources.join('\n'); - dataTransfer.set(Mimes.uriList, { - value, - asString: async () => value, - asFile: () => undefined, - }); + dataTransfer.append(Mimes.uriList, createStringDataTransferItem(value)); } } diff --git a/src/vs/editor/contrib/dropIntoEditor/browser/dropIntoEditorContribution.ts b/src/vs/editor/contrib/dropIntoEditor/browser/dropIntoEditorContribution.ts index b713b71a7dd..4abda27de6c 100644 --- a/src/vs/editor/contrib/dropIntoEditor/browser/dropIntoEditorContribution.ts +++ b/src/vs/editor/contrib/dropIntoEditor/browser/dropIntoEditorContribution.ts @@ -5,11 +5,12 @@ import { distinct } from 'vs/base/common/arrays'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; -import { VSDataTransfer } from 'vs/base/common/dataTransfer'; +import { createStringDataTransferItem, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { Disposable } from 'vs/base/common/lifecycle'; import { Mimes } from 'vs/base/common/mime'; import { relativePath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; +import { toVSDataTransfer } from 'vs/editor/browser/dnd'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { registerEditorContribution } from 'vs/editor/browser/editorExtensions'; import { IPosition } from 'vs/editor/common/core/position'; @@ -20,7 +21,7 @@ import { ITextModel } from 'vs/editor/common/model'; import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures'; import { performSnippetEdit } from 'vs/editor/contrib/snippet/browser/snippetController2'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { extractEditorsDropData, FileAdditionalNativeProperties } from 'vs/platform/dnd/browser/dnd'; +import { extractEditorsDropData } from 'vs/platform/dnd/browser/dnd'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; @@ -94,27 +95,11 @@ export class DropIntoEditorController extends Disposable implements IEditorContr } public async extractDataTransferData(dragEvent: DragEvent): Promise { - const textEditorDataTransfer = new VSDataTransfer(); if (!dragEvent.dataTransfer) { - return textEditorDataTransfer; - } - - for (const item of dragEvent.dataTransfer.items) { - const type = item.type; - if (item.kind === 'string') { - const asStringValue = new Promise(resolve => item.getAsString(resolve)); - textEditorDataTransfer.setString(type, asStringValue); - } else if (item.kind === 'file') { - const file = item.getAsFile(); - if (file) { - const uri = (file as FileAdditionalNativeProperties).path ? URI.parse((file as FileAdditionalNativeProperties).path!) : undefined; - textEditorDataTransfer.setFile(type, file.name, uri, async () => { - return new Uint8Array(await file.arrayBuffer()); - }); - } - } + return new VSDataTransfer(); } + const textEditorDataTransfer = toVSDataTransfer(dragEvent.dataTransfer); if (!textEditorDataTransfer.has(Mimes.uriList)) { const editorData = (await this._instantiationService.invokeFunction(extractEditorsDropData, dragEvent)) .filter(input => input.resource) @@ -122,7 +107,7 @@ export class DropIntoEditorController extends Disposable implements IEditorContr if (editorData.length) { const str = distinct(editorData).join('\n'); - textEditorDataTransfer.setString(Mimes.uriList.toLowerCase(), str); + textEditorDataTransfer.replace(Mimes.uriList, createStringDataTransferItem(str)); } } diff --git a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts index f89e0698b9d..ff55fc5844f 100644 --- a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts +++ b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts @@ -5,7 +5,7 @@ import { VSBuffer } from 'vs/base/common/buffer'; import { CancellationToken } from 'vs/base/common/cancellation'; -import { VSDataTransfer } from 'vs/base/common/dataTransfer'; +import { createStringDataTransferItem, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { CancellationError } from 'vs/base/common/errors'; import { Emitter, Event } from 'vs/base/common/event'; import { combinedDisposable, Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; @@ -385,7 +385,7 @@ export class MainThreadLanguageFeatures extends Disposable implements MainThread const dataTransferOut = new VSDataTransfer(); result.items.forEach(([type, item]) => { - dataTransferOut.setString(type, item.asString); + dataTransferOut.replace(type, createStringDataTransferItem(item.asString)); }); return dataTransferOut; } diff --git a/src/vs/workbench/api/browser/mainThreadTreeViews.ts b/src/vs/workbench/api/browser/mainThreadTreeViews.ts index 56133dc4b20..7c8aa9d2323 100644 --- a/src/vs/workbench/api/browser/mainThreadTreeViews.ts +++ b/src/vs/workbench/api/browser/mainThreadTreeViews.ts @@ -14,7 +14,7 @@ import { Registry } from 'vs/platform/registry/common/platform'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { ILogService } from 'vs/platform/log/common/log'; import { CancellationToken } from 'vs/base/common/cancellation'; -import { VSDataTransfer } from 'vs/base/common/dataTransfer'; +import { createStringDataTransferItem, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { VSBuffer } from 'vs/base/common/buffer'; import { DataTransferCache } from 'vs/workbench/api/common/shared/dataTransferCache'; import * as typeConvert from 'vs/workbench/api/common/extHostTypeConverters'; @@ -225,7 +225,7 @@ class TreeViewDragAndDropController implements ITreeViewDragAndDropController { const additionalDataTransfer = new VSDataTransfer(); additionalDataTransferDTO.items.forEach(([type, item]) => { - additionalDataTransfer.setString(type, item.asString); + additionalDataTransfer.replace(type, createStringDataTransferItem(item.asString)); }); return additionalDataTransfer; } diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 95e24e8692a..18d71be8a26 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -40,6 +40,7 @@ import { ACTIVE_GROUP, SIDE_GROUP } from 'vs/workbench/services/editor/common/ed import type * as vscode from 'vscode'; import * as types from './extHostTypes'; import { once } from 'vs/base/common/functional'; +import { VSDataTransfer } from 'vs/base/common/dataTransfer'; export namespace Command { @@ -1980,14 +1981,13 @@ export namespace DataTransferItem { export namespace DataTransfer { export function toDataTransfer(value: extHostProtocol.DataTransferDTO, resolveFileData: (dataItemIndex: number) => Promise): types.DataTransfer { - const newDataTransfer = new types.DataTransfer(); - value.items.forEach(([type, item], index) => { - newDataTransfer.set(type, DataTransferItem.toDataTransferItem(item, () => resolveFileData(index))); + const init = value.items.map(([type, item], index) => { + return [type, DataTransferItem.toDataTransferItem(item, () => resolveFileData(index))] as const; }); - return newDataTransfer; + return new types.DataTransfer(init); } - export async function toDataTransferDTO(value: vscode.DataTransfer): Promise { + export async function toDataTransferDTO(value: vscode.DataTransfer | VSDataTransfer): Promise { const newDTO: extHostProtocol.DataTransferDTO = { items: [] }; const promises: Promise[] = []; diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 76ae9232db9..a1c2a3ea107 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -2451,18 +2451,33 @@ export class DataTransferItem { @es5ClassCompat export class DataTransfer { - #items = new Map(); + #items = new Map(); + + constructor(init?: Iterable) { + for (const [mime, item] of init ?? []) { + const existing = this.#items.get(mime); + if (existing) { + existing.push(item); + } else { + this.#items.set(mime, [item]); + } + } + } get(mimeType: string): DataTransferItem | undefined { - return this.#items.get(mimeType); + return this.#items.get(mimeType)?.[0]; } set(mimeType: string, value: DataTransferItem): void { - this.#items.set(mimeType, value); + // This intentionally overwrites all entries for a given mimetype. + // This is similar to how the DOM DataTransfer type works + this.#items.set(mimeType, [value]); } forEach(callbackfn: (value: DataTransferItem, key: string) => void): void { - this.#items.forEach(callbackfn); + for (const [mime, items] of this.#items) { + items.forEach(item => callbackfn(item, mime)); + } } } diff --git a/src/vs/workbench/browser/parts/views/treeView.ts b/src/vs/workbench/browser/parts/views/treeView.ts index 051926ebcb4..524979149fe 100644 --- a/src/vs/workbench/browser/parts/views/treeView.ts +++ b/src/vs/workbench/browser/parts/views/treeView.ts @@ -31,7 +31,7 @@ import { isString } from 'vs/base/common/types'; import { URI } from 'vs/base/common/uri'; import { generateUuid } from 'vs/base/common/uuid'; import 'vs/css!./media/views'; -import { VSDataTransfer } from 'vs/base/common/dataTransfer'; +import { createStringDataTransferItem, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { Command } from 'vs/editor/common/languages'; import { localize } from 'vs/nls'; import { createActionViewItem, createAndFillInContextMenuActions } from 'vs/platform/actions/browser/menuEntryActionViewItem'; @@ -66,7 +66,8 @@ import { IExtensionService } from 'vs/workbench/services/extensions/common/exten import { IHoverService } from 'vs/workbench/services/hover/browser/hover'; import { ThemeSettings } from 'vs/workbench/services/themes/common/workbenchThemeService'; import { ITreeViewsService } from 'vs/workbench/services/views/browser/treeViewsService'; -import { CodeDataTransfers, FileAdditionalNativeProperties } from 'vs/platform/dnd/browser/dnd'; +import { CodeDataTransfers } from 'vs/platform/dnd/browser/dnd'; +import { createFileDataTransferItemFromFile } from 'vs/editor/browser/dnd'; export class TreeViewPane extends ViewPane { @@ -1524,7 +1525,7 @@ export class CustomTreeViewDragAndDrop implements ITreeDragAndDrop { } if (dataValue) { const converted = this.convertKnownMimes(type, kind, dataValue); - treeDataTransfer.setString(converted.type, converted.value + ''); + treeDataTransfer.append(converted.type, createStringDataTransferItem(converted.value + '')); } resolve(); })); @@ -1532,11 +1533,8 @@ export class CustomTreeViewDragAndDrop implements ITreeDragAndDrop { const file = dataItem.getAsFile(); if (file) { uris.push(URI.file(file.path)); - const uri = (file as FileAdditionalNativeProperties).path ? URI.parse((file as FileAdditionalNativeProperties).path!) : undefined; if (dndController.supportsFileDataTransfers) { - treeDataTransfer.setFile(type, file.name, uri, async () => { - return new Uint8Array(await file.arrayBuffer()); - }); + treeDataTransfer.append(type, createFileDataTransferItemFromFile(file)); } } } @@ -1545,7 +1543,7 @@ export class CustomTreeViewDragAndDrop implements ITreeDragAndDrop { // Check if there are uris to add and add them if (uris.length) { - treeDataTransfer.setString(Mimes.uriList, uris.map(uri => uri.toString()).join('\n')); + treeDataTransfer.replace(Mimes.uriList, createStringDataTransferItem(uris.map(uri => uri.toString()).join('\n'))); } const additionalWillDropPromise = this.treeViewsDragAndDropService.removeDragOperationTransfer(willDropUuid); @@ -1555,7 +1553,7 @@ export class CustomTreeViewDragAndDrop implements ITreeDragAndDrop { return additionalWillDropPromise.then(additionalDataTransfer => { if (additionalDataTransfer) { for (const item of additionalDataTransfer.entries()) { - treeDataTransfer.set(item[0], item[1]); + treeDataTransfer.append(item[0], item[1]); } } return dndController.handleDrop(treeDataTransfer, targetNode, new CancellationTokenSource().token, willDropUuid, treeSourceInfo?.id, treeSourceInfo?.itemHandles);