From 3c210df8a44a3f3494dc006170ad5fee565b4ba8 Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Mon, 7 Sep 2026 00:11:54 +0200 Subject: [PATCH] fix: magazine dropzone opened the file in the browser instead of accepting the drop Two real bugs, both confirmed live: 1. dragover's preventDefault() was gated on dataTransfer.types including 'Files' -- during a real OS file drag, Chromium does not reliably populate that list synchronously on every dragover event, so the condition failed often enough that preventDefault was silently skipped. Per the HTML5 DnD spec, an element only becomes a valid drop target once dragover (and, in some browsers, dragenter) has been cancelled -- without that, the browser falls through to its native "open the dropped file" behaviour, which is exactly what was reported. Removed the gate entirely; dragenter is now also handled, not just dragover. 2. No page-wide safety net existed, so a drop landing even slightly outside the dropzone element bubbled to the browser's default action uncontested. Added a window-level dragover/drop preventDefault installed once regardless of which section is active. This also explains the "no hover highlight" symptom: the same gated-preventDefault bug meant dragover never reliably reached the class-toggle code either. Verified live: dragenter/dragover/drop each report defaultPrevented, the highlight class appears on dragenter and clears on drop, the dropped file populates the hidden input, and a drop dispatched directly on (simulating one that misses the zone) is also intercepted by the new window-level guard. Co-Authored-By: Claude Sonnet 5 --- static/js/magazine.js | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/static/js/magazine.js b/static/js/magazine.js index 2fca30f..f54d087 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -114,23 +114,44 @@ function magShowDroppedFile(file) { if (noteEl) noteEl.textContent = file ? file.name : MAG_DROPZONE_DEFAULT_NOTE; } +let _magDropGuardInstalled = false; + function setupMagDropzone() { const zone = $('mag-dropzone'); const input = $('mag-new-file'); - if (!zone || !input || zone.dataset.magDropzoneWired) return; + if (!zone || !input) return; + + // A page-wide safety net: if a drop ever lands outside the zone (or the + // zone's own handler below fails to intercept it for any reason), the + // browser's native behaviour is to navigate to/open the dropped file -- + // confirmed live as exactly what happened before this existed. Installed + // once regardless of which section is active, since a stray drop can land + // anywhere on the page. + if (!_magDropGuardInstalled) { + _magDropGuardInstalled = true; + window.addEventListener('dragover', (e) => { e.preventDefault(); }); + window.addEventListener('drop', (e) => { e.preventDefault(); }); + } + + if (zone.dataset.magDropzoneWired) return; zone.dataset.magDropzoneWired = '1'; - zone.addEventListener('dragover', (e) => { - if (e.dataTransfer && Array.from(e.dataTransfer.types || []).includes('Files')) { - e.preventDefault(); - zone.classList.add('dragover'); - } - }); + + // dragenter AND dragover both need preventDefault() -- some browsers only + // arm the element as a valid drop target once dragenter itself has been + // cancelled, not just the dragover events that follow it. Not gated on + // dataTransfer.types: during a real OS file drag, Chromium does not + // always populate that list synchronously on every one of these events, + // and gating on it here (confirmed live) silently skipped preventDefault + // often enough that neither the hover highlight nor the drop interception + // ever fired -- the browser fell through to opening the file itself. + const armDropTarget = (e) => { e.preventDefault(); zone.classList.add('dragover'); }; + zone.addEventListener('dragenter', armDropTarget); + zone.addEventListener('dragover', armDropTarget); zone.addEventListener('dragleave', () => zone.classList.remove('dragover')); zone.addEventListener('drop', (e) => { - if (!(e.dataTransfer && Array.from(e.dataTransfer.types || []).includes('Files'))) return; e.preventDefault(); zone.classList.remove('dragover'); - const file = e.dataTransfer.files?.[0]; + const file = e.dataTransfer?.files?.[0]; if (!file) return; if (file.type !== 'application/pdf' && !/\.pdf$/i.test(file.name)) { return say('Only PDF files are supported', 'err');