Skip to content

fix(uui-file-dropzone): fix multi-folder drop losing all but first folder#1339

Merged
leekelleher merged 2 commits intov1/devfrom
v1/bugfix/file-dropzone-datatransfer-staleness
Mar 9, 2026
Merged

fix(uui-file-dropzone): fix multi-folder drop losing all but first folder#1339
leekelleher merged 2 commits intov1/devfrom
v1/bugfix/file-dropzone-datatransfer-staleness

Conversation

@iOvergaard
Copy link
Contributor

Summary

Fixes a browser-level data staleness bug in uui-file-dropzone that caused only the first dropped folder to be processed when multiple folders were dragged and dropped simultaneously (closes umbraco/Umbraco-CMS#21837).

Root cause

The DataTransfer object (and its items list) is only valid for the duration of a synchronous event handler. Once an async drop handler hits its first await, the browser marks the drag data store as expired — all subsequent calls to DataTransferItem.webkitGetAsEntry() return null.

The old _getAllEntries loop called await this._mkdir(firstFolder) before reading the second item, so only folder #1 was ever retrieved:

sequenceDiagram
    participant B as Browser
    participant H as _getAllEntries (OLD)
    participant DT as DataTransfer

    B->>H: drop event (DataTransfer valid)
    H->>DT: getEntry(item[0]) → folderA ✓
    H->>H: await _mkdir(folderA)
    Note over DT: ⚠️ Browser expires drag data store
    H->>DT: getEntry(item[1]) → null ✗
    H->>DT: getEntry(item[2]) → null ✗
    H-->>B: { folders: [folderA] }  ← only first folder!
Loading

Fix

Split _getAllEntries into two phases:

  1. Phase 1 (synchronous): Collect all root FileSystemEntry refs in one pass, before any await. FileSystemEntry objects remain valid indefinitely once obtained.
  2. Phase 2 (async, new _processRootEntries): Iterate the stable refs and recursively read each folder/file.
sequenceDiagram
    participant B as Browser
    participant H as _getAllEntries (NEW)
    participant DT as DataTransfer

    B->>H: drop event (DataTransfer valid)
    H->>DT: getEntry(item[0]) → folderA ✓
    H->>DT: getEntry(item[1]) → folderB ✓
    H->>DT: getEntry(item[2]) → fileC  ✓
    Note over H: Phase 1 complete — DataTransfer no longer needed
    H->>H: await _processRootEntries([folderA, folderB, fileC])
    H-->>B: { folders: [folderA, folderB], files: [fileC] } ✓
Loading

_processRootEntries also replaces the removed _processFileEntry method, which had the same staleness issue (DataTransferItem.getAsFile() → now uses FileSystemFileEntry.file() instead).

Changes

  • _getEntry return type: FileSystemDirectoryEntry | nullFileSystemEntry | null (files at the root level have a FileSystemFileEntry, not a FileSystemDirectoryEntry)
  • _getAllEntries: rewritten to extract all refs synchronously first
  • _processRootEntries: new private method for phase 2 — directly testable without mocking DataTransfer
  • _processFileEntry: removed (dead code — replaced by _processRootEntries)

Test plan

Automated tests

4 new unit tests for _processRootEntries using mock FileSystemEntry objects (no DataTransfer mocking needed):

  • Returns all folders when multiple=true
  • Skips folders when multiple=false
  • Returns all accepted files
  • Separates accepted/rejected files by MIME type

Manual — Storybook

The uui-file-dropzone Storybook story (packages/uui-file-dropzone) can be used to verify the fix:

  1. Run npm run storybook and open the uui-file-dropzone story
  2. Create two or more folders on your desktop, each containing at least one file
  3. Select all folders and drag them onto the dropzone simultaneously
  4. Expected (fixed): The change event fires with all folders in event.detail.folders
  5. Expected (broken): Only the first folder appears in event.detail.folders

The story's event log panel shows the fired events and their payloads, making it easy to inspect the result without needing a full CMS.

…fore first await

DataTransferItem.webkitGetAsEntry() returns null after the first await
because the browser expires the drag data store. By collecting all
FileSystemEntry references in a synchronous pass first, all dropped
folders are processed — not just the first one.

Also removes _processFileEntry which used DataTransferItem.getAsFile()
(same staleness issue), replaced by FileSystemFileEntry.file().
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-beach-055ecb503-1339.westeurope.azurestaticapps.net

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a browser-level data staleness bug in uui-file-dropzone where only the first dropped folder was processed when multiple folders were dragged and dropped simultaneously. The root cause is that DataTransfer.items and DataTransferItem.webkitGetAsEntry() become invalid after the first await in an async event handler.

Changes:

  • _getAllEntries is rewritten to extract all FileSystemEntry refs synchronously (before any await) in a Phase 1 pass, then delegates to a new _processRootEntries for async processing.
  • _processRootEntries replaces the removed _processFileEntry, using FileSystemFileEntry.file() instead of the stale DataTransferItem.getAsFile().
  • _getEntry return type broadened from FileSystemDirectoryEntry | nullFileSystemEntry | null.
  • 4 new unit tests for _processRootEntries added, exercising the new method directly with mock FileSystemEntry objects.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/uui-file-dropzone/lib/uui-file-dropzone.element.ts Core fix: refactors _getAllEntries/removes _processFileEntry/adds _processRootEntries; updates _getEntry return type
packages/uui-file-dropzone/lib/uui-file-dropzone.test.ts Adds 4 unit tests for the new _processRootEntries method

Two issues were identified:

  1. Stale internal type in _getEntry (moderate): The internal dir variable is still typed as FileSystemDirectoryEntry | null and the value from webkitGetAsEntry() is still cast as FileSystemDirectoryEntry on line 160, even though the method now correctly returns FileSystemEntry | null. Since webkitGetAsEntry() can return a FileSystemFileEntry for root-level files, the cast is incorrect and should be changed to FileSystemEntry.

  2. Outdated JSDoc on _getEntry (nit): The description still says "Get the directory entry" but the method now returns any filesystem entry (file or directory).

Comments suppressed due to low confidence (1)

packages/uui-file-dropzone/lib/uui-file-dropzone.element.ts:160

  • The internal variable dir in _getEntry is still typed as FileSystemDirectoryEntry | null, even though the method's return type was broadened to FileSystemEntry | null. A file entry dropped at the root level is a FileSystemFileEntry, not a FileSystemDirectoryEntry. The forced cast on line 160 (as FileSystemDirectoryEntry) is now incorrect — it should be cast to FileSystemEntry to match the updated return type. While this doesn't cause a runtime error (because the value is treated as FileSystemEntry in callers), the internal typing is misleading and the wrong cast (as FileSystemDirectoryEntry) remains on the assignment.
    let dir: FileSystemDirectoryEntry | null = null;

    if ('webkitGetAsEntry' in entry) {
      dir = entry.webkitGetAsEntry() as FileSystemDirectoryEntry;

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@iOvergaard iOvergaard added the bug Something isn't working label Mar 9, 2026
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 9, 2026

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-beach-055ecb503-1339.westeurope.azurestaticapps.net

@leekelleher leekelleher self-requested a review March 9, 2026 10:43
@leekelleher leekelleher merged commit d15cac0 into v1/dev Mar 9, 2026
11 of 12 checks passed
@leekelleher leekelleher deleted the v1/bugfix/file-dropzone-datatransfer-staleness branch March 9, 2026 10:48
iOvergaard added a commit to umbraco/Umbraco-CMS that referenced this pull request Mar 9, 2026
Includes the fix for multi-folder drop DataTransfer staleness
(umbraco/Umbraco.UI#1339).
iOvergaard added a commit to umbraco/Umbraco-CMS that referenced this pull request Mar 10, 2026
…r drag-and-drop failing (closes #21837) (#21886)

* fix(media): ensure sequential creation in media drag-and-drop

When multiple folders are dragged into the Media section, the creation
handlers (#handleFile/#handleFolder) were not awaited in the batch loop.
This caused child items to attempt server operations before their parent
folders were fully created, resulting in 404 errors for subsequent items.

Adding await ensures each item is fully created before the next is
processed, which is required because child items in the flat list
reference parent folder IDs that must exist on the server.

Closes #21837

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Task: Bump @umbraco-ui/uui to 1.17.2

Includes the fix for multi-folder drop DataTransfer staleness
(umbraco/Umbraco.UI#1339).

* qa(dropzone): add unit tests for UmbDropzoneManager folder flattening order

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants