From acc4fd5ad202e4857893312d3ce081e5fb518e3e Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 16 Sep 2026 15:01:47 -0400 Subject: [PATCH] Pass a temp copy of dragged attachment files on Windows Allowing only 'copy' for file attachment drags kept File Explorer from moving files out of 'storage' but locked the cursor at '+' even for moves within Zotero. Instead, provide the file via a flavor data provider that copies it to the temp directory when a target asks for it, so the drag can allow 'copyMove' again and Explorer's move only touches the copy. A copy's directory is removed once Explorer has moved the file out of it, at the next drag, or with the temp directory at shutdown. --- .../zotero/xpcom/fileDragDataProvider.js | 100 ++++++++++++++++++ .../zotero/xpcom/utilities_internal.js | 28 ++++- test/tests/fileDragDataProviderTest.js | 48 +++++++++ 3 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 test/tests/fileDragDataProviderTest.js diff --git a/chrome/content/zotero/xpcom/fileDragDataProvider.js b/chrome/content/zotero/xpcom/fileDragDataProvider.js index 618f0a899c..7749d5728d 100644 --- a/chrome/content/zotero/xpcom/fileDragDataProvider.js +++ b/chrome/content/zotero/xpcom/fileDragDataProvider.js @@ -243,3 +243,103 @@ Zotero.FileDragDataProvider.prototype = { } } + +/** + * Implements nsIFlavorDataProvider for a single dragged attachment file, handing drop targets a + * copy in the temp directory instead of the file in storage. The copy is made only when a target + * asks for the file, so drags within Zotero don't copy anything, and a target that moves the + * dropped file (e.g., File Explorer on Windows for an unmodified drag) moves the copy. + * + * A target can still be working on the copy after the drag ends -- e.g., while a replace/skip + * dialog is open -- and gives no signal when it's done, so a copy's directory is removed only + * once it's empty (the target moved the file), at the start of the next drag, or with the temp + * directory at shutdown. + * + * @param {String} path + */ +Zotero.TempFileDragDataProvider = function (path) { + this._path = path; + this._file = null; +}; + +Zotero.TempFileDragDataProvider._copiedSinceDragEnd = false; + +/** + * Remove directories created for copies + * + * @param {Object} [options] + * @param {Boolean} [options.onlyEmpty=false] - Remove only directories whose file the target has + * moved away, which is safe while a target may still be using another copy + */ +Zotero.TempFileDragDataProvider.removeCopies = async function ({ onlyEmpty = false } = {}) { + let tmpDir = Zotero.getTempDirectory().path; + let children; + try { + children = await IOUtils.getChildren(tmpDir); + } + catch (e) { + Zotero.debug(e, 2); + return; + } + for (let dir of children) { + if (!/^drag(-\d+)?$/.test(PathUtils.filename(dir))) { + continue; + } + try { + if (onlyEmpty) { + if ((await IOUtils.getChildren(dir)).length) { + continue; + } + // Non-recursive, so a copy made in the meantime is never removed + await IOUtils.remove(dir); + } + else { + await IOUtils.remove(dir, { recursive: true }); + } + Zotero.debug(`Removed drag copy directory ${dir}`); + } + // A target may still have a copy open + catch (e) { + Zotero.debug(e, 2); + } + } +}; + +/** + * Call when a drag that offered files through these providers ends + */ +Zotero.TempFileDragDataProvider.onDragEnd = function () { + if (!this._copiedSinceDragEnd) { + return; + } + this._copiedSinceDragEnd = false; + // An unmodified drop in File Explorer moves the copy right away, so remove the emptied + // directory shortly after, with a later pass for a target that took longer + for (let delay of [2000, 30000]) { + setTimeout(() => this.removeCopies({ onlyEmpty: true }), delay); + } +}; + +Zotero.TempFileDragDataProvider.prototype = { + QueryInterface: ChromeUtils.generateQI(["nsIFlavorDataProvider"]), + + getFlavorData(_transferable, flavor, data) { + if (flavor != 'application/x-moz-file') { + return; + } + // Targets ask for the file repeatedly during a drag, so make the copy once + if (!this._file) { + let file = Zotero.File.pathToFile(this._path); + // Copy into a directory of its own so that the copy keeps its filename + let dir = Zotero.getTempDirectory(); + dir.append('drag'); + dir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + Zotero.debug(`Copying ${this._path} to ${dir.path} for drag`); + file.copyTo(dir, null); + Zotero.TempFileDragDataProvider._copiedSinceDragEnd = true; + dir.append(file.leafName); + this._file = dir; + } + data.value = this._file; + } +}; diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js index c259bcc6b2..33d5a1cf7f 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -3442,6 +3442,11 @@ Zotero.Utilities.Internal.onDragItems = function (event, itemIDs, dragImage = ev .filter(file => file.exists()); if (files.length) { + if (Zotero.isWin) { + // Copies from earlier drags are no longer in use + Zotero.TempFileDragDataProvider.removeCopies(); + } + // Advanced multi-file drag (with unique filenames, which otherwise happen automatically on // Windows but not Linux) and auxiliary snapshot file copying on macOS let dataProvider; @@ -3464,15 +3469,30 @@ Zotero.Utilities.Internal.onDragItems = function (event, itemIDs, dragImage = ev event.dataTransfer.mozSetDataAt("text/x-moz-url", uri + '\n' + file.leafName, i); } - // Allow dragging to web targets (e.g., Gmail) + // Allow dragging to the filesystem and web targets (e.g., Gmail). On Windows, hand + // targets a copy in the temp directory, since File Explorer moves a dropped file for + // an unmodified drag and would otherwise move it out of storage. Zotero.debug("Adding application/x-moz-file " + i); - event.dataTransfer.mozSetDataAt("application/x-moz-file", file, i); + event.dataTransfer.mozSetDataAt( + "application/x-moz-file", + Zotero.isWin ? new Zotero.TempFileDragDataProvider(files[i]) : file, + i + ); - if (Zotero.isWin || Zotero.isLinux) { - // Copy rather than move (Windows) or symlink (Linux) for an unmodified drag + if (Zotero.isLinux) { + // Copy rather than symlink for an unmodified drag. Drops within Zotero can still + // move -- see LibraryTreeView::setDropEffect(). event.dataTransfer.effectAllowed = 'copy'; } } + + if (Zotero.isWin) { + event.currentTarget.addEventListener( + 'dragend', + () => Zotero.TempFileDragDataProvider.onDragEnd(), + { once: true } + ); + } } // Get Quick Copy format for current URL (set via /ping from connector) diff --git a/test/tests/fileDragDataProviderTest.js b/test/tests/fileDragDataProviderTest.js new file mode 100644 index 0000000000..179a71f1fa --- /dev/null +++ b/test/tests/fileDragDataProviderTest.js @@ -0,0 +1,48 @@ +"use strict"; + +describe("Zotero.TempFileDragDataProvider", function () { + it("should hand out a copy of the file with the same name and remove its directory once emptied", async function () { + var attachment = await importFileAttachment('test.pdf'); + var path = attachment.getFilePath(); + var provider = new Zotero.TempFileDragDataProvider(path); + + var data = {}; + provider.getFlavorData(null, 'application/x-moz-file', data); + var file = data.value.QueryInterface(Ci.nsIFile); + var copyPath = file.path; + var copyDir = PathUtils.parent(copyPath); + + assert.notEqual(copyPath, path); + assert.equal(file.leafName, PathUtils.filename(path)); + assert.isTrue(copyPath.startsWith(Zotero.getTempDirectory().path)); + assert.equal( + await Zotero.File.getBinaryContentsAsync(copyPath), + await Zotero.File.getBinaryContentsAsync(path) + ); + + // Repeated requests get the same copy + var data2 = {}; + provider.getFlavorData(null, 'application/x-moz-file', data2); + assert.equal(data2.value.QueryInterface(Ci.nsIFile).path, copyPath); + + // A copy the target may still be using stays + await Zotero.TempFileDragDataProvider.removeCopies({ onlyEmpty: true }); + assert.isTrue(await IOUtils.exists(copyPath)); + + // Once the target has moved the file away, the directory goes + await IOUtils.remove(copyPath); + await Zotero.TempFileDragDataProvider.removeCopies({ onlyEmpty: true }); + assert.isFalse(await IOUtils.exists(copyDir)); + }); + + it("should remove leftover copies from earlier drags", async function () { + var attachment = await importFileAttachment('test.pdf'); + var provider = new Zotero.TempFileDragDataProvider(attachment.getFilePath()); + var data = {}; + provider.getFlavorData(null, 'application/x-moz-file', data); + var copyDir = PathUtils.parent(data.value.QueryInterface(Ci.nsIFile).path); + + await Zotero.TempFileDragDataProvider.removeCopies(); + assert.isFalse(await IOUtils.exists(copyDir)); + }); +});