Merge pull request #60901 from nextcloud/backport/60519/stable32

[stable32] fix(files): Chromium-based browsers drag-and-drop

apps/files/src/components/FileEntryMixin.ts

@@ -6,19 +6,16 @@
 import type { PropType } from 'vue'
 import type { FileSource } from '../types.ts'
 
-import { openConflictPicker } from '@nextcloud/dialogs'
 import { FileType, Folder, getFileActions, File as NcFile, Node, NodeStatus, Permission } from '@nextcloud/files'
 import { t } from '@nextcloud/l10n'
 import { extname } from '@nextcloud/paths'
 import { isPublicShare } from '@nextcloud/sharing/public'
 import { generateUrl } from '@nextcloud/router'
-import { getConflicts, getUploader } from '@nextcloud/upload'
 import { vOnClickOutside } from '@vueuse/components'
-import { relative } from 'path'
 import Vue, { computed, defineComponent } from 'vue'
 
 import { action as sidebarAction } from '../actions/sidebarAction.ts'
-import { onDropInternalFiles } from '../services/DropService.ts'
+import { dataTransferToFileTree, onDropExternalFiles, onDropInternalFiles } from '../services/DropService.ts'
 import { getDragAndDropPreview } from '../utils/dragUtils.ts'
 import { hashCode } from '../utils/hashUtils.ts'
 import { isDownloadable } from '../utils/permissions.ts'
@@ -472,46 +469,34 @@ export default defineComponent({
 			const items = Array.from(event.dataTransfer?.items || [])
 
 			if (selection.length === 0 && items.some((item) => item.kind === 'file')) {
-				const files = items.filter((item) => item.kind === 'file')
-					.map((item) => 'webkitGetAsEntry' in item ? item.webkitGetAsEntry() : item.getAsFile())
-					.filter(Boolean) as (FileSystemEntry | File)[]
-				const uploader = getUploader()
-				const root = uploader.destination.path
-				const relativePath = relative(root, this.source.path)
-				logger.debug('Start uploading dropped files', { target: this.source.path, root, relativePath, files: files.map((file) => file.name) })
+				// Snapshot DataTransfer items immediately so Blink clears data.items
+				// after the first async yield. Then convert FileSystemEntry to File
+				// inside dataTransferToFileTree (duck-typed via entry.isFile) rather
+				// than deferring to @nextcloud/upload's batchUpload, whose
+				// instanceof-based conversion silently no-ops on some Chromium builds.
+				// See https://github.com/nextcloud/server/issues/60139
+				const fileTree = await dataTransferToFileTree(items)
 
-				await uploader.batchUpload(
-					relativePath,
-					files,
-					async (nodes, path) => {
-						try {
-							const { contents, folder } = await this.currentView!.getContents(path)
-							const conflicts = getConflicts(nodes, contents)
-							if (conflicts.length === 0) {
-								return nodes
-							}
+				// canDrop already gates this branch on FileType.Folder, but the
+				// type system can't see that — narrow defensively so a future
+				// loosening of canDrop can't silently lie via the cast below.
+				// Use the `type` field rather than `instanceof Folder`: apps
+				// bundle their own copy of @nextcloud/files, so a Folder from
+				// an app would not be `instanceof` the server's Folder class.
+				if (this.source.type !== FileType.Folder) {
+					logger.error('onDrop: external drop target is not a Folder', { source: this.source })
+					this.dragover = false
+					return
+				}
 
-							const result = await openConflictPicker(
-								folder.displayname,
-								conflicts,
-								(contents as Node[]).filter((node) => conflicts.some((conflict) => conflict.name === node.basename)),
-								{
-									recursive: true,
-								},
-							)
-							if (result === null) {
-								return false
-							}
-							return [
-								...nodes.filter((node) => !conflicts.some((conflict) => conflict.name === node.name)),
-								...result.selected,
-								...result.renamed,
-							]
-						} catch {
-							return nodes
-						}
-					},
-				)
+				// Fetch destination contents for conflict resolution
+				const cachedContents = this.filesStore.getNodesByPath(this.activeView.id, this.source.path)
+				const contents = cachedContents.length === 0
+					? (await this.activeView!.getContents(this.source.path)).contents
+					: cachedContents
+
+				logger.debug('Start uploading dropped files', { target: this.source.path, fileTree })
+				await onDropExternalFiles(fileTree, this.source as Folder, contents)
 				this.dragover = false
 				return
 			}

cypress/e2e/files/drag-n-drop.cy.ts

@@ -2,7 +2,9 @@
  * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-import { getRowForFile } from './FilesUtils.ts'
+import type { User } from '@nextcloud/e2e-test-server/cypress'
+
+import { getRowForFile, navigateToFolder } from './FilesUtils.ts'
 
 describe('files: Drag and Drop', { testIsolation: true }, () => {
 	beforeEach(() => {
@@ -138,3 +140,99 @@ describe('files: Drag and Drop', { testIsolation: true }, () => {
 		getRowForFile('Bar').should('not.exist')
 	})
 })
+
+// Regression coverage for https://github.com/nextcloud/server/issues/60139
+// The per-row drop handler in FileEntryMixin used to pass raw FileSystemEntry
+// objects to @nextcloud/upload's batchUpload; on some Chromium builds the
+// instanceof-based conversion silently failed and the chunk uploader crashed
+// with "e.slice is not a function". The fix routes the per-row drop through
+// the same dataTransferToFileTree pipeline as the main file-list drop.
+//
+// Sibling describe (not nested) so the outer suite's `beforeEach` doesn't
+// spin up an unused user before each test in this block.
+describe('files: Drag and Drop onto a folder row', { testIsolation: true }, () => {
+	let user: User
+
+	beforeEach(() => {
+		cy.createRandomUser().then((u) => {
+			user = u
+			cy.mkdir(user, '/subfolder')
+			cy.login(user)
+		})
+		cy.visit('/apps/files')
+		getRowForFile('subfolder').should('be.visible')
+	})
+
+	it('can drop a single file onto a subfolder row', () => {
+		cy.intercept('PUT', /\/remote.php\/dav\/files\//).as('uploadFile')
+
+		getRowForFile('subfolder').selectFile({
+			fileName: 'dropped-into-subfolder.txt',
+			contents: ['hello '.repeat(1024)],
+		}, { action: 'drag-drop' })
+
+		cy.wait('@uploadFile').its('request.url')
+			.should('match', /\/subfolder\/dropped-into-subfolder\.txt$/)
+
+		cy.get('[data-cy-upload-picker] progress').should('not.be.visible')
+
+		navigateToFolder('/subfolder')
+		getRowForFile('dropped-into-subfolder.txt').should('be.visible')
+	})
+
+	it('can drop multiple files onto a subfolder row', () => {
+		cy.intercept('PUT', /\/remote.php\/dav\/files\//).as('uploadFile')
+
+		getRowForFile('subfolder').selectFile([
+			{ fileName: 'one.txt', contents: ['A'.repeat(1024)] },
+			{ fileName: 'two.txt', contents: ['B'.repeat(1024)] },
+		], { action: 'drag-drop' })
+
+		// Both files must land under the subfolder, not the current dir.
+		cy.wait(['@uploadFile', '@uploadFile']).then((intercepts) => {
+			const urls = intercepts.map((i) => i.request.url).sort()
+			expect(urls).to.have.length(2)
+			urls.forEach((url) => {
+				expect(url).to.match(/\/subfolder\/(one|two)\.txt$/)
+			})
+		})
+
+		cy.get('[data-cy-upload-picker] progress').should('not.be.visible')
+
+		navigateToFolder('/subfolder')
+		getRowForFile('one.txt').should('be.visible')
+		getRowForFile('two.txt').should('be.visible')
+	})
+
+	it('opens the conflict picker when dropping a colliding name onto a subfolder row', () => {
+		// Pre-populate the subfolder with a file the drop will collide with.
+		// cy.uploadContent internally clears session cookies, so re-login
+		// before revisiting so it matches the uploadContent > login > visit
+		// pattern used elsewhere in the suite.
+		cy.uploadContent(user, new Blob(['original']), 'text/plain', '/subfolder/collide.txt')
+		cy.login(user)
+
+		// Reload so the pre-populated file lands in the store before the drop.
+		// The drop handler reads filesStore.getNodesByPath first and only
+		// fetches fresh contents when the cache is empty, so a stale cache
+		// from the beforeEach visit would let the upload proceed without
+		// triggering the conflict picker. If this ever flaps on CI, replace
+		// the visit with cy.reload() + an explicit wait on store settlement.
+		cy.visit('/apps/files')
+		getRowForFile('subfolder').should('be.visible')
+
+		cy.intercept('PUT', /\/remote.php\/dav\/files\//).as('uploadFile')
+
+		getRowForFile('subfolder').selectFile({
+			fileName: 'collide.txt',
+			contents: ['replacement '.repeat(1024)],
+		}, { action: 'drag-drop' })
+
+		// Wait for the conflict picker to appear, then assert no PUT has
+		// fired yet — chained so the upload-count check happens *after* the
+		// dialog is visible, enforcing the "dialog blocks upload" invariant.
+		cy.findByRole('dialog').should('be.visible').then(() => {
+			cy.get('@uploadFile.all').should('have.length', 0)
+		})
+	})
+})