Drag and drop improvements

[AllTerrainDeveloper]

Now everything (almost everything) is drag and drop enabled. Including iframes as destinations.

[AllTerrainDeveloper]

@claude can you review these changes?

[claude]

Claude finished @AllTerrainDeveloper's task in 3m 19s —— View job

---

PR Review: Drag and Drop Improvements

• Gather context and read changed files
• Review new files (iframe-drop-targets.ts, gutenberg-drop-receiver.ts)
• Review modified core files (drag-bridge.ts, desktop.ts, public-api.ts)
• Review PHP changes and CSS
• Review tests
• Review docs
• Post final review

---

Good overall design: the pointer-events suppression approach is well-motivated, the tests cover the pure buildBlockSpec function well, and the docs are thorough. A few issues to address before merge:

Issues

1. console.log in production code will fail lint

src/drag/iframe-drop-targets.ts, lines 192–198

The ESLint config sets no-console: ['error', { allow: ['warn', 'error', 'info'] }] — console.log is not in the allowlist. The // eslint-disable-next-line no-console suppresses the check, but the intent was clearly a diagnostic aid. Use console.info instead — it satisfies the rule without needing a disable comment, and it's more semantically accurate for a status message rather than an error or warning.

// Before (requires eslint-disable):
// eslint-disable-next-line no-console
console.log( '[desktop-mode] drag-start: suppressing %d iframe(s); ...', ... );

// After (no disable needed, matches the allowlist):
console.info( '[desktop-mode] drag-start: suppressing %d iframe(s); ...', ... );

Fix this →

2. Mid-file imports in layer.ts violate import ordering

src/desktop-files/layer.ts, lines 59–103

The function buildBridgePayloadFromPlacement is inserted between two groups of import statements: the new import type { DragBridgePayload } lands at line 45, the function body occupies lines 59–103, and then imports resume at lines 104–110 (isConflict, DragManagerApi, trashFolderWithUndo, etc.). ES modules hoist import declarations, so runtime behaviour is fine, but @wordpress/eslint-plugin's import/first rule (inherited from the recommended-with-formatting config) will flag any import that appears after non-import statements.

Move the function down past the last import block, or move all three "post-function" imports (conflict-toast, drag, trash, drag-payloads) up before the function.

Fix this →

3. javascript: URL not blocked in escapeHtml

src/gutenberg-drop-receiver.ts, lines 91–98 and 159

escapeHtml correctly encodes the five HTML special chars, but javascript:alert(1) passes through untouched and would be inserted verbatim as an href attribute in the core/paragraph block content. The same-origin postMessage check is the first line of defence, but defence-in-depth here is cheap:

function isSafeUrl( s: string ): boolean {
    const lower = s.trimStart().toLowerCase();
    return ! lower.startsWith( 'javascript:' ) && ! lower.startsWith( 'data:' );
}

Then in buildBlockSpec:

if ( ! payload.url || ! isSafeUrl( payload.url ) ) {
    return null;
}

Fix this →

4. site-editor.php drops may silently fail

includes/render/assets.php, line 52 and src/gutenberg-drop-receiver.ts, line 184

The receiver is now enqueued on site-editor.php (the full-site editor), but wp.data.dispatch('core/block-editor') in the FSE context is only available once a template/template-part is opened for editing in the canvas iframe — not immediately on page load. The waitForEditor() poll may time out (reject after ~5s) and the drop is silently lost with only a console.error. This will be confusing for anyone who tries dropping onto a template window.

Either gate the site-editor.php enqueue behind a known-working path, or extend waitForEditor to surface a user-visible message (a toast via window.parent.wp.desktop.toasts or similar) when the timeout is hit rather than failing silently.

5. Minor: Map mutation during forEach is non-idiomatic

src/drag/iframe-drop-targets.ts, lines 289–303 (WINDOW_CLOSED handler)

The code calls _suppressedIframes.delete(iframe) and _activeRegistrations.delete(iframe) inside a Map.prototype.forEach loop. This is spec-safe (deleted entries that haven't been visited yet are simply skipped), but it's non-idiomatic and surprises readers. A cleaner pattern:

for ( const [ iframe ] of Array.from( _suppressedIframes ) ) {
    if ( ! iframe.isConnected ) {
        _suppressedIframes.delete( iframe );
    }
}

Positive observations

• The pointer-events suppression approach is the right design. The comment in the file header explaining why the previous overlay approach failed is excellent context.
• buildBlockSpec is a pure function and its test suite is thorough — all five MIME categories, XSS vectors (both & in URL and <script> in title), empty-URL guard, empty-title fallback.
• The DragBridgePayload type duplication in gutenberg-drop-receiver.ts is correctly motivated (standalone bundle, no shell deps). Good comment explaining it.
• The PHP additions (sourceUrl, alt, link) are backwards-compatible and correctly avoid a REST roundtrip at drop time.
• Cache-busting windows.css with an mtime stamp is a smart move. The CSS comment explaining why drop-highlight rules must live in the parent file and not in an @import'd sub-sheet is valuable.
• Origin check on every postMessage path (e.origin !== expectedOrigin) is correct.
• Debug helper window.__desktopModeIframeDropDebug with a typed cast is a nice touch for diagnostics.
• Doc additions to bridge-protocol.md and javascript-reference.md are complete and follow the existing table format.

[github-actions]

✅ 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