✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

@claude what about a review here? those are many fixes for drag and drop (it was not working dragging from media to Gutenberg, and from OS to Gutenberg).

Then another refactor happened, as we were mixing owner_id and user_id columns.

And Lastly we were not able to drag and drop anything from My WordPress to the trash. This PR tries to address it all.

Claude finished @AllTerrainDeveloper's task in 4m 36s —— View job

PR Review: Many drag and drop fixes

Reviewed PR #270 — 22 files, three independent streams: Gutenberg DnD, owner_id rename, My WordPress → Trash.

Todo:

• Post initial comment
• Read diff and changed source files
• Review drag/drop Gutenberg changes (src/gutenberg-drop-receiver.ts)
• Review My WordPress changes (src/my-wordpress/index.ts)
• Review drag infrastructure changes (src/drag/)
• Review PHP/schema changes (includes/desktop-files/)
• Review cascade cleanup (includes/desktop-files/cascade-cleanup.php)
• Review test coverage
• Post final review

Stream 1 — Gutenberg drag and drop (OS files + Media Library list view)

src/gutenberg-drop-receiver.ts

The core fix (stash the bridge payload + native HTML5 backstop for when Chromium strips the custom MIME at the iframe boundary) already existed. This PR adds two meaningful improvements:

1. dragCarriesOsFiles() guard — correct fix. When the user drags a real OS file onto Gutenberg's canvas, the native backstop was claiming the dragover/drop before Gutenberg's own dropzone could handle upload + insert. The guard returns early for Files-typed drags, letting Gutenberg's native handler take over. The cross-browser detection (DOMStringList.contains vs Array.includes) is correct for the APIs in play.

2. MutationObserver optimization — the previous version called attachToAllFrames() on every DOM mutation (Gutenberg is extremely noisy). The new version filters to mutations that add an HTMLIFrameElement or an element containing iframes. This avoids walking every existing iframe on every keystroke. One subtle note: node.querySelector?.('iframe') is checked on Element nodes but not on HTMLIFrameElement itself (which has a separate fast-path instanceof check above it). This is fine — the two arms are not mutually exclusive and no drop is missed.

3. Removal of _debugCounters and __desktopModeDropReceiverDebug — appropriate for production code. The debug helper was useful in development but has no place shipping.

assets/js/media-library-enhanced.js

This file is documented in its own header as "a vanilla IIFE with no build step" — a deliberate exception to the TypeScript build pipeline that predates v0.14.0 and is separate from the src/** tree. The list-view addition is correct:

• Event delegation on document.body (capture phase) rather than per-row listeners is the right pattern here — it avoids the <a>/<img> default-draggable races with draggable=false at runtime.
• The wp.media availability guard is handled correctly: the list table is server-rendered so wp.media is absent; the DOM fallbacks (rowImg.src, rowAnchor.href) produce a usable payload without it.
• LIST_DELEGATION_INSTALLED sentinel makes the call from start() + the MutationObserver walk idempotent.
• enhanceListRow() is a no-op shim so the MutationObserver walk's forEach(enhanceListRow) call is safe even though the delegation is document-level, not per-row. Clean.

Minor note (not blocking): assets/js/media-library-enhanced.js pre-dates the TS-only rule. A future cleanup could migrate it to src/ with a proper Vite entry. Out of scope for this PR.

Stream 2 — user_id → owner_id rename

includes/desktop-files/schema.php — desktop_mode_files_rename_user_id_to_owner_id()

The migration function is well-designed:

• INFORMATION_SCHEMA guards for table existence + column existence make it idempotent.
• Running BEFORE dbDelta() prevents dbDelta's "missing column → ADD it" false-positive.
• Schema version bump to 12 ensures a re-delivery for any install that stamped v11 but ran an earlier draft that silently failed.
• The deliberate choice NOT to suppress errors on the CHANGE COLUMN is correct reasoning (the prior suppression hid a real failure).

One compatibility concern worth noting:

RENAME INDEX (line ~502) requires MySQL 5.7.4+ or MariaDB 10.5.2+. WordPress's stated minimum is MySQL 5.0+ / MariaDB 10.0+, so the query can fail on older installations. When it fails:

• The index is still functionally correct (CHANGE COLUMN auto-updates the index column reference, the optimizer uses it regardless of its name).
• Without error suppression, the failure is logged to debug.log on WP_DEBUG_LOG installs, which will confuse site owners.
• No data corruption — purely cosmetic.

Suggestion: either suppress errors on the RENAME INDEX specifically (with a comment explaining why, distinct from the CHANGE COLUMN), or use the universally-supported DROP INDEX + ADD INDEX sequence. Fix this →

The rest of the user_id → owner_id changes across store.php, trash.php, heartbeat.php, rest.php, shares-store.php are mechanical and correct — no instances of the old column name were missed.

Stream 3 — My WordPress → Trash drag and drop

src/drag/types.ts / ghost.ts / manager.ts — acceptLabel on DropTarget

Clean, backwards-compatible extension. The optional acceptLabel field threads from DropTarget → DragManager.update() → GhostHandle.setMode(). Existing drop targets that don't set it fall through to labels.accept as before. The "Move to Trash" chip text is a UX win.

src/desktop-files/drag-payloads.ts / tile-spec.ts — entityId propagation

The entityId field is optional and correctly forwarded from TileDragOutPayload → ShortcutDragData via attachTileDragOut. The comment explaining why kind: 'post' alone is ambiguous (posts and pages both carry it) is correct and useful.

src/desktop-files/recycle-bin-targets.ts

The accept refactor from "only desktop-file" to "also shortcut with entityId" is clean. isTrashableShortcut() correctly guards against:

• Missing entityId
• Unknown kind (only 'post' is wired for now, with clear comment about why 'user' and 'media' are deferred)
• Non-numeric ref
• Absent wp.desktop.myWordpress.trashEntity API

The onDrop handler re-checks the API and ref validity before calling. The error is caught and logged — non-blocking failure is the right behavior (tile snaps back visually, user gets a console message they can report).

src/my-wordpress/index.ts — trashEntityById + event-driven tile pruning

Well-designed. The public wp.desktop.myWordpress.trashEntity() API lets the cross-bundle recycle-bin target trash entities without depending on this bundle's internals, which follows the framework's event-driven pattern.

One minor UX gap: confirmTrash (the right-click path) handles deselection — it checks ctx.selectedId === id and clears the preview pane if the trashed item was selected. trashEntityById (the drag-to-trash path) only removes the tile via the event listener, not the selected-item preview. If the user selects an item (loading its preview), then drags it to the trash, the preview panel will still show the now-deleted entity. Low priority in the macOS drag pattern (the tile disappears, the preview stale-show is temporary until the next click), but worth a follow-up. Fix this →

includes/desktop-files/cascade-cleanup.php + tests

This is a solid piece of infrastructure. The cascade-trash-on-entity-trash hooks cover all four deletion paths correctly:

The wp_trash_post handler correctly detects post_type === 'attachment' and redirects to file_type='attachment' so the wrong file-type isn't searched. The desktop_mode_files_cascade_trash_placements_for_entity() function is idempotent, bulk-friendly, and fires a per-placement action for plugins.

Tests cover: soft-trash, idempotency, file-type isolation, all four hook paths, action firing, cascade marker in trashed_meta. That's comprehensive for a new file.

One small note on trashed_by in the cascade: it's set to $owner_id (the placement owner) rather than the acting user, since cascade-trashing is a side-effect of an entity-level authorized action, not a direct user gesture. This is correct and well-documented in the function's docblock.

Overall assessment

The three streams are logically independent and well-separated. The code quality is high:

• No security issues found
• All SQL uses $wpdb->prepare()
• Error paths are handled defensively without silent no-ops
• The cascade-cleanup design follows the framework's hook patterns

Two items worth a follow-up before merge:

1. RENAME INDEX MySQL/MariaDB compatibility (cosmetic/logging risk, not data corruption)
2. Preview-pane deselection on drag-to-trash (minor UX gap)

Neither is a blocker but the RENAME INDEX one could produce confusing error log entries for some users.