feat(signature): add HTML signature support - #10876
Conversation
|
Missing report label. Set exactly one of: |
|
Thank you for this @joshua-grimmett - very cool stuff. |
|
hey @joshua-grimmett, we haven't forgotten about this and appreciate you doing this (especially since it's one of our main roadmap items this year). @rafaeltonholo was put on some last minute changes for our Thundermail project, but will be back on this in the next week or so. |
|
No worries, @ryanleesipes glad to contribute. I love the work you guys are doing. |
|
Thanks @jbott-tbird, look forward to seeing how it goes! |
rafaeltonholo
left a comment
There was a problem hiding this comment.
Hi @joshua-grimmett, thanks for this contribution and sorry for the delay in reviewing this PR.
Before merging this, we have some points we need to address first.
| // multi-identity accounts preserve each identity's setting. If no match, fall back | ||
| // to the currently-loaded default identity. | ||
| Identity matchedIdentity = null; | ||
| String draftEmail = newIdentity.getEmail(); |
There was a problem hiding this comment.
nit: could be final
| String draftEmail = newIdentity.getEmail(); | |
| final String draftEmail = newIdentity.getEmail(); |
| // Ignore — fall back to default identity below. | ||
| } | ||
| } | ||
| boolean signatureIsHtml = matchedIdentity != null |
There was a problem hiding this comment.
nit: could be final
| boolean signatureIsHtml = matchedIdentity != null | |
| final boolean signatureIsHtml = matchedIdentity != null |
|
Hi @joshua-grimmett, thank you again for working on this. It is a feature on our roadmap for this year, and I would love to get this in. Are you able to make the change that @rafaeltonholo suggested? We would love to work with you on getting this through! |
|
@joshua-grimmett thank you for your work! Are there any ways to support this PR? |
|
Hi! I’m interested in helping get HTML signature support completed and merged into Thunderbird for Android. I’d be happy to contribute code to this PR, address outstanding review comments, add or improve tests, and help with any remaining work needed to make the feature ready for merging. Please let me know if it would be helpful for me to pick up any specific remaining tasks on this PR, or if you’re happy for me to start working through the open review feedback. |
Hi @apidemy, Unfortunately, I have already begun working on this PR to ensure we can deliver it as planned. However, we will soon have additional items related to HTML signature support, and we would be glad to have your assistance if you're still interested. |
Add a per-identity "Format signature as HTML" toggle so users can set rich-text signatures and have them sent as HTML in outgoing mail, addressing issue thunderbird#8841. - Identity.signatureIsHtml persisted alongside the existing signature fields; LegacyAccountStorageHandler reads/writes/deletes the new key; IdentitySettingsDescriptions bumps to settings version 111. - Edit Identity and Account Composition defaults screens gain a "Format signature as HTML" checkbox. - TextBodyBuilder short-circuits HtmlConverter.textToHtmlFragment when the flag is set, and runs the signature through HtmlConverter.htmlToText for the plain-text message path. - HtmlSignatureSanitizer sanitizes user HTML via a Jsoup Safelist.relaxed baseline, stripping scripts, event handlers, and javascript: URLs. - MessageCompose renders the HTML signature in a MessageWebView preview instead of raw text, so drafts and new composes show the rendered result. Remote images and natural device-size rendering are enabled for the user's own signature content. - Draft reload looks up the matching identity by email via account.findIdentity() to preserve signatureIsHtml for multi-identity accounts, since the X-K9mail-Identity header does not encode the flag. - Tests: HtmlSignatureSanitizerTest (10 cases) and TextBodyBuilderHtmlSignatureTest (4 cases).
de45233 to
fdc3210
Compare
- Convert legacy tests to use AAA pattern with explicit sections - Replace inline assertions with descriptive test names using backticks - Add testSubject property for clarity - Reorder tests logically (basic → allowed → removed content) - Add language hints to HTML string literals - Add missing assertions for sanitize tests - Replace mock() with FakeGeneralSettingsManager in TextBodyBuilderHtmlSignatureTest
… implementation - Extract HtmlSignatureSanitizer to interface in feature.mail.message.composer - Move implementation from legacy.core to DefaultHtmlSignatureSanitizer - Replace jsoup with ksoup for Kotlin Multiplatform compatibility - Inject HtmlSignatureSanitizer via DI into MessageBuilder and TextBodyBuilder - Move tests to new package and convert to kotlin.test
…essageWebView - Add ThunderbirdWebViewSettings interface in webkit api module - Move WebViewConfig from legacy.ui.legacy to core.android.webkit.api - Deprecate legacy WebViewConfig with replacement directive - Make MessageWebView implement ThunderbirdWebViewSettings - Expose loadWithOverviewMode property in interface - Add KDoc to interface methods
…to reusable function - Move SignaturePreviewWebView configuration logic from MessageCompose to new SignaturePreviewWebView.kt - Add configureForSignaturePreview extension function on ThunderbirdWebViewSettings - Include KDoc explaining network data unblocking and overview mode behavior - Add comprehensive tests covering config application and signature-specific overrides
… content - Replace manual HTML document construction with displayHtml.wrapMessageContent - Remove duplicate HtmlSignatureSanitizer import - Remove fully qualified class names for MessageWebView and WebViewConfigProvider
- Add saveSignatureAsHtml to AccountSetupComposition state and OnFormatSignatureAsHtmlCheck event, persisted via LegacyAccount.signatureIsHtml. - Render a live signature preview with RichText when a signature is set. - Pass the whole state object to AccountSetupCompositionScreen instead of individual parameters. - Replace the hand-rolled checkbox row with CheckboxInput and adjust signature labels.
… header - Add SIGNATURE_IS_HTML field to IdentityField enum - Persist signatureIsHtml flag in identity blob when signature is customized - Parse and apply signature HTML setting from identity header on draft restore - Remove workaround that matched identity by email to infer HTML signature setting
…on and edit identity - Extract LegacyIdentitySignatureWebViewConfigurator to share WebView config and HTML preview logic - Add signature preview WebView to EditIdentity layout with debounced updates - Adjust AccountSetupComposition signature UI spacing and label styling - Remove unused account_setup_composition.xml layout file - Add edit_identity_screen dimension resources for consistent padding
fdc3210 to
fc71357
Compare
I've applied all the required changes
dani-zilla
left a comment
There was a problem hiding this comment.
Looks good! Tested locally and sent some beautiful HTML signatures to test it out. Great work!
|
|
||
| @Test | ||
| fun `sanitize should return empty string when input is empty`() { | ||
| // Arrange |
There was a problem hiding this comment.
Is there a reason for the arrange/act/assert labels and language as well? Feels like something AI may have done, but I don't know if it has value to analog intelligence 😆
There was a problem hiding this comment.
I've always used the comments Arrange/Act/Assert to organize the test cases by blocks, so it is easier to understand:
- Where I'm preparing the test case (Arrange)
- Where I'm actually triggering the test subject action (Act), and
- Where I'm verifying the test subject output (Assert).
This is a well-known test pattern that predates AI; see more: "Arrange-Act-Assert" and Arrange-Act-Assert: A Pattern for Writing Good Tests
Regarding the language feature, it is just my OCD of seeing code inside a string and not having it show in the syntax colour. The language comment makes AS understand that this is code and show the syntax colour.
|
Thanks for your contribution! Your pull request has been merged and will be part of Thunderbird 24. We appreciate the time and effort you put into improving Thunderbird. If you haven’t already, you’re welcome to join our Matrix chat for contributors. It’s where we discuss development and help each other out. https://matrix.to/#/#tb-android-dev:mozilla.org |

Closes #11405
Summary
Adds a per-identity "Format signature as HTML" toggle so users can write rich-text signatures and have them sent as HTML in outgoing mail. Refs #8841.
When the flag is on, the signature is embedded verbatim in outgoing HTML messages (no text-to-HTML conversion), converted via Jsoup for outgoing plain-text messages, and rendered as a live WebView preview in the compose screen so drafts and new composes show the rendered result instead of raw tags.
Scope (vs #8841)
In scope ✅
Stretch (not in this PR)
Out of scope per #8841
Changes
Identity.signatureIsHtml: Booleanadded, persisted viaLegacyAccountStorageHandlerunder keysignatureIsHtml.$ident.IdentitySettingsDescriptionsregisters the new setting at version 111.TextBodyBuilder.getSignatureHtml()short-circuitsHtmlConverter.textToHtmlFragment()when the flag is set. For plain-text messages,TextBodyBuilder.getSignature()runs the HTML signature throughHtmlConverter.htmlToText()to produce a plain-text fallback.HtmlSignatureSanitizerwrapsJsoup.clean()with aSafelist.relaxed()baseline that strips<script>, event handler attributes,javascript:URLs,<iframe>,<object>, and<embed>elements.MessageComposerenders the HTML signature in aMessageWebView(reusing the component already used for quoted HTML) instead of showing raw tags in the plainEditText. Remote images and natural device-size rendering are enabled since the signature is the user's own content.account.findIdentity(email)looks up the matching account identity and inherits itssignatureIsHtmlvalue, preserving the flag across drafts for multi-identity accounts.Tests
HtmlSignatureSanitizerTest— 10 cases covering sanitisation (preserves basic formatting, strips scripts/event handlers /javascript:URLs / iframes, passes plain text unchanged).TextBodyBuilderHtmlSignatureTest— 4 cases covering the HTML short-circuit in both the HTML and plain-text body paths.TextBodyBuilderTest,MessageBuilderTest,ReplyToPresenterTest,IdentityHeaderBuilderTestupdated for the newIdentityfield and continue to pass../gradlew spotlessCheck detektpass locally.Screenshots
Edit Identity screen — new HTML checkbox:
Compose screen — rendered HTML signature preview:
Test plan
<script>/onclick/javascript:URLsHtmlConverter.htmlToText()