Skip to content

feat(email): open external links in a new tab (safely)#598

Open
hildebrandttk wants to merge 1 commit into
bulwarkmail:mainfrom
hildebrandttk:feat/email-links-new-tab
Open

feat(email): open external links in a new tab (safely)#598
hildebrandttk wants to merge 1 commit into
bulwarkmail:mainfrom
hildebrandttk:feat/email-links-new-tab

Conversation

@hildebrandttk

Copy link
Copy Markdown
Contributor

Summary

External web links (http/https) in rendered email bodies now open in a new browser tab with target="_blank" rel="noopener noreferrer". mailto:, tel:, and in-page #anchors keep their default behavior (hand off to the mail client / navigate in place) instead of spawning a blank tab.

Two root causes were fixed:

  1. The iframe HTML render path set target=_blank on EVERY , including mailto:/tel:. It is now scoped to http(s) links.

  2. Plaintext emails (and HTML alternatives that are really plaintext) have no tags in the source - they are generated by plainTextToSafeHtml and rendered in the main DOM via sanitizePlainTextRenderedHtml. Although the generator emits target/rel, DOMPurify silently stripped them: when a custom ALLOWED_URI_REGEXP is set it validates target/rel against the regexp (which "_blank" never matches) and drops them, so those links opened in the same tab.

Changes

  • email-sanitization: add isHttpLinkHref() and a shared applyNewTabToAnchor() helper (http/https -> target+rel; mailto/tel/#/other -> strip target/rel).
  • sanitizePlainTextRenderedHtml: re-apply target/rel after sanitization via an afterSanitizeAttributes hook (the established codebase pattern).
  • sanitizeI18nHtml: same DOMPurify strip affected the in-app docs link in settings.security.not_available (target="_blank") - keep the translator's target and harden rel="noopener noreferrer".
  • email-viewer: both anchor passes (DOMPurify hook + post-render walk) now use the shared applyNewTabToAnchor() helper.
  • tests: unit coverage for isHttpLinkHref / applyNewTabToAnchor / sanitizeI18nHtml plus an end-to-end integration suite exercising the real plaintext and HTML/iframe render pipelines so this can't regress unnoticed.

Related Issues

Closes #

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactor / code quality improvement
  • Chore / dependency update / CI change

Checklist

  • I have read the Contributing Guide
  • My code follows the project's code style and conventions
  • I have run npm run typecheck && npm run lint and there are no errors
  • The build passes (npm run build)
  • I have tested my changes locally
  • I have added or updated documentation if needed
  • I have updated translations (locales/) if my changes affect user-facing text
  • I have included screenshots or a screen recording for UI changes

Screenshots / Demo

Notes for Reviewers

External web links (http/https) in rendered email bodies now open in a new
browser tab with target="_blank" rel="noopener noreferrer". mailto:, tel:,
and in-page #anchors keep their default behavior (hand off to the mail
client / navigate in place) instead of spawning a blank tab.

Two root causes were fixed:

1. The iframe HTML render path set target=_blank on EVERY <a>, including
   mailto:/tel:. It is now scoped to http(s) links.

2. Plaintext emails (and HTML alternatives that are really plaintext) have
   no <a> tags in the source - they are generated by plainTextToSafeHtml and
   rendered in the main DOM via sanitizePlainTextRenderedHtml. Although the
   generator emits target/rel, DOMPurify silently stripped them: when a
   custom ALLOWED_URI_REGEXP is set it validates target/rel against the
   regexp (which "_blank" never matches) and drops them, so those links
   opened in the same tab.

Changes:
- email-sanitization: add isHttpLinkHref() and a shared applyNewTabToAnchor()
  helper (http/https -> target+rel; mailto/tel/#/other -> strip target/rel).
- sanitizePlainTextRenderedHtml: re-apply target/rel after sanitization via an
  afterSanitizeAttributes hook (the established codebase pattern).
- sanitizeI18nHtml: same DOMPurify strip affected the in-app docs link in
  settings.security.not_available (target="_blank") - keep the translator's
  target and harden rel="noopener noreferrer".
- email-viewer: both anchor passes (DOMPurify hook + post-render walk) now use
  the shared applyNewTabToAnchor() helper.
- tests: unit coverage for isHttpLinkHref / applyNewTabToAnchor / sanitizeI18nHtml
  plus an end-to-end integration suite exercising the real plaintext and
  HTML/iframe render pipelines so this can't regress unnoticed.
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.

1 participant