Merge pull request #1694 from PR0M3TH3AN/fix-upload-modal-exit-reset-2907666759560590370

Reset upload state on modal exit

js/ui/components/UploadModal.js

@@ -123,6 +123,10 @@ export class UploadModal {
     // Automation
     this.summaryLocked = true;
     this.isUploading = false; // Global lock
+
+    // Upload Session IDs (to guard against zombie callbacks)
+    this.videoUploadId = 0;
+    this.thumbnailUploadId = 0;
   }
 
   // --- Core Lifecycle ---
@@ -472,6 +476,9 @@ export class UploadModal {
           return;
       }
 
+      // Start new session
+      const currentUploadId = ++this.videoUploadId;
+
       // Reset Video State
       this.videoUploadState = {
           status: 'uploading',
@@ -508,6 +515,8 @@ export class UploadModal {
 
           const { settings, bucketEntry } = await service.prepareUpload(npub, { credentials: this.activeCredentials });
 
+          if (this.videoUploadId !== currentUploadId) return;
+
           // 2. Determine Keys
           const videoKey = buildR2Key(npub, file);
           const baseDomain = bucketEntry.publicBaseUrl;
@@ -539,7 +548,9 @@ export class UploadModal {
               forcePathStyle: settings.forcePathStyle,
               createBucketIfMissing: true,
               onProgress: (fraction) => {
-                  this.updateVideoProgress(fraction);
+                  if (this.videoUploadId === currentUploadId) {
+                      this.updateVideoProgress(fraction);
+                  }
               }
           });
 
@@ -552,6 +563,8 @@ export class UploadModal {
 
           const [uploadResult, torrentResult] = await Promise.all([uploadPromise, torrentPromise]);
 
+          if (this.videoUploadId !== currentUploadId) return;
+
           // Video Complete
           this.videoUploadState.status = 'complete';
           this.videoUploadState.url = videoPublicUrl;
@@ -585,6 +598,8 @@ export class UploadModal {
                   createBucketIfMissing: true,
               });
 
+              if (this.videoUploadId !== currentUploadId) return;
+
               this.torrentState.status = 'complete';
               this.torrentState.infoHash = torrentResult.infoHash;
               this.torrentState.url = torrentPublicUrl;
@@ -610,6 +625,8 @@ export class UploadModal {
           }
 
       } catch (err) {
+          if (this.videoUploadId !== currentUploadId) return;
+
           userLogger.error("Video upload sequence failed:", err);
           this.videoUploadState.status = 'error';
           this.updateVideoProgress(null, "Upload failed.");
@@ -628,6 +645,8 @@ export class UploadModal {
           return;
       }
 
+      const currentUploadId = ++this.thumbnailUploadId;
+
       this.thumbnailUploadState = {
           status: 'uploading',
           progress: 0,
@@ -648,6 +667,8 @@ export class UploadModal {
 
           const { settings, bucketEntry } = await service.prepareUpload(npub, { credentials: this.activeCredentials });
 
+          if (this.thumbnailUploadId !== currentUploadId) return;
+
           // Derive Key (randomish or based on file)
           // We don't have the video key here easily if video isn't selected yet.
           // Use a standalone key structure: npub/thumbnails/timestamp-name
@@ -673,9 +694,15 @@ export class UploadModal {
               secretAccessKey: settings.secretAccessKey,
               forcePathStyle: settings.forcePathStyle,
               createBucketIfMissing: true,
-              onProgress: (fraction) => this.updateThumbnailProgress(fraction)
+              onProgress: (fraction) => {
+                  if (this.thumbnailUploadId === currentUploadId) {
+                      this.updateThumbnailProgress(fraction);
+                  }
+              }
           });
 
+          if (this.thumbnailUploadId !== currentUploadId) return;
+
           this.thumbnailUploadState.status = 'complete';
           this.thumbnailUploadState.url = publicUrl;
           this.inputs.thumbnail.value = publicUrl;
@@ -684,10 +711,14 @@ export class UploadModal {
 
           // Hide progress after a delay
           setTimeout(() => {
-              this.sourceSections.thumbnailProgress.classList.add("hidden");
+              if (this.thumbnailUploadId === currentUploadId) {
+                  this.sourceSections.thumbnailProgress.classList.add("hidden");
+              }
           }, 2000);
 
       } catch (err) {
+          if (this.thumbnailUploadId !== currentUploadId) return;
+
           userLogger.error("Thumbnail upload failed:", err);
           this.thumbnailUploadState.status = 'error';
           this.updateThumbnailProgress(null, "Failed.");
@@ -1050,6 +1081,66 @@ export class UploadModal {
       }
   }
 
+  resetUploads() {
+    // Invalidate active uploads
+    this.videoUploadId++;
+    this.thumbnailUploadId++;
+
+    // Reset State
+    this.videoUploadState = {
+      status: 'idle',
+      progress: 0,
+      url: '',
+      key: '',
+      file: null,
+    };
+    this.thumbnailUploadState = {
+      status: 'idle',
+      progress: 0,
+      url: '',
+      key: '',
+      file: null,
+    };
+    this.torrentState = {
+      status: 'idle',
+      infoHash: '',
+      magnet: '',
+      url: '', // xs (torrent file url)
+      file: null,
+    };
+
+    // Reset UI
+    if (this.sourceSections.progress) {
+        this.sourceSections.progress.classList.add("hidden");
+    }
+    if (this.sourceSections.thumbnailProgress) {
+        this.sourceSections.thumbnailProgress.classList.add("hidden");
+    }
+    if (this.sourceSections.results) {
+        this.sourceSections.results.classList.add("hidden");
+    }
+
+    // Clear Result Inputs
+    if (this.results.videoUrl) this.results.videoUrl.value = "";
+    if (this.results.magnet) this.results.magnet.value = "";
+    if (this.results.torrentUrl) this.results.torrentUrl.value = "";
+
+    // Reset Inputs
+    if (this.inputs.file) {
+        this.inputs.file.value = "";
+    }
+    if (this.inputs.thumbnailFile) {
+        this.inputs.thumbnailFile.value = "";
+    }
+    // Re-enable thumbnail input if it was disabled by an upload
+    if (this.inputs.thumbnail) {
+        this.inputs.thumbnail.disabled = false;
+        if (this.inputs.thumbnail.placeholder && this.inputs.thumbnail.placeholder.startsWith("Selected:")) {
+             this.inputs.thumbnail.placeholder = "https://example.com/thumbnail.jpg";
+        }
+    }
+  }
+
   resetForm() {
       this.form.reset();
       this.nip71FormManager.resetSection("main");
@@ -1059,23 +1150,7 @@ export class UploadModal {
       this.toggles.nsfw.checked = false;
       this.toggles.kids.checked = false;
 
-      // Reset State
-      this.videoUploadState = { status: 'idle', progress: 0, url: '', key: '', file: null };
-      this.thumbnailUploadState = { status: 'idle', progress: 0, url: '', key: '', file: null };
-      this.torrentState = { status: 'idle', infoHash: '', magnet: '', url: '', file: null };
-
-      this.sourceSections.progress.classList.add("hidden");
-      this.sourceSections.thumbnailProgress.classList.add("hidden");
-      this.sourceSections.results.classList.add("hidden");
-
-      // Reset thumbnail UI
-      if (this.inputs.thumbnail) {
-          this.inputs.thumbnail.disabled = false;
-          this.inputs.thumbnail.placeholder = "https://example.com/thumbnail.jpg";
-      }
-      if (this.inputs.thumbnailFile) {
-          this.inputs.thumbnailFile.value = "";
-      }
+      this.resetUploads();
   }
 
   // --- Modal Control ---
@@ -1102,6 +1177,7 @@ export class UploadModal {
 
   close() {
     if (!this.root) return;
+    this.resetUploads();
     this.root.classList.add("hidden");
     this.setGlobalModalState("upload", false);
     this.isVisible = false;

tests/ui/uploadModal-reset.test.mjs

@@ -0,0 +1,224 @@
+import { test, describe, it, before, after } from "node:test";
+import assert from "node:assert";
+import { createUiDom } from "./helpers/jsdom-test-helpers.mjs";
+
+describe("UploadModal Reset Logic", () => {
+  let dom;
+  let UploadModal;
+
+  // Mock StorageService
+  const mockStorageService = {
+      isUnlocked: () => true,
+      unlock: async () => {},
+      listConnections: async () => [],
+      getConnection: async () => null,
+      saveConnection: async () => {},
+      isMock: true
+  };
+
+  // Mock R2Service
+  const mockR2Service = {
+      prepareUpload: async () => ({
+          settings: {},
+          bucketEntry: { bucket: 'test', publicBaseUrl: 'https://test.com' }
+      }),
+      uploadFile: async () => {
+          // Default immediate resolution
+          return {};
+      }
+  };
+
+  before(async () => {
+    dom = createUiDom();
+    global.window = dom.window;
+    global.document = dom.document;
+    global.EventTarget = dom.window.EventTarget;
+    global.alert = (msg) => { console.log("ALERT CALLED:", msg); };
+    global.HTMLElement = dom.window.HTMLElement;
+    global.HTMLInputElement = dom.window.HTMLInputElement;
+    global.HTMLButtonElement = dom.window.HTMLButtonElement;
+    global.HTMLDivElement = dom.window.HTMLDivElement;
+
+    const module = await import("../../js/ui/components/UploadModal.js");
+    UploadModal = module.UploadModal;
+  });
+
+  after(() => {
+    if (dom) dom.cleanup();
+  });
+
+  const mockHtml = `
+      <div id="uploadModal" class="hidden">
+        <form id="unifiedUploadForm">
+          <button id="btn-mode-upload"></button>
+          <button id="btn-mode-external"></button>
+          <div id="section-source-upload">
+             <button id="btn-storage-settings"></button>
+             <div id="section-storage-settings" class="hidden">
+                <div id="storage-summary-view" class="hidden"></div>
+                <div id="storage-empty-view" class="hidden"></div>
+             </div>
+             <div id="upload-status-text"></div>
+             <div id="upload-percent-text"></div>
+             <div id="upload-progress-container"></div>
+             <div id="thumbnail-progress-container"></div>
+             <div id="upload-results-container"></div>
+             <progress id="input-progress"></progress>
+             <progress id="input-thumbnail-progress"></progress>
+          </div>
+          <div id="section-source-external"></div>
+
+          <input id="input-title" />
+          <textarea id="input-description"></textarea>
+          <input id="input-thumbnail" />
+          <input id="input-thumbnail-file" type="file" />
+          <input id="input-file" type="file" />
+          <input id="input-url" />
+          <input id="input-magnet" />
+
+          <input id="result-video-url" />
+          <input id="result-magnet" />
+          <input id="result-torrent-url" />
+
+          <input id="check-nsfw" type="checkbox" />
+          <input id="check-kids" type="checkbox" />
+          <input id="check-comments" type="checkbox" />
+          <input id="check-summary-unlock" type="checkbox" />
+
+          <button id="btn-advanced-toggle"></button>
+          <button id="btn-thumbnail-file"></button>
+          <button id="btn-storage-unlock"></button>
+          <button id="btn-manage-storage"></button>
+          <button id="btn-configure-storage"></button>
+
+          <div id="section-advanced"></div>
+
+          <button id="btn-submit"></button>
+          <div id="submit-status"></div>
+        </form>
+        <button id="closeUploadModal"></button>
+      </div>
+  `;
+
+  it("should reset upload state and inputs when resetUploads is called", async () => {
+      const container = document.createElement("div");
+      container.id = "modalContainer";
+      document.body.appendChild(container);
+
+      global.fetch = async () => ({ ok: true, text: async () => mockHtml });
+
+      const modal = new UploadModal({
+          storageService: mockStorageService,
+          r2Service: mockR2Service,
+          getCurrentPubkey: () => "pk1",
+          removeTrackingScripts: () => {},
+          setGlobalModalState: () => {},
+      });
+
+      await modal.load({ container });
+
+      // Simulate modified state
+      modal.videoUploadState.status = 'complete';
+      modal.inputs.file = { value: 'fake-path' }; // Mock input value
+      modal.inputs.thumbnail.disabled = true;
+      modal.inputs.thumbnail.placeholder = "Selected: file.jpg";
+
+      const initialVideoId = modal.videoUploadId;
+
+      modal.resetUploads();
+
+      // Assertions
+      assert.strictEqual(modal.videoUploadState.status, 'idle', "State should be idle");
+      assert.strictEqual(modal.inputs.file.value, "", "File input should be cleared");
+      assert.strictEqual(modal.inputs.thumbnail.disabled, false, "Thumbnail input should be enabled");
+      assert.strictEqual(modal.inputs.thumbnail.placeholder, "https://example.com/thumbnail.jpg", "Thumbnail placeholder should be reset");
+      assert.strictEqual(modal.videoUploadId, initialVideoId + 1, "Video upload ID should be incremented");
+  });
+
+  it("should guard against zombie callbacks when upload is reset during process", async () => {
+      const container = document.createElement("div");
+      container.id = "modalContainer2";
+      document.body.appendChild(container);
+
+      let resolveUpload;
+      const delayedUpload = new Promise(r => { resolveUpload = r; });
+
+      const slowR2Service = {
+          prepareUpload: async () => ({
+               settings: {},
+               bucketEntry: { bucket: 'test', publicBaseUrl: 'https://test.com' }
+          }),
+          uploadFile: () => delayedUpload // Return promise that waits
+      };
+
+      const modal = new UploadModal({
+          storageService: mockStorageService,
+          r2Service: slowR2Service,
+          getCurrentPubkey: () => "pk1",
+          removeTrackingScripts: () => {},
+          setGlobalModalState: () => {},
+      });
+
+      await modal.load({ container });
+
+      // Force configured state
+      modal.storageConfigured = true;
+      modal.isStorageUnlocked = true;
+      modal.activeProvider = "cloudflare_r2";
+      modal.activeCredentials = { accountId: 'acc', accessKeyId: 'key', secretAccessKey: 'sec' };
+
+      // Start upload
+      const file = { name: "video.mp4" };
+      const event = { target: { files: [file], value: "video.mp4" } };
+
+      // Start the async process without awaiting it yet
+      const uploadPromise = modal.handleVideoSelection(event);
+
+      // Verify it started
+      assert.strictEqual(modal.videoUploadState.status, 'uploading');
+
+      // RESET immediately
+      modal.resetUploads();
+
+      // Verify it reset
+      assert.strictEqual(modal.videoUploadState.status, 'idle');
+
+      // Now let the upload promise resolve
+      resolveUpload({ bucket: 'b', key: 'k' });
+
+      // Also need to handle generateTorrentMetadata which might be awaited
+      // Assuming it resolves quickly or we mock it too.
+      // UploadModal mocks `generateTorrentMetadata` internally? No, it's a method on instance or imported.
+      // It calls `this.generateTorrentMetadata`. We can spy/mock it.
+      modal.generateTorrentMetadata = async () => ({ hasValidInfoHash: false });
+
+      await uploadPromise;
+
+      // Assert that state remains 'idle' and wasn't overwritten by 'complete'
+      assert.strictEqual(modal.videoUploadState.status, 'idle', "State should remain idle after zombie completion");
+      assert.strictEqual(modal.results.videoUrl.value, "", "Result URL should be empty");
+  });
+
+  it("should call resetUploads when close is called", async () => {
+      const container = document.createElement("div");
+      container.id = "modalContainer3";
+      document.body.appendChild(container);
+
+      const modal = new UploadModal({
+          storageService: mockStorageService,
+          r2Service: mockR2Service,
+          setGlobalModalState: () => {},
+      });
+      await modal.load({ container });
+
+      let resetCalled = false;
+      const originalReset = modal.resetUploads.bind(modal);
+      modal.resetUploads = () => {
+          resetCalled = true;
+          originalReset();
+      };
+
+      modal.close();
+      assert.strictEqual(resetCalled, true, "resetUploads should be called on close");
+  });
+});