Skip to content

Comments

new email renderer#1584

Merged
ahmetskilinc merged 8 commits intostagingfrom
07-01-new_email_renderer
Jul 4, 2025
Merged

new email renderer#1584
ahmetskilinc merged 8 commits intostagingfrom
07-01-new_email_renderer

Conversation

@ahmetskilinc
Copy link
Contributor

@ahmetskilinc ahmetskilinc commented Jul 1, 2025

Replace iFrame with Shadow DOM for Email Content Rendering

Description

Replaced the MailIframe component with a new MailContent component that uses Shadow DOM instead of iframes for rendering email content. This approach provides better security isolation while maintaining styling control and performance. The implementation uses DOMPurify for sanitization and includes proper handling of external images, trusted senders, and theme adaptation.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🔒 Security enhancement
  • ⚡ Performance improvement

Summary by CodeRabbit

  • New Features

    • Introduced a new email content viewer that securely displays HTML emails with improved security, style isolation, and theme support.
    • Added user controls to manage image loading and trust email senders directly from the email view.
  • Refactor

    • Replaced the previous email iframe viewer with the new secure content viewer, streamlining functionality and enhancing user experience.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 1, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

A new MailContent React component is introduced to securely render sanitized HTML email content using a shadow DOM, handling image loading, CSP violations, and sender trust. The previous MailIframe component is removed, and MailIframe is now a re-export of MailContent, consolidating all rendering logic in the new component.

Changes

File(s) Change Summary
apps/mail/components/mail/mail-content.tsx Added new MailContent React component for secure, theme-aware, sanitized email rendering with controls.
apps/mail/components/mail/mail-iframe.tsx Removed previous MailIframe implementation entirely.
apps/mail/components/mail/mail-display.tsx Replaced MailIframe usage with MailContent; added safe optional chaining for email body rendering.
apps/mail/lib/email-utils.ts Updated fixNonReadableColors to accept options object with configurable defaultBackground.
apps/server/src/lib/email-processor.ts Added processEmailHtml function to sanitize and style email HTML with image loading and theme options.
apps/server/src/trpc/routes/mail.ts Added processEmailContent private procedure to process and sanitize email content via TRPC mutation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant MailContent
    participant UserSettings
    participant TRPC/ReactQuery

    User->>MailContent: Render with {html, senderEmail}
    MailContent->>UserSettings: Fetch settings (trusted senders, image prefs)
    MailContent->>TRPC/ReactQuery: Request processed HTML via processEmailContent mutation
    TRPC/ReactQuery->>MailContent: Return sanitized, themed HTML + blocked images flag
    MailContent->>MailContent: Inject sanitized HTML into shadow DOM
    alt Images blocked
        MailContent->>User: Show warning banner
        User->>MailContent: Click "Show images" or "Trust sender"
        alt Trust sender
            MailContent->>TRPC/ReactQuery: Update trusted senders
            TRPC/ReactQuery-->>MailContent: Refetch settings
        end
    end
    MailContent->>User: Render email content with theme-aware styles
    User->>MailContent: Click link in email
    MailContent->>User: Open link in new tab
Loading

Possibly related PRs

  • Hotfix #1390: The main PR completely removes the MailIframe component and replaces its usage with a new MailContent component that renders sanitized email HTML inside a shadow DOM, so the changes directly replace and supersede the functionality of the MailIframe component modified in the retrieved PR.

  • Hotfix #1391: This PR modifies the sandbox attribute of the existing MailIframe component, while the main PR removes MailIframe entirely and replaces it with MailContent, indicating related but distinct changes to email content rendering components.

  • Only show external images banner if there was a CSP violation #648: Both PRs address CSP violation detection and image loading UI for email content, but the main PR replaces the iframe-based approach with a shadow DOM-based component, making their implementations related at the feature level but distinct in code.

Poem

🐇 In shadows deep, where emails glow,
Safe and sound, their secrets show.
No iframes cage, no images stray,
Trust blooms bright with one click’s sway.
A rabbit’s cheer for safer mail flow! ✨📧


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd1663b and f03884f.

📒 Files selected for processing (6)
  • apps/mail/components/mail/mail-content.tsx (1 hunks)
  • apps/mail/components/mail/mail-display.tsx (3 hunks)
  • apps/mail/components/mail/mail-iframe.tsx (0 hunks)
  • apps/mail/lib/email-utils.ts (3 hunks)
  • apps/server/src/lib/email-processor.ts (1 hunks)
  • apps/server/src/trpc/routes/mail.ts (2 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ahmetskilinc ahmetskilinc marked this pull request as ready for review July 1, 2025 12:46
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 1, 2025

Deploying zero-prod with  Cloudflare Pages  Cloudflare Pages

Latest commit: bd1663b
Status: ✅  Deploy successful!
Preview URL: https://cb1ef5dd.zero-76h.pages.dev
Branch Preview URL: https://07-01-new-email-renderer.zero-76h.pages.dev

View logs

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Major architectural change replacing iframe-based email rendering with Shadow DOM implementation for improved security and performance in the email content display system.

  • Introduced new apps/mail/components/mail/mail-content.tsx using Shadow DOM for content isolation and DOMPurify for sanitization
  • Implemented theme-aware styling system that properly handles dark/light modes within Shadow DOM context
  • Added strict Content Security Policy (CSP) violation detection for enhanced security monitoring
  • Enhanced external resource handling with granular controls for trusted/untrusted sender image loading
  • Integrated caching optimizations for improved rendering performance

2 files reviewed, no comments
Edit PR Review Bot Settings | Greptile

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 1, 2025

Deploying zero-staging with  Cloudflare Pages  Cloudflare Pages

Latest commit: bd1663b
Status: ✅  Deploy successful!
Preview URL: https://68425a9f.zero-staging-c02.pages.dev
Branch Preview URL: https://07-01-new-email-renderer.zero-staging-c02.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
apps/mail/components/mail/mail-content.tsx (1)

18-268: Consider extracting logic into custom hooks for better maintainability

The component handles multiple responsibilities. Consider extracting the email sanitization logic into a custom hook like useEmailSanitization and the shadow DOM management into useShadowDOM for better separation of concerns and reusability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f3b21a8 and f5ac1fb.

📒 Files selected for processing (2)
  • apps/mail/components/mail/mail-content.tsx (1 hunks)
  • apps/mail/components/mail/mail-iframe.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
apps/mail/components/mail/mail-content.tsx (2)
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:0-0
Timestamp: 2025-04-07T20:46:04.726Z
Learning: In the Zero mail application, the "Trust Sender" button for external images is only shown when a sender is not already in the trusted senders list (`settings?.trustedSenders`). This is controlled by the condition `!imagesEnabled && !settings?.externalImages`, where `imagesEnabled` is based on `isTrustedSender || settings?.externalImages` and `isTrustedSender` is determined by `settings?.trustedSenders?.includes(senderEmail)`. This design pattern prevents duplicate entries in the trusted senders list.
🧬 Code Graph Analysis (1)
apps/mail/components/mail/mail-content.tsx (4)
apps/mail/components/mail/mail-iframe.tsx (1)
  • MailContent (1-1)
apps/mail/hooks/use-settings.ts (1)
  • useSettings (5-17)
apps/server/src/lib/schemas.ts (1)
  • defaultUserSettings (137-151)
apps/mail/lib/utils.ts (1)
  • cn (51-51)
🔇 Additional comments (4)
apps/mail/components/mail/mail-iframe.tsx (1)

1-1: Clean refactoring approach!

Good use of the re-export pattern to maintain backward compatibility while moving the implementation to a new component.

apps/mail/components/mail/mail-content.tsx (3)

1-16: Well-structured imports and interface!

The imports are appropriately organized and the interface is clear and concise.


200-210: Proper shadow DOM initialization!

The shadow DOM setup correctly checks for existing shadow roots and updates content appropriately.


212-242: Well-implemented event handling with proper cleanup!

Good use of event delegation on the shadow root and proper cleanup in the effect return. The security measures for link handling are appropriate.

@ahmetskilinc ahmetskilinc marked this pull request as draft July 1, 2025 15:36
@retrogtx
Copy link
Contributor

retrogtx commented Jul 2, 2025

#1593

@ahmetskilinc ahmetskilinc force-pushed the 07-01-new_email_renderer branch from e7a5ced to 6d49f78 Compare July 2, 2025 13:06
@ahmetskilinc ahmetskilinc marked this pull request as ready for review July 2, 2025 13:07
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Major changes to the optimistic action system and email processing pipeline to support the new Shadow DOM renderer implementation.

  • Enhanced apps/server/src/lib/email-processor.ts with robust HTML sanitization and theme-aware styling for Shadow DOM
  • Added strongly-typed action parameters in apps/mail/lib/optimistic-actions-manager.ts replacing generic 'any' types
  • Updated production Hyperdrive database ID in apps/server/wrangler.jsonc requiring careful deployment coordination
  • Simplified message selection logic in apps/mail/components/mail/mail-list.tsx by removing special handling for non-draft messages
  • Improved color contrast handling in apps/mail/lib/email-utils.ts with configurable minimum contrast and background colors

10 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
apps/mail/components/mail/mail-content.tsx (1)

48-50: Fix potential null reference error in trusted senders logic.

The code uses optional chaining to check data?.settings?.trustedSenders but then accesses data.settings.trustedSenders without optional chaining, which could cause a runtime error if settings is undefined.

-        trustedSenders: data?.settings?.trustedSenders
-          ? data.settings.trustedSenders.concat(senderEmail)
+        trustedSenders: data?.settings?.trustedSenders
+          ? data.settings.trustedSenders.concat(senderEmail)
🧹 Nitpick comments (4)
apps/mail/components/mail/mail-content.tsx (4)

69-88: Optimize query key dependencies for better cache efficiency.

The query key includes isTrustedSender || temporaryImagesEnabled as a boolean, but this could be optimized by using the actual values that determine image loading state for more granular caching.

Consider using more specific cache keys:

-    queryKey: ['email-content', html, isTrustedSender || temporaryImagesEnabled, resolvedTheme],
+    queryKey: ['email-content', html, { 
+      trustedSender: isTrustedSender, 
+      tempImages: temporaryImagesEnabled, 
+      externalImages: data?.settings?.externalImages 
+    }, resolvedTheme],

78-80: Consider batching state updates to prevent unnecessary re-renders.

Setting setCspViolation(true) here and potentially in the image error handler could cause multiple state updates. Consider consolidating CSP violation detection logic.

+      // Track if we need to show CSP warning
+      const shouldShowCspWarning = result.hasBlockedImages && !(isTrustedSender || temporaryImagesEnabled);
       if (result.hasBlockedImages) {
-        setCspViolation(true);
+        setCspViolation(shouldShowCspWarning);
       }

107-121: Improve error handling and performance in color fixing logic.

The color fixing logic processes all children but lacks proper error boundaries and could be optimized to avoid processing hidden elements upfront.

     const applyFix: () => void = () => {
       const topLevelEls = Array.from(root.children) as HTMLElement[];
       topLevelEls.forEach((el) => {
+        // Skip hidden elements early
+        const style = getComputedStyle(el);
+        if (style.display === 'none' || style.visibility === 'hidden') return;
+        
         try {
           fixNonReadableColors(el, {
             defaultBackground: resolvedTheme === 'dark' ? 'rgb(10,10,10)' : '#ffffff',
           });
         } catch (err) {
           console.error('Failed to fix colors in email content:', err);
+          // Continue processing other elements even if one fails
         }
       });
     };

163-163: Simplify complex conditional logic in render.

The conditional for showing the CSP violation warning is complex and could be simplified for better readability and maintainability.

Consider extracting the condition to a computed value:

+  const shouldShowImageWarning = cspViolation && 
+    !isTrustedSender && 
+    !data?.settings?.externalImages;
+
   return (
     <>
-      {cspViolation && !isTrustedSender && !data?.settings?.externalImages && (
+      {shouldShowImageWarning && (
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5ac1fb and 6d49f78.

📒 Files selected for processing (6)
  • apps/mail/components/mail/mail-content.tsx (1 hunks)
  • apps/mail/components/mail/mail-display.tsx (3 hunks)
  • apps/mail/components/mail/mail-iframe.tsx (0 hunks)
  • apps/mail/lib/email-utils.ts (3 hunks)
  • apps/server/src/lib/email-processor.ts (1 hunks)
  • apps/server/src/trpc/routes/mail.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • apps/mail/components/mail/mail-iframe.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
apps/mail/components/mail/mail-display.tsx (2)
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
apps/mail/components/mail/mail-content.tsx (7)
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:102-102
Timestamp: 2025-04-07T20:48:48.213Z
Learning: In the Zero mail application, when implementing the "Trust Sender" feature, the banner should disappear immediately when a user clicks the "Trust Sender" button without showing a loading state. This is achieved by setting `setImagesEnabled(true)` at the beginning of the `onTrustSender` function, providing immediate visual feedback while the backend operation continues asynchronously.
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:0-0
Timestamp: 2025-04-07T20:46:04.726Z
Learning: In the Zero mail application, the "Trust Sender" button for external images is only shown when a sender is not already in the trusted senders list (`settings?.trustedSenders`). This is controlled by the condition `!imagesEnabled && !settings?.externalImages`, where `imagesEnabled` is based on `isTrustedSender || settings?.externalImages` and `isTrustedSender` is determined by `settings?.trustedSenders?.includes(senderEmail)`. This design pattern prevents duplicate entries in the trusted senders list.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.351Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
apps/mail/lib/email-utils.ts (3)
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.351Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
apps/server/src/trpc/routes/mail.ts (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.351Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
apps/server/src/lib/email-processor.ts (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
🧬 Code Graph Analysis (3)
apps/mail/components/mail/mail-display.tsx (1)
apps/mail/components/mail/mail-content.tsx (1)
  • MailContent (18-182)
apps/mail/components/mail/mail-content.tsx (4)
apps/mail/hooks/use-settings.ts (1)
  • useSettings (5-17)
apps/server/src/lib/schemas.ts (1)
  • defaultUserSettings (137-151)
apps/mail/lib/email-utils.ts (1)
  • fixNonReadableColors (5-43)
apps/mail/lib/utils.ts (1)
  • cn (51-51)
apps/server/src/trpc/routes/mail.ts (2)
apps/server/src/trpc/trpc.ts (1)
  • privateProcedure (20-28)
apps/server/src/lib/email-processor.ts (1)
  • processEmailHtml (10-136)
🔇 Additional comments (10)
apps/mail/lib/email-utils.ts (2)

5-9: LGTM! Well-designed API enhancement.

The function signature change maintains backward compatibility while adding flexibility for configurable background colors. The default values are sensible (3.5 contrast ratio meets WCAG AA standards, white background is a common default).


28-28: Proper implementation of configurable background.

The implementation correctly threads the defaultBackground parameter through to getEffectiveBackgroundColor, enabling theme-aware color contrast adjustments as mentioned in the AI summary.

Also applies to: 45-45, 52-52

apps/server/src/lib/email-processor.ts (3)

16-44: Strong security-focused sanitization configuration.

The sanitization approach is well-designed with:

  • Conservative defaults (no tags/attributes allowed by default)
  • Conditional scheme allowlisting based on image loading preferences
  • Proper transformation of img tags to hidden spans when blocked
  • Security attributes (target="_blank", rel="noopener noreferrer") added to anchor tags

This effectively prevents XSS while maintaining controlled functionality.


50-71: Effective tracking prevention and content cleanup.

The removal of tracking pixels (1x1 and 0x0 images) and hidden preheader elements is comprehensive. The style attribute checks cover common hiding techniques used by email marketing platforms.


73-131: Well-structured theme-aware styling.

The minimal CSS styles provide:

  • Proper shadow DOM styling with :host selector
  • Theme-specific colors for light/dark modes
  • Responsive image handling (max-width: 100%)
  • Accessible link colors with proper contrast ratios

The use of the Geist font family aligns with the application's design system.

apps/mail/components/mail/mail-display.tsx (2)

50-50: Clean component migration.

The import change from MailIframe to MailContent aligns with the shadow DOM-based rendering approach mentioned in the PR objectives.


1771-1771: Safe component usage with proper props.

The component correctly passes the required html and senderEmail props with optional chaining to prevent errors when emailData is undefined. The sender email is guaranteed to be non-empty based on the retrieved learning from the Mail-0/Zero application.

apps/server/src/trpc/routes/mail.ts (2)

1-1: Proper imports for the new procedure.

All necessary imports are included: privateProcedure for authentication, processEmailHtml for the core logic, and TRPCError for error handling.

Also applies to: 5-5, 11-11, 15-15


405-432: Well-implemented tRPC procedure with proper security and error handling.

The implementation demonstrates good practices:

  • Uses privateProcedure to ensure authentication (as shown in the relevant code snippets)
  • Comprehensive input validation with Zod schema enforcing required types
  • Proper error handling with logging and TRPC error responses
  • Direct delegation to the processEmailHtml function with consistent return format

The procedure correctly exposes the email processing functionality as a mutation, which aligns with its side-effect nature of processing and transforming content.

apps/mail/components/mail/mail-content.tsx (1)

18-30: Excellent implementation of secure email rendering with Shadow DOM.

The component architecture effectively addresses the security and performance goals outlined in the PR objectives. Using Shadow DOM provides better isolation than iframes while maintaining styling control, and the integration with React Query ensures efficient caching of processed content.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/mail/components/mail/mail-content.tsx (1)

48-50: Fix inconsistent optional chaining to prevent runtime errors.

The code uses optional chaining in the condition but not in the accessed property, which could cause runtime errors.

  trustedSenders: data?.settings?.trustedSenders
-    ? data.settings.trustedSenders.concat(senderEmail)
+    ? data.settings.trustedSenders.concat(senderEmail)
    : [senderEmail],
🧹 Nitpick comments (3)
apps/server/src/lib/email-processor.ts (2)

118-118: Consider improving dark theme border contrast.

The border color for quoted content uses #000 in dark theme, which may provide insufficient contrast against dark backgrounds.

-        border-left: 1px solid ${theme === 'dark' ? '#000' : '#ccc'};
+        border-left: 1px solid ${theme === 'dark' ? '#444' : '#ccc'};

19-24: Enhance URL scheme validation for better security.

Consider adding validation for blob URLs and restricting data URLs to specific MIME types to prevent potential security issues.

  allowedSchemes: shouldLoadImages
-    ? ['http', 'https', 'mailto', 'tel', 'data', 'cid', 'blob']
+    ? ['http', 'https', 'mailto', 'tel', 'data', 'cid']
    : ['http', 'https', 'mailto', 'tel', 'cid'],
  allowedSchemesByTag: {
-    img: shouldLoadImages ? ['http', 'https', 'data', 'cid', 'blob'] : ['cid'],
+    img: shouldLoadImages ? ['http', 'https', 'data', 'cid'] : ['cid'],
  },
apps/mail/components/mail/mail-content.tsx (1)

107-118: Consider adding error boundary for color fixing failures.

While the individual color fixing attempts are wrapped in try-catch, consider adding an error boundary or more granular error handling for the entire color fixing process.

  const applyFix: () => void = () => {
    const topLevelEls = Array.from(root.children) as HTMLElement[];
    topLevelEls.forEach((el) => {
      try {
        fixNonReadableColors(el, {
          defaultBackground: resolvedTheme === 'dark' ? 'rgb(10,10,10)' : '#ffffff',
        });
      } catch (err) {
-       console.error('Failed to fix colors in email content:', err);
+       console.warn('Skipping color fix for element due to error:', err);
      }
    });
  };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d49f78 and bd1663b.

📒 Files selected for processing (2)
  • apps/mail/components/mail/mail-content.tsx (1 hunks)
  • apps/server/src/lib/email-processor.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
apps/server/src/lib/email-processor.ts (3)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/mail/note-panel.tsx:511-513
Timestamp: 2025-05-07T16:45:40.468Z
Learning: The amber color for notes (text-amber-500, fill-amber-200, etc.) is intentionally fixed and should not be converted to theme variables, as it serves as a recognizable visual metaphor for sticky notes.
apps/mail/components/mail/mail-content.tsx (9)
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:102-102
Timestamp: 2025-04-07T20:48:48.213Z
Learning: In the Zero mail application, when implementing the "Trust Sender" feature, the banner should disappear immediately when a user clicks the "Trust Sender" button without showing a loading state. This is achieved by setting `setImagesEnabled(true)` at the beginning of the `onTrustSender` function, providing immediate visual feedback while the backend operation continues asynchronously.
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:0-0
Timestamp: 2025-04-07T20:46:04.726Z
Learning: In the Zero mail application, the "Trust Sender" button for external images is only shown when a sender is not already in the trusted senders list (`settings?.trustedSenders`). This is controlled by the condition `!imagesEnabled && !settings?.externalImages`, where `imagesEnabled` is based on `isTrustedSender || settings?.externalImages` and `isTrustedSender` is determined by `settings?.trustedSenders?.includes(senderEmail)`. This design pattern prevents duplicate entries in the trusted senders list.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.351Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/connection/add.tsx:77-77
Timestamp: 2025-05-07T16:55:46.513Z
Learning: For the "Upgrade" link in AddConnectionDialog, using a proper <button> element instead of a <span> with onClick is recognized as an accessibility improvement but was deferred as out of scope in PR #902 (CSS variables PR).
Learnt from: danteissaias
PR: Mail-0/Zero#458
File: apps/mail/lib/email-utils.ts:126-131
Timestamp: 2025-03-16T23:14:09.209Z
Learning: When working with mailto URLs in JavaScript/TypeScript, the `url.pathname` property correctly extracts the email address from a mailto URL (e.g., for "mailto:test@example.com?subject=Test", `url.pathname` will be "test@example.com").
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
🧬 Code Graph Analysis (1)
apps/mail/components/mail/mail-content.tsx (4)
apps/mail/hooks/use-settings.ts (1)
  • useSettings (5-17)
apps/server/src/lib/schemas.ts (1)
  • defaultUserSettings (137-151)
apps/mail/lib/email-utils.ts (1)
  • fixNonReadableColors (5-43)
apps/mail/lib/utils.ts (1)
  • cn (51-51)
🪛 Biome (1.9.4)
apps/mail/components/mail/mail-content.tsx

[error] 154-154: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Cloudflare Pages: zero-prod
🔇 Additional comments (8)
apps/server/src/lib/email-processor.ts (2)

16-44: Excellent security configuration for HTML sanitization.

The sanitization configuration is well-designed with strict defaults and conditional scheme allowances based on image loading permissions. The transform functions properly handle security attributes for anchors and image blocking logic.


50-71: Good approach to cleaning tracking elements and hidden content.

The removal of tracking pixels (1x1 and 0x0 images) and hidden preheader elements effectively cleans up email content. The style pattern matching covers various ways emails hide content.

apps/mail/components/mail/mail-content.tsx (6)

18-31: Well-structured component setup with proper dependency management.

The component initialization correctly uses React hooks and memoization for optimal performance. The isTrustedSender memoization efficiently combines external images setting and trusted senders list.


69-88: Effective caching strategy for processed email content.

The query key properly includes all dependencies (html, image loading state, theme) and the caching configuration with 30-minute stale time and 60-minute garbage collection time is appropriate for email content.


123-127: Good implementation of CSP violation state reset.

The effect properly resets the CSP violation state when images are enabled, addressing the concern about persistent warnings.


129-140: Improved image error handling with proper context validation.

The error handler now correctly checks if images are actually blocked before setting CSP violation state, preventing false positives from network errors or invalid sources.


147-158: Secure link handling with protocol validation.

The click handler properly validates URL protocols and handles both HTTP/HTTPS and mailto links securely. The prevention of default behavior and explicit URL validation prevents malicious protocol execution.


182-188: Proper error handling for trust sender action.

The async error handling for the trust sender button is correctly implemented with try-catch block and error logging.

@ahmetskilinc ahmetskilinc force-pushed the 07-01-new_email_renderer branch from bd1663b to f03884f Compare July 4, 2025 02:16
Copy link
Contributor Author

ahmetskilinc commented Jul 4, 2025

Merge activity

  • Jul 4, 2:16 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 4, 2:17 AM UTC: @ahmetskilinc merged this pull request with Graphite.

@ahmetskilinc ahmetskilinc merged commit f623c91 into staging Jul 4, 2025
5 of 6 checks passed
@ahmetskilinc ahmetskilinc deleted the 07-01-new_email_renderer branch July 4, 2025 02:17
@coderabbitai coderabbitai bot mentioned this pull request Jul 4, 2025
34 tasks
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.

3 participants