Merge branch 'main' of https://github.com/bulwarkmail/webmail

components/email/email-composer.tsx

@@ -18,6 +18,7 @@ import { isEditableEventTarget } from "@/lib/keyboard";
 import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html";
 import { buildSignatureBlock, containsEmbeddedSignature, SIGNATURE_RANGE_MARKER } from "@/components/email/signature-block";
 import { emailHooks, contactHooks, isExternalAttachmentResult } from "@/lib/plugin-hooks";
+import { onUploadProgress } from "@/lib/upload-progress";
 import type { AlmostSavedDraft, OutgoingEmail, RecipientSuggestion } from "@/lib/plugin-types";
 import { useAuthStore } from "@/stores/auth-store";
 import { useIdentityStore } from "@/stores/identity-store";
@@ -217,6 +218,14 @@ type ComposerAttachment = {
   size: number;
   blobId?: string;
   uploading?: boolean;
+  /**
+   * Transferred share of the upload, 0-100. Set while `uploading` when the
+   * transport reports byte progress: the stock path via the JMAP client's
+   * `onProgress`, a plugin offload via the staged-file-id registry
+   * (`lib/upload-progress.ts`). Absent when nothing has reported yet, in
+   * which case the chip falls back to its indeterminate bar.
+   */
+  progress?: number;
   error?: boolean;
   abortController?: AbortController;
   // Carried through for parts hydrated from an existing draft so inline
@@ -1387,60 +1396,82 @@ export function EmailComposer({
         
         const fileId = generateUUID();
         await fileStorage.saveFile(fileId, file);
-        
-        const transformed = await emailHooks.onBeforeBlobUpload.transform<unknown>(fileId);
 
-        // A handler can offload the file elsewhere and hand back replacement
-        // content instead of a file id. Drop the binary attachment and put the
-        // replacement in the body.
-        if (isExternalAttachmentResult(transformed)) {
-          // `transformed.fileId` when the handler re-saved the staged file
-          // under a new id; otherwise the id we handed it.
-          await fileStorage.deleteFile(transformed.fileId ?? fileId);
-          // The user may have removed the attachment while the handler was
-          // offloading it - don't drop a link for a file they cancelled.
-          if (controller?.signal.aborted) continue;
-          setAttachments(prev => prev.filter(att => att.file !== file));
-          if (plainTextMode) {
-            setBody(previous =>
-              previous && !previous.endsWith('\n') ? `${previous}\n${transformed.text}` : previous + transformed.text
-            );
-          } else {
-            // Never trust plugin markup in the document (or in the message the
-            // user then sends); insert through the editor so it lands at the
-            // caret rather than after the signature and quoted block.
-            const html = sanitizePluginBodyHtml(transformed.html);
-            if (!html) continue;
-            if (editorRef.current) {
-              editorRef.current.chain().focus().insertContent(html).run();
+        // Byte progress for the chip, from whichever transport moves the
+        // bytes: the JMAP client below reports directly, a plugin offloading
+        // via `api.http.post` reports through the staged-file-id registry
+        // (it echoes `fileId` as `progressFileId`, see doHttpPost).
+        const reportProgress = (loaded: number, total: number) => {
+          if (total <= 0 || controller?.signal.aborted) return;
+          const pct = Math.min(100, Math.floor((loaded / total) * 100));
+          setAttachments(prev =>
+            prev.map(att => (att.file === file ? { ...att, progress: pct } : att))
+          );
+        };
+        const stopProgress = onUploadProgress(fileId, reportProgress);
+
+        try {
+          const transformed = await emailHooks.onBeforeBlobUpload.transform<unknown>(fileId);
+
+          // A handler can offload the file elsewhere and hand back replacement
+          // content instead of a file id. Drop the binary attachment and put the
+          // replacement in the body.
+          if (isExternalAttachmentResult(transformed)) {
+            // `transformed.fileId` when the handler re-saved the staged file
+            // under a new id; otherwise the id we handed it.
+            await fileStorage.deleteFile(transformed.fileId ?? fileId);
+            // The user may have removed the attachment while the handler was
+            // offloading it - don't drop a link for a file they cancelled.
+            if (controller?.signal.aborted) continue;
+            setAttachments(prev => prev.filter(att => att.file !== file));
+            if (plainTextMode) {
+              setBody(previous =>
+                previous && !previous.endsWith('\n') ? `${previous}\n${transformed.text}` : previous + transformed.text
+              );
             } else {
-              setBody(previous => previous + html);
+              // Never trust plugin markup in the document (or in the message the
+              // user then sends); insert through the editor so it lands at the
+              // caret rather than after the signature and quoted block.
+              const html = sanitizePluginBodyHtml(transformed.html);
+              if (!html) continue;
+              if (editorRef.current) {
+                editorRef.current.chain().focus().insertContent(html).run();
+              } else {
+                setBody(previous => previous + html);
+              }
             }
+            continue;
           }
-          continue;
+
+          const newFileId = typeof transformed === 'string' ? transformed : fileId;
+
+          const newFile = await fileStorage.getFile(newFileId) || file;
+          await fileStorage.deleteFile(newFileId);
+
+          // Passing the signal also makes cancel abort the transfer itself,
+          // instead of only being checked once the upload has finished.
+          const { blobId } = await client.uploadBlob(newFile, {
+            onProgress: reportProgress,
+            signal: controller?.signal,
+          });
+
+          if (controller?.signal.aborted) continue;
+          setAttachments(prev =>
+            prev.map(att =>
+              att.file === file
+                ? { ...att, blobId, uploading: false, abortController: undefined }
+                : att
+            )
+          );
+          emailHooks.onAfterAttachmentUpload.emit({
+            name: file.name,
+            type: file.type || 'application/octet-stream',
+            size: file.size,
+            blobId,
+          });
+        } finally {
+          stopProgress();
         }
-
-        const newFileId = typeof transformed === 'string' ? transformed : fileId;
-
-        const newFile = await fileStorage.getFile(newFileId) || file;
-        await fileStorage.deleteFile(newFileId);
-
-        const { blobId } = await client.uploadBlob(newFile);
-
-        if (controller?.signal.aborted) continue;
-        setAttachments(prev =>
-          prev.map(att =>
-            att.file === file
-              ? { ...att, blobId, uploading: false, abortController: undefined }
-              : att
-          )
-        );
-        emailHooks.onAfterAttachmentUpload.emit({
-          name: file.name,
-          type: file.type || 'application/octet-stream',
-          size: file.size,
-          blobId,
-        });
       } catch (error) {
         if (controller?.signal.aborted) continue;
         debug.error(`Failed to upload ${file.name}:`, error);
@@ -2891,7 +2922,16 @@ export function EmailComposer({
                   {att.uploading && (
                     <div className="absolute inset-0 pointer-events-none">
                       <div className="h-full bg-primary/10 animate-pulse" />
-                      <div className="absolute bottom-0 left-0 h-0.5 bg-primary/40 animate-[indeterminate_1.5s_ease-in-out_infinite]" style={{ width: '40%' }} />
+                      {typeof att.progress === 'number' ? (
+                        // Real byte progress from the transport; see
+                        // ComposerAttachment.progress for who reports it.
+                        <div
+                          className="absolute bottom-0 left-0 h-0.5 bg-primary/60 transition-[width] duration-300 ease-out"
+                          style={{ width: `${att.progress}%` }}
+                        />
+                      ) : (
+                        <div className="absolute bottom-0 left-0 h-0.5 bg-primary/40 animate-[indeterminate_1.5s_ease-in-out_infinite]" style={{ width: '40%' }} />
+                      )}
                     </div>
                   )}
                   <div className="relative flex items-center gap-2">

lib/__tests__/plugin-upload-progress.test.ts

@@ -0,0 +1,154 @@
+import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest';
+import type { InstalledPlugin } from '../plugin-types';
+import { useAuthStore } from '@/stores/auth-store';
+import { onUploadProgress } from '../upload-progress';
+
+vi.mock('@/stores/toast-store', () => ({
+  toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() },
+}));
+
+import { dispatchApiCall } from '../plugin-sandbox/host-api';
+
+function plugin(overrides: Partial<InstalledPlugin> = {}): InstalledPlugin {
+  return {
+    id: 'p1',
+    name: 'Test plugin',
+    version: '1.0.0',
+    author: 'test',
+    description: '',
+    type: 'hook',
+    permissions: ['http:post'],
+    grantedPermissions: ['http:post'],
+    entrypoint: 'index.js',
+    enabled: true,
+    status: 'running',
+    settings: {},
+    apiPostPaths: ['/api/upload'],
+    ...overrides,
+  };
+}
+
+/**
+ * Stand-in for XMLHttpRequest that records the request and lets the test
+ * drive upload progress and completion by hand.
+ */
+class FakeXHR {
+  static instances: FakeXHR[] = [];
+  method = '';
+  url = '';
+  headers: Record<string, string> = {};
+  sentBody: unknown = null;
+  status = 200;
+  responseText = '{"url":"https://example.test/d/1"}';
+  upload: { onprogress: ((e: { lengthComputable: boolean; loaded: number; total: number }) => void) | null } = { onprogress: null };
+  onload: (() => void) | null = null;
+  onerror: (() => void) | null = null;
+  ontimeout: (() => void) | null = null;
+  open(method: string, url: string) { this.method = method; this.url = url; }
+  setRequestHeader(name: string, value: string) { this.headers[name] = value; }
+  send(body: unknown) { this.sentBody = body; FakeXHR.instances.push(this); }
+}
+
+describe('plugin binary upload progress', () => {
+  beforeEach(() => {
+    FakeXHR.instances = [];
+    vi.stubGlobal('XMLHttpRequest', FakeXHR);
+    useAuthStore.setState({
+      client: {
+        getAuthHeader: () => 'Basic dGVzdA==',
+        getUsername: () => 'tester',
+      } as never,
+    });
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    useAuthStore.setState({ client: null });
+  });
+
+  it('reports byte progress into the registry keyed by progressFileId', async () => {
+    const seen: Array<[number, number]> = [];
+    const off = onUploadProgress('staged-1', (loaded, total) => seen.push([loaded, total]));
+
+    const call = dispatchApiCall(plugin(), 'http.post', [
+      '/api/upload',
+      new Blob(['0123456789']),
+      { progressFileId: 'staged-1' },
+    ]);
+
+    // send() is synchronous inside the promise executor, so the request is
+    // already captured even though the call promise is still pending.
+    expect(FakeXHR.instances).toHaveLength(1);
+    const xhr = FakeXHR.instances[0];
+    xhr.upload.onprogress?.({ lengthComputable: true, loaded: 4, total: 10 });
+    xhr.upload.onprogress?.({ lengthComputable: true, loaded: 10, total: 10 });
+    xhr.onload?.();
+
+    const result = await call as { ok: boolean; status: number; data: unknown };
+    expect(seen).toEqual([[4, 10], [10, 10]]);
+    expect(result.ok).toBe(true);
+    expect(result.data).toEqual({ url: 'https://example.test/d/1' });
+    off();
+  });
+
+  it('still applies credential headers after plugin headers on the XHR path', async () => {
+    const call = dispatchApiCall(plugin(), 'http.post', [
+      '/api/upload',
+      new Blob(['x'], { type: 'application/octet-stream' }),
+      { headers: { 'X-Plugin-Name': 'a.bin' }, progressFileId: 'staged-2' },
+    ]);
+    const xhr = FakeXHR.instances[0];
+    expect(xhr.headers['Authorization']).toBe('Basic dGVzdA==');
+    expect(xhr.headers['X-JMAP-Username']).toBe('tester');
+    expect(xhr.headers['X-Plugin-Name']).toBe('a.bin');
+    xhr.onload?.();
+    await call;
+  });
+
+  it('skips progress events whose length is not computable', async () => {
+    const listener = vi.fn();
+    const off = onUploadProgress('staged-3', listener);
+    const call = dispatchApiCall(plugin(), 'http.post', [
+      '/api/upload',
+      new Blob(['x']),
+      { progressFileId: 'staged-3' },
+    ]);
+    const xhr = FakeXHR.instances[0];
+    xhr.upload.onprogress?.({ lengthComputable: false, loaded: 0, total: 0 });
+    xhr.onload?.();
+    await call;
+    expect(listener).not.toHaveBeenCalled();
+    off();
+  });
+
+  it('rejects a malformed progressFileId', async () => {
+    await expect(
+      dispatchApiCall(plugin(), 'http.post', [
+        '/api/upload',
+        new Blob(['x']),
+        { progressFileId: 'x'.repeat(129) },
+      ]),
+    ).rejects.toThrow(/progressFileId/);
+    expect(FakeXHR.instances).toHaveLength(0);
+  });
+
+  it('keeps the fetch path when no progressFileId is given', async () => {
+    const fetchSpy = vi.fn().mockResolvedValue(
+      new Response(JSON.stringify({ ok: 1 }), { status: 200 }),
+    );
+    vi.stubGlobal('fetch', fetchSpy);
+    await dispatchApiCall(plugin(), 'http.post', ['/api/upload', new Blob(['x']), {}]);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+    expect(FakeXHR.instances).toHaveLength(0);
+  });
+
+  it('surfaces a network error as a rejection', async () => {
+    const call = dispatchApiCall(plugin(), 'http.post', [
+      '/api/upload',
+      new Blob(['x']),
+      { progressFileId: 'staged-4' },
+    ]);
+    FakeXHR.instances[0].onerror?.();
+    await expect(call).rejects.toThrow(/Network error/);
+  });
+});

lib/__tests__/upload-progress.test.ts

@@ -0,0 +1,50 @@
+import { describe, it, expect, vi } from 'vitest';
+import { onUploadProgress, reportUploadProgress } from '../upload-progress';
+
+describe('upload progress registry', () => {
+  it('delivers reports to the listener registered for the id', () => {
+    const listener = vi.fn();
+    const off = onUploadProgress('file-1', listener);
+    reportUploadProgress('file-1', 512, 2048);
+    expect(listener).toHaveBeenCalledWith(512, 2048);
+    off();
+  });
+
+  it('drops reports for an id nobody listens to', () => {
+    const listener = vi.fn();
+    const off = onUploadProgress('file-1', listener);
+    reportUploadProgress('file-2', 1, 2);
+    expect(listener).not.toHaveBeenCalled();
+    off();
+  });
+
+  it('stops delivering after unsubscribe', () => {
+    const listener = vi.fn();
+    const off = onUploadProgress('file-1', listener);
+    off();
+    reportUploadProgress('file-1', 1, 2);
+    expect(listener).not.toHaveBeenCalled();
+  });
+
+  it('ignores non-finite byte counts', () => {
+    const listener = vi.fn();
+    const off = onUploadProgress('file-1', listener);
+    reportUploadProgress('file-1', NaN, 100);
+    reportUploadProgress('file-1', 50, Infinity);
+    expect(listener).not.toHaveBeenCalled();
+    off();
+  });
+
+  it("unsubscribing a stale registration does not drop the id's current listener", () => {
+    const first = vi.fn();
+    const second = vi.fn();
+    const offFirst = onUploadProgress('file-1', first);
+    const offSecond = onUploadProgress('file-1', second);
+    // The composer re-registered the id; the stale unsubscribe must be a no-op.
+    offFirst();
+    reportUploadProgress('file-1', 10, 100);
+    expect(second).toHaveBeenCalledWith(10, 100);
+    expect(first).not.toHaveBeenCalled();
+    offSecond();
+  });
+});

lib/plugin-sandbox/host-api.ts

@@ -18,7 +18,8 @@ import {
   type KeywordVisibility,
 } from '@/stores/settings-store';
 import type { MessageListTabsConfig } from '../plugin-types';
-import { apiFetch } from '../browser-navigation';
+import { apiFetch, getPathPrefix } from '../browser-navigation';
+import { reportUploadProgress } from '../upload-progress';
 import { DEFAULT_KEYWORD_SCAN_LIMIT } from '../jmap/client';
 import { suggestKeywordColor } from '../keyword-discovery';
 import { MAX_KEYWORD_LENGTH } from '../keyword-nesting';
@@ -286,6 +287,15 @@ function isApiPostPathAllowed(path: string, allowlist: readonly string[]): boole
 
 interface PluginHttpPostOptions {
   headers?: Record<string, string>;
+  /**
+   * Staged-attachment file id (the one `onBeforeBlobUpload` handed to the
+   * plugin) to report byte-level upload progress for. Only meaningful on a
+   * `Blob`/`File` body. Progress goes to the host-side registry the composer
+   * listens on (`lib/upload-progress.ts`), never back into the sandbox, so
+   * the attachment chip can show a real percentage while a plugin offloads
+   * the file. Reports for an id the composer isn't tracking are dropped.
+   */
+  progressFileId?: string;
 }
 
 /**
@@ -356,6 +366,21 @@ async function doHttpPost(
     headers['Authorization'] = client.getAuthHeader();
     headers['X-JMAP-Username'] = client.getUsername();
   }
+
+  // Progress requires XMLHttpRequest: fetch() exposes no upload progress
+  // events (request streaming is Chrome-only), and the JMAP client's blob
+  // upload already made the same trade for the same reason (#333). Callers
+  // that don't ask for progress keep the fetch path untouched.
+  if (body instanceof Blob && options?.progressFileId !== undefined) {
+    const fileId = options.progressFileId;
+    if (typeof fileId !== 'string' || fileId.length === 0 || fileId.length > 128) {
+      throw new Error('progressFileId must be a non-empty string of at most 128 characters');
+    }
+    return xhrPost(url.pathname + url.search, headers, body, (loaded, total) =>
+      reportUploadProgress(fileId, loaded, total),
+    );
+  }
+
   const res = await apiFetch(url.pathname + url.search, {
     method: 'POST',
     headers,
@@ -365,6 +390,37 @@ async function doHttpPost(
   return { ok: res.ok, status: res.status, data };
 }
 
+/**
+ * POST via XMLHttpRequest so `xhr.upload.onprogress` can report transferred
+ * bytes. Same shape of result as the fetch path in `doHttpPost`; the path is
+ * prefixed exactly like `apiFetch` does for a same-origin `/api/*` path.
+ */
+function xhrPost(
+  path: string,
+  headers: Record<string, string>,
+  body: Blob,
+  onProgress: (loaded: number, total: number) => void,
+): Promise<{ ok: boolean; status: number; data: unknown }> {
+  return new Promise((resolve, reject) => {
+    const xhr = new XMLHttpRequest();
+    xhr.open('POST', getPathPrefix() + path);
+    for (const [name, value] of Object.entries(headers)) {
+      xhr.setRequestHeader(name, value);
+    }
+    xhr.upload.onprogress = (event) => {
+      if (event.lengthComputable) onProgress(event.loaded, event.total);
+    };
+    xhr.onload = () => {
+      let data: unknown = null;
+      try { data = JSON.parse(xhr.responseText); } catch { /* non-JSON body */ }
+      resolve({ ok: xhr.status >= 200 && xhr.status < 300, status: xhr.status, data });
+    };
+    xhr.onerror = () => reject(new Error('Network error during plugin upload'));
+    xhr.ontimeout = () => reject(new Error('Plugin upload timed out'));
+    xhr.send(body);
+  });
+}
+
 // ─── http.fetch (cross-origin, manifest-allowlisted) ──────────
 
 interface PluginFetchInit {

lib/upload-progress.ts

@@ -0,0 +1,42 @@
+/**
+ * Byte-level progress for staged-attachment uploads, keyed by the staged file
+ * id the composer hands to `onBeforeBlobUpload`.
+ *
+ * The composer cannot observe a plugin's own transfer directly: a plugin that
+ * offloads an attachment (see `isExternalAttachmentResult`) uploads the bytes
+ * itself via `api.http.post`, and the File it holds is a structured clone from
+ * the sandbox boundary, so object identity can't correlate that request back
+ * to an attachment. The staged file id can: the composer generated it, handed
+ * it to the hook, and the plugin passes it back via `progressFileId` on the
+ * post options. The host counts the bytes (see `doHttpPost`) and reports them
+ * here; the composer subscribes before running the hook.
+ *
+ * Progress never crosses into the sandbox. Reports for an id nobody listens to
+ * are dropped, so a plugin sending noise for ids it was never handed reaches
+ * nothing.
+ */
+
+export type UploadProgressListener = (loaded: number, total: number) => void;
+
+const listeners = new Map<string, UploadProgressListener>();
+
+/**
+ * Subscribe to progress reports for one staged file id. Returns the
+ * unsubscribe function. One listener per id: the composer owns the staged id,
+ * nothing else has a reason to listen.
+ */
+export function onUploadProgress(fileId: string, listener: UploadProgressListener): () => void {
+  listeners.set(fileId, listener);
+  return () => {
+    // Only clear our own registration, in case the id was re-registered.
+    if (listeners.get(fileId) === listener) listeners.delete(fileId);
+  };
+}
+
+/** Report transferred bytes for a staged file id. No listener, no effect. */
+export function reportUploadProgress(fileId: string, loaded: number, total: number): void {
+  const listener = listeners.get(fileId);
+  if (!listener) return;
+  if (!Number.isFinite(loaded) || !Number.isFinite(total)) return;
+  listener(loaded, total);
+}