Skip to content

Commit e780720

Browse files
solracsfsusnux
authored andcommitted
fix(files): Chromium-based browsers drag-and-drop
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent 6aa4c89 commit e780720

2 files changed

Lines changed: 121 additions & 45 deletions

File tree

apps/files/src/components/FileEntryMixin.ts

Lines changed: 26 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,16 @@
66
import type { PropType } from 'vue'
77
import type { FileSource } from '../types.ts'
88

9-
import { openConflictPicker } from '@nextcloud/dialogs'
109
import { FileType, Folder, getFileActions, File as NcFile, Node, NodeStatus, Permission } from '@nextcloud/files'
1110
import { t } from '@nextcloud/l10n'
1211
import { extname } from '@nextcloud/paths'
1312
import { isPublicShare } from '@nextcloud/sharing/public'
1413
import { generateUrl } from '@nextcloud/router'
15-
import { getConflicts, getUploader } from '@nextcloud/upload'
1614
import { vOnClickOutside } from '@vueuse/components'
17-
import { relative } from 'path'
1815
import Vue, { computed, defineComponent } from 'vue'
1916

2017
import { action as sidebarAction } from '../actions/sidebarAction.ts'
21-
import { onDropInternalFiles } from '../services/DropService.ts'
18+
import { dataTransferToFileTree, onDropExternalFiles, onDropInternalFiles } from '../services/DropService.ts'
2219
import { getDragAndDropPreview } from '../utils/dragUtils.ts'
2320
import { hashCode } from '../utils/hashUtils.ts'
2421
import { isDownloadable } from '../utils/permissions.ts'
@@ -472,46 +469,31 @@ export default defineComponent({
472469
const items = Array.from(event.dataTransfer?.items || [])
473470

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

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

0 commit comments

Comments
 (0)