Skip to content

Commit e219863

Browse files
solracsfbackportbot[bot]
authored andcommitted
fix(files): Chromium-based browsers drag-and-drop
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent cc66864 commit e219863

2 files changed

Lines changed: 121 additions & 44 deletions

File tree

apps/files/src/components/FileEntryMixin.ts

Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@ import type { IFileAction } from '@nextcloud/files'
77
import type { PropType } from 'vue'
88
import type { FileSource } from '../types.ts'
99

10-
import { openConflictPicker } from '@nextcloud/dialogs'
1110
import { FileType, Folder, File as NcFile, Node, NodeStatus, Permission } from '@nextcloud/files'
1211
import { t } from '@nextcloud/l10n'
1312
import { generateUrl } from '@nextcloud/router'
1413
import { isPublicShare } from '@nextcloud/sharing/public'
15-
import { getConflicts, getUploader } from '@nextcloud/upload'
1614
import { vOnClickOutside } from '@vueuse/components'
17-
import { extname, relative } from 'path'
15+
import { extname } from 'path'
1816
import Vue, { computed, defineComponent } from 'vue'
1917
import { action as sidebarAction } from '../actions/sidebarAction.ts'
2018
import logger from '../logger.ts'
@@ -488,46 +486,31 @@ export default defineComponent({
488486
const items = Array.from(event.dataTransfer?.items || [])
489487

490488
if (selection.length === 0 && items.some((item) => item.kind === 'file')) {
491-
const files = items.filter((item) => item.kind === 'file')
492-
.map((item) => 'webkitGetAsEntry' in item ? item.webkitGetAsEntry() : item.getAsFile())
493-
.filter(Boolean) as (FileSystemEntry | File)[]
494-
const uploader = getUploader()
495-
const root = uploader.destination.path
496-
const relativePath = relative(root, this.source.path)
497-
logger.debug('Start uploading dropped files', { target: this.source.path, root, relativePath, files: files.map((file) => file.name) })
498-
499-
await uploader.batchUpload(
500-
relativePath,
501-
files,
502-
async (nodes, path) => {
503-
try {
504-
const { contents, folder } = await this.activeView!.getContents(path)
505-
const conflicts = getConflicts(nodes, contents)
506-
if (conflicts.length === 0) {
507-
return nodes
508-
}
509-
510-
const result = await openConflictPicker(
511-
folder.displayname,
512-
conflicts,
513-
(contents as Node[]).filter((node) => conflicts.some((conflict) => conflict.name === node.basename)),
514-
{
515-
recursive: true,
516-
},
517-
)
518-
if (result === null) {
519-
return false
520-
}
521-
return [
522-
...nodes.filter((node) => !conflicts.some((conflict) => conflict.name === node.name)),
523-
...result.selected,
524-
...result.renamed,
525-
]
526-
} catch {
527-
return nodes
528-
}
529-
},
530-
)
489+
// Snapshot DataTransfer items immediately so Blink clears data.items
490+
// after the first async yield. Then convert FileSystemEntry to File
491+
// inside dataTransferToFileTree (duck-typed via entry.isFile) rather
492+
// than deferring to @nextcloud/upload's batchUpload, whose
493+
// instanceof-based conversion silently no-ops on some Chromium builds.
494+
// See https://github.com/nextcloud/server/issues/60139
495+
const fileTree = await dataTransferToFileTree(items)
496+
497+
// canDrop already gates this branch on FileType.Folder, but the
498+
// type system can't see that — narrow defensively so a future
499+
// loosening of canDrop can't silently lie via the cast below.
500+
if (!(this.source instanceof Folder)) {
501+
logger.error('onDrop: external drop target is not a Folder', { source: this.source })
502+
this.dragover = false
503+
return
504+
}
505+
506+
// Fetch destination contents for conflict resolution
507+
const cachedContents = this.filesStore.getNodesByPath(this.activeView.id, this.source.path)
508+
const contents = cachedContents.length === 0
509+
? (await this.activeView!.getContents(this.source.path)).contents
510+
: cachedContents
511+
512+
logger.debug('Start uploading dropped files', { target: this.source.path, fileTree })
513+
await onDropExternalFiles(fileTree, this.source, contents)
531514
this.dragover = false
532515
return
533516
}

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

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
5-
import { getRowForFile } from './FilesUtils.ts'
5+
import type { User } from '@nextcloud/e2e-test-server/cypress'
6+
7+
import { getRowForFile, navigateToFolder } from './FilesUtils.ts'
68

79
describe('files: Drag and Drop', { testIsolation: true }, () => {
810
beforeEach(() => {
@@ -146,3 +148,95 @@ describe('files: Drag and Drop', { testIsolation: true }, () => {
146148
getRowForFile('Bar').should('not.exist')
147149
})
148150
})
151+
152+
// Regression coverage for https://github.com/nextcloud/server/issues/60139
153+
// The per-row drop handler in FileEntryMixin used to pass raw FileSystemEntry
154+
// objects to @nextcloud/upload's batchUpload; on some Chromium builds the
155+
// instanceof-based conversion silently failed and the chunk uploader crashed
156+
// with "e.slice is not a function". The fix routes the per-row drop through
157+
// the same dataTransferToFileTree pipeline as the main file-list drop.
158+
//
159+
// Sibling describe (not nested) so the outer suite's `beforeEach` doesn't
160+
// spin up an unused user before each test in this block.
161+
describe('files: Drag and Drop onto a folder row', { testIsolation: true }, () => {
162+
let user: User
163+
164+
beforeEach(() => {
165+
cy.createRandomUser().then((u) => {
166+
user = u
167+
cy.mkdir(user, '/subfolder')
168+
cy.login(user)
169+
})
170+
cy.visit('/apps/files')
171+
getRowForFile('subfolder').should('be.visible')
172+
})
173+
174+
it('can drop a single file onto a subfolder row', () => {
175+
cy.intercept('PUT', /\/remote.php\/dav\/files\//).as('uploadFile')
176+
177+
getRowForFile('subfolder').selectFile({
178+
fileName: 'dropped-into-subfolder.txt',
179+
contents: ['hello '.repeat(1024)],
180+
}, { action: 'drag-drop' })
181+
182+
cy.wait('@uploadFile').its('request.url')
183+
.should('match', /\/subfolder\/dropped-into-subfolder\.txt$/)
184+
185+
cy.get('[data-cy-upload-picker] progress').should('not.be.visible')
186+
187+
navigateToFolder('/subfolder')
188+
getRowForFile('dropped-into-subfolder.txt').should('be.visible')
189+
})
190+
191+
it('can drop multiple files onto a subfolder row', () => {
192+
cy.intercept('PUT', /\/remote.php\/dav\/files\//).as('uploadFile')
193+
194+
getRowForFile('subfolder').selectFile([
195+
{ fileName: 'one.txt', contents: ['A'.repeat(1024)] },
196+
{ fileName: 'two.txt', contents: ['B'.repeat(1024)] },
197+
], { action: 'drag-drop' })
198+
199+
// Both files must land under the subfolder, not the current dir.
200+
cy.wait(['@uploadFile', '@uploadFile']).then((intercepts) => {
201+
const urls = intercepts.map((i) => i.request.url).sort()
202+
expect(urls).to.have.length(2)
203+
urls.forEach((url) => {
204+
expect(url).to.match(/\/subfolder\/(one|two)\.txt$/)
205+
})
206+
})
207+
208+
cy.get('[data-cy-upload-picker] progress').should('not.be.visible')
209+
210+
navigateToFolder('/subfolder')
211+
getRowForFile('one.txt').should('be.visible')
212+
getRowForFile('two.txt').should('be.visible')
213+
})
214+
215+
it('opens the conflict picker when dropping a colliding name onto a subfolder row', () => {
216+
// Pre-populate the subfolder with a file the drop will collide with.
217+
cy.uploadContent(user, new Blob(['original']), 'text/plain', '/subfolder/collide.txt')
218+
219+
// Reload so the pre-populated file lands in the store before the drop.
220+
// The drop handler reads filesStore.getNodesByPath first and only
221+
// fetches fresh contents when the cache is empty, so a stale cache
222+
// from the beforeEach visit would let the upload proceed without
223+
// triggering the conflict picker. If this ever flaps on CI, replace
224+
// the visit with cy.reload() + an explicit wait on store settlement.
225+
cy.visit('/apps/files')
226+
getRowForFile('subfolder').should('be.visible')
227+
228+
cy.intercept('PUT', /\/remote.php\/dav\/files\//).as('uploadFile')
229+
230+
getRowForFile('subfolder').selectFile({
231+
fileName: 'collide.txt',
232+
contents: ['replacement '.repeat(1024)],
233+
}, { action: 'drag-drop' })
234+
235+
// Wait for the conflict picker to appear, then assert no PUT has
236+
// fired yet — chained so the upload-count check happens *after* the
237+
// dialog is visible, enforcing the "dialog blocks upload" invariant.
238+
cy.findByRole('dialog').should('be.visible').then(() => {
239+
cy.get('@uploadFile.all').should('have.length', 0)
240+
})
241+
})
242+
})

0 commit comments

Comments
 (0)