Skip to content

feat: make email bodies follow the active theme - #332

Open
salmonumbrella wants to merge 1 commit into
maathimself:mainfrom
salmonumbrella:feat/theme-aware-email-bodies
Open

feat: make email bodies follow the active theme#332
salmonumbrella wants to merge 1 commit into
maathimself:mainfrom
salmonumbrella:feat/theme-aware-email-bodies

Conversation

@salmonumbrella

@salmonumbrella salmonumbrella commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #228.

HTML email bodies now follow MailFlow's effective theme by default, including built-in themes and custom CSS overrides. The renderer lets the browser resolve sender CSS, then performs a bounded, transactional repair of computed colors; if adaptation cannot complete safely, it reveals the existing readable forced-light rendering instead. Users can choose original colors globally or for the current message.

Changes

  • Persist an allow-listed email-body appearance preference and expose it in Appearance settings, with translated labels and a per-message override.
  • Preserve safe sender dark-mode CSS while continuing to remove active and network-capable content; the iframe remains scriptless and sandboxed.
  • Resolve a validated palette from effective app CSS, select sender light/dark media rules, repair only provable color pairs, and protect images, gradients, and other unknown visual islands.
  • Use the same bounded controller for iframe and scoped-div renderers, with immutable sanitized input, generation-safe cancellation, rollback, forced-light fallback, print isolation, and strict node/CSS/time budgets.
  • Keep the appearance engine lazy-loaded and enforce production bundle budgets in CI; add unit, cross-browser, resource-isolation, and visual regression coverage.

Testing

  • Backend tests on Node 22: 1,058 passed across 50 files
  • Backend lint: npm run lint
  • Frontend tests on Node 22: 1,733 passed across 83 suites
  • Frontend lint: npm run lint
  • Frontend production build: npm run build
  • Bundle gates: npm run test:bundle (appearance chunk 7,182 gzip bytes; eager safety chunk 445 gzip bytes)
  • Playwright across Chromium, Firefox, and WebKit: 756 passed
  • Pinned Playwright Linux visual matrix: 48 passed
  • git diff --check

Contributor License Agreement

By submitting this pull request I confirm that:

  • I have read and agree to the Contributor License Agreement.
  • My contribution is my own original work (or I have identified any
    third-party material and confirmed it is compatible with the CLA).
  • I have the right to submit this contribution under the terms of the CLA.

@salmonumbrella
salmonumbrella force-pushed the feat/theme-aware-email-bodies branch from 527ab59 to a4485ae Compare July 31, 2026 12:07
@maathimself

Copy link
Copy Markdown
Owner

Going to pass on this implementation, but I'm keeping the dark mode request (#228) open. Appreciate the work.

@maathimself maathimself closed this Aug 3, 2026
@maathimself maathimself reopened this Aug 13, 2026
@salmonumbrella

salmonumbrella commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@maathimself seeing you reopen this gives me hope for #321 🌭

@maathimself

Copy link
Copy Markdown
Owner

Hey @salmonumbrella,

I pulled #332 onto current main to give it a proper evaluation and ran it against my real inbox. Two things came up that I wanted to get back to you on.

1. Blank preview on some emails (I have a fix)

A few HTML emails rendered as a completely blank preview pane. Tracked it down: the appearance engine only reveals the body on the iframe's load event, and load waits for every subresource. Marketing emails with remote images/fonts that never resolve leave the iframe stuck at readyState: "interactive", so load never fires, processDraft never runs, and the body stays visibility: hidden forever. Repro was a GreyNoise newsletter but it'll hit anything with a slow/dead remote resource.

Fix that worked for me: reveal as soon as the DOM is parsed (readyState !== 'loading') instead of waiting for full load. The load path stays as an idempotent no-op and the image-driven re-measurement is unchanged. Happy to send you the diff (it's a small change in MessagePane's iframe effect, mirrored in the harness), or open it as a PR against your branch, whatever's easiest.

2. Design question on the complexity fallback

Once emails actually show, the branded ones render stripped of all styling. Digging in, that's the style_complexity_limit path: when the CSS is too complex to analyze, the engine strips every sender style and shows a bare forced-light version. I get the intent (if you can't analyze it you can't prove it's readable), but it fires on normal rich newsletters, not just pathological CSS, so a lot of real mail ends up looking broken.

The same email in "original colors" mode renders perfectly, with its full design, since that keeps the sanitized styles forced-light (which is how old MailFlow and Gmail/Apple Mail handle complex mail). So the question is really: should a complexity fallback keep the sender's styles (forced-light) rather than strip them? That'd give up the readability guarantee for rare pathological emails but fix the common case. Didn't want to override your call on it or rewrite the ~24 tests that lock in the strip behavior without checking with you first. What's your read?

@salmonumbrella

salmonumbrella commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

good catch on the blank preview, and ya you read it right: gating reveal on load means one dead tracking pixel holds the whole body hostage. readyState !== 'loading' with load kept as an idempotent no-op is the right shape. open it as a PR against my branch and i'll fold it in.

on the complexity fallback, i think you're right and i'd make the change. stripping was me collapsing two different claims: style_complexity_limit means "can't prove the themed adaptation is safe", not "can't prove forced-light is safe". two reasons i'm convinced:

  1. by the time analysis bails, the forced-light baseline already passed fail-closed on that exact root. that's the same safety gate "original colors" mode ships with, so keeping sender styles forced-light gives up nothing users can't already opt into today.
  2. it's inconsistent as written: every other unproven reason (dynamic_paint_unproven, geometry_condition_unproven, the pseudo limits) already keeps sender styles and just restores forced light on the same root. complexity limit is the odd one out getting the stripped shell.

so the plan i'd go with: route style_complexity_limit through the same keep-styles forced-light path as the other reasons, and keep the stripped recovery shell strictly as last resort (forced-light restore fails or rollback fails).

@maathimself

Copy link
Copy Markdown
Owner

Here's the reveal-on-DOM-parsed fix as a diff (the cross-fork PR route hit a wall pushing the branch, so pasting it here per your "whatever's easiest"). Two files, MessagePane's iframe effect and the appearance harness, mirrored. Verified: frontend lint + build clean, and the email-appearance e2e suite passes on chromium.

diff --git a/frontend/e2e/email-appearance/harness.jsx b/frontend/e2e/email-appearance/harness.jsx
index 66905db..e8d541a 100644
--- a/frontend/e2e/email-appearance/harness.jsx
+++ b/frontend/e2e/email-appearance/harness.jsx
@@ -1259,8 +1259,26 @@ function Harness() {
       window.mailflowFixtureIframeLifecycle.readyStateShortcuts += 1;
       void processFrameDocument(iframe.contentDocument);
     }
+    // Mirror MessagePane: reveal on DOM-parsed rather than the full `load` event, so an
+    // email whose remote subresources never resolve (readyState stuck "interactive")
+    // still processes. Scoped to normal fixtures so the timing-scenario tests keep their
+    // exact wiring. processFrameDocument is idempotent (processedDocument guard).
+    let domReadyRafId = 0;
+    if (!scenario) {
+      const revealWhenParsed = () => {
+        const doc = iframe.contentDocument;
+        if (doc && doc.readyState !== 'loading'
+          && emailFrameDocumentMatchesSource(doc, frameSourceToken)) {
+          void processFrameDocument(doc);
+          return;
+        }
+        domReadyRafId = requestAnimationFrame(revealWhenParsed);
+      };
+      revealWhenParsed();
+    }
     return () => {
       cancelled = true;
+      if (domReadyRafId) cancelAnimationFrame(domReadyRafId);
       iframe.removeEventListener('load', onLoaded);
     };
   }, [appearance.processDraft, appearance.recovery, appearance.rootKey, frameSourceToken, html, renderer, scenario]);
diff --git a/frontend/src/components/MessagePane.jsx b/frontend/src/components/MessagePane.jsx
index 7b8fc7b..3c5c9f7 100644
--- a/frontend/src/components/MessagePane.jsx
+++ b/frontend/src/components/MessagePane.jsx
@@ -590,14 +590,29 @@ export default function MessagePane() {
       }
     };
 
+    // The iframe `load` event waits for EVERY subresource. Marketing emails routinely
+    // carry remote images/fonts that never resolve, which leaves the document stuck at
+    // readyState "interactive" and the message permanently hidden. Reveal as soon as the
+    // DOM is parsed instead; onLoaded is idempotent (its processedDocument + source-token
+    // guards), so the later `load` event — if it ever fires — is a harmless no-op, and
+    // image-driven re-measurement still runs.
     iframe.addEventListener('load', onLoaded, { once: true });
-    if (iframe.contentDocument?.readyState === 'complete') {
-      onLoaded();
-    }
+    let domReadyRafId = 0;
+    const revealWhenParsed = () => {
+      const doc = iframe.contentDocument;
+      if (doc && doc.readyState !== 'loading'
+        && emailFrameDocumentMatchesSource(doc, iframeSourceToken)) {
+        onLoaded();
+        return;
+      }
+      domReadyRafId = requestAnimationFrame(revealWhenParsed);
+    };
+    revealWhenParsed();
 
     return () => {
       cancelled = true;
       cancelAnimationFrame(rafId);
+      if (domReadyRafId) cancelAnimationFrame(domReadyRafId);
       if (roRef.current) { roRef.current.disconnect(); roRef.current = null; }
       iframe.removeEventListener('load', onLoaded);
       emailScaleRef.current = 1;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support inverted mail body (dark mode mail body)

2 participants