diff --git a/static/js/emailInbox.js b/static/js/emailInbox.js index 18f883a..2655b33 100644 --- a/static/js/emailInbox.js +++ b/static/js/emailInbox.js @@ -8,6 +8,7 @@ import sessionModule from './sessions.js'; import { initEmailLibrary, openEmailLibrary, closeEmailLibrary, isOpen as isLibOpen, prewarmEmailLibrary } from './emailLibrary.js'; import * as Modals from './modalManager.js'; import { applyEdgeDock } from './modalSnap.js'; +import { buildReplyAllCc } from './emailLibrary/replyRecipients.js'; const API_BASE = window.location.origin; const _acct = () => window.__odysseusActiveEmailAccount @@ -696,17 +697,7 @@ async function _openEmail(em, itemEl, preloadedData = null, mode = 'reply') { if (mode === 'reply-all') { // Build reply-all: TO = original sender, CC = everyone else (To + Cc minus me) - const origTo = (data.to || '').split(',').map(s => s.trim()).filter(Boolean); - const origCc = (data.cc || '').split(',').map(s => s.trim()).filter(Boolean); - const allOthers = [...origTo, ...origCc] - .filter(addr => { - // Extract email from "Name " or "email@x" - const match = addr.match(/<([^>]+)>/) || [null, addr]; - return !match[1].toLowerCase().includes(myAddress); - }); - if (allOthers.length > 0) { - ccAddresses = allOthers.join(', '); - } + ccAddresses = buildReplyAllCc(data, myAddress); } else if (mode === 'forward') { toAddress = ''; subjectPrefix = 'Fwd: '; diff --git a/static/js/emailLibrary.js b/static/js/emailLibrary.js index 1d00582..e1a4fb6 100644 --- a/static/js/emailLibrary.js +++ b/static/js/emailLibrary.js @@ -492,6 +492,15 @@ function _libCacheWriteBack() { // Simple global rather than cross-module import to keep coupling minimal. function _publishActiveAccount() { try { window.__odysseusActiveEmailAccount = state._libAccountId || null; } catch (_) {} + // Publish the active account's own address so reply-all can exclude us from + // the recipient list. This global was read in emailInbox.js but never set. + try { + const accts = state._libAccounts || []; + const active = accts.find(a => a && a.id === state._libAccountId) + || accts.find(a => a && a.is_default) + || accts[0]; + window._myEmailAddress = (active && (active.from_address || active.imap_user)) || ''; + } catch (_) {} } export function initEmailLibrary(config) { diff --git a/static/js/emailLibrary/replyRecipients.js b/static/js/emailLibrary/replyRecipients.js new file mode 100644 index 0000000..89f0341 --- /dev/null +++ b/static/js/emailLibrary/replyRecipients.js @@ -0,0 +1,25 @@ +// static/js/emailLibrary/replyRecipients.js +// +// Pure helpers for building reply-all recipient lists. No DOM, no fetch, +// no shared state — safe to import anywhere and to unit-test under node. + +// Extract the bare email from "Name " or a plain "email@x". +export function extractEmail(addr) { + const m = (addr || '').match(/<([^>]+)>/); + return (m ? m[1] : (addr || '')).trim().toLowerCase(); +} + +// Reply-all CC = everyone on the original To + Cc, minus ourselves, with the +// original "Name " form preserved. +// +// `myAddress` empty/unknown ⇒ no exclusion. Comparing by exact extracted email +// (not a substring `includes`) is what fixes issue #360: an empty self address +// made `"...".includes("")` true for every recipient, so reply-all dropped the +// entire Cc list and kept only the original sender. +export function buildReplyAllCc(data, myAddress) { + const me = (myAddress || '').toLowerCase(); + const split = (s) => (s || '').split(',').map((x) => x.trim()).filter(Boolean); + return [...split(data && data.to), ...split(data && data.cc)] + .filter((addr) => !me || extractEmail(addr) !== me) + .join(', '); +} diff --git a/tests/test_reply_recipients_js.py b/tests/test_reply_recipients_js.py new file mode 100644 index 0000000..77dcc97 --- /dev/null +++ b/tests/test_reply_recipients_js.py @@ -0,0 +1,53 @@ +"""Pin the pure reply-all recipient helpers in emailLibrary/replyRecipients.js. + +Driven through `node --input-type=module` so we exercise the real JS without a +full Vitest/Jest setup (same approach as test_compare_js.py). Skips when `node` +is not installed rather than failing. + +Regression for issue #360: reply-all dropped every Cc recipient when the user's +own address was unknown, because the old filter used `includes("")` (always +true) instead of an exact-email comparison. +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parent.parent +_HELPER = _REPO / "static" / "js" / "emailLibrary" / "replyRecipients.js" +_HAS_NODE = shutil.which("node") is not None + + +def _run(js: str) -> str: + proc = subprocess.run( + ["node", "--input-type=module"], + input=js, capture_output=True, text=True, cwd=str(_REPO), timeout=30, + ) + assert proc.returncode == 0, proc.stderr + return proc.stdout.strip() + + +@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") +def test_reply_all_keeps_cc_when_self_unknown(): + data = {"to": "Alice , bob@x.com", "cc": "Carol "} + js = f""" + import {{ buildReplyAllCc }} from '{_HELPER.as_posix()}'; + console.log(JSON.stringify(buildReplyAllCc({json.dumps(data)}, ''))); + """ + cc = json.loads(_run(js)) + # Empty self address must NOT wipe everyone (the #360 bug). + assert cc == "Alice , bob@x.com, Carol " + + +@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") +def test_reply_all_excludes_only_self_exactly(): + data = {"to": "Me , Alice ", "cc": "bob@x.com"} + js = f""" + import {{ buildReplyAllCc }} from '{_HELPER.as_posix()}'; + console.log(JSON.stringify(buildReplyAllCc({json.dumps(data)}, 'me@x.com'))); + """ + cc = json.loads(_run(js)) + # Our own address is dropped; a substring-similar address is kept. + assert cc == "Alice , bob@x.com"