From a2dd38f75f2d384c8d37229ad96b2dd79951a46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Aguilar?= Date: Tue, 25 Aug 2026 22:22:29 -0300 Subject: [PATCH 1/3] Render nested Action Text attachment content --- internal/htmlutil/htmlutil.go | 33 ++++++++++++++++++++++++++---- internal/htmlutil/htmlutil_test.go | 21 +++++++++++++++++++ internal/htmlutil/markdown.go | 6 ++++++ internal/htmlutil/markdown_test.go | 18 ++++++++++++++++ 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/internal/htmlutil/htmlutil.go b/internal/htmlutil/htmlutil.go index 4d69dcc30..a81dc5324 100644 --- a/internal/htmlutil/htmlutil.go +++ b/internal/htmlutil/htmlutil.go @@ -106,8 +106,9 @@ func walkNode(b *strings.Builder, n *html.Node, depth int) { if isImageContentType(getAttr(n, "content-type")) && isDecorativeImage(getAttr(n, "width"), getAttr(n, "height")) { return } - filename := getAttr(n, "filename") - if filename != "" { + if doc := parseEmbeddedActionText(n, depth); doc != nil { + walkNode(b, doc, depth+1) + } else if filename := getAttr(n, "filename"); filename != "" { fmt.Fprintf(b, "\n[%s]\n", filename) } return @@ -172,6 +173,13 @@ func walkMessageSourceNode(b *strings.Builder, n *html.Node, depth int) { case "hr": writeMessageSourceBoundary(b) return + case "action-text-attachment": + if doc := parseEmbeddedActionText(n, depth); doc != nil { + writeMessageSourceBoundary(b) + walkMessageSourceNode(b, doc, depth+1) + writeMessageSourceBoundary(b) + } + return case "template": if depth == 0 || n.Parent == nil || n.Parent.Data != "shadow-content" { return @@ -206,7 +214,7 @@ func elementProvidesMessageSourceText(n *html.Node) bool { return false } switch n.Data { - case "script", "style", "noscript", "head", "action-text-attachment": + case "script", "style", "noscript", "head": return false case "dialog": return hasAttr(n, "open") @@ -305,6 +313,21 @@ func parseTrixAttachment(n *html.Node) *trixAttachment { return &att } +func embeddedActionTextContent(n *html.Node) string { + if getAttr(n, "filename") != "" { + return "" + } + return getAttr(n, "content") +} + +func parseEmbeddedActionText(n *html.Node, depth int) *html.Node { + content := embeddedActionTextContent(n) + if content == "" { + return nil + } + return parseEmbeddedContent(content, depth) +} + func getAttr(n *html.Node, key string) string { for _, a := range n.Attr { if a.Key == key { @@ -455,7 +478,9 @@ func findImages(n *html.Node, urls *[]string, depth int) { if isImageContentType(getAttr(n, "content-type")) && isDecorativeImage(getAttr(n, "width"), getAttr(n, "height")) { return } - if imageURL := getAttr(n, "url"); isImageContentType(getAttr(n, "content-type")) && imageURL != "" { + if doc := parseEmbeddedActionText(n, depth); doc != nil { + findImages(doc, urls, depth+1) + } else if imageURL := getAttr(n, "url"); isImageContentType(getAttr(n, "content-type")) && imageURL != "" { *urls = append(*urls, imageURL) } case "figure": diff --git a/internal/htmlutil/htmlutil_test.go b/internal/htmlutil/htmlutil_test.go index 7ee423004..ebc096503 100644 --- a/internal/htmlutil/htmlutil_test.go +++ b/internal/htmlutil/htmlutil_test.go @@ -105,6 +105,13 @@ func TestMessageSourceTextIncludesEmbeddedEmailBody(t *testing.T) { } } +func TestMessageSourceTextIncludesEmbeddedActionTextAttachment(t *testing.T) { + html := `` + if got := strings.Join(strings.Fields(MessageSourceText(html)), " "); got != "External confirmation: BLUE-42" { + t.Errorf("MessageSourceText = %q", got) + } +} + func TestMessageSourceTextFailsClosedWhenHTMLExceedsParserDepth(t *testing.T) { html := strings.Repeat("
", 1_000) + "not selectable" + strings.Repeat("
", 1_000) if got := MessageSourceText(html); got != "" { @@ -176,6 +183,13 @@ func TestToTextActionTextAttachment(t *testing.T) { } } +func TestToTextRendersEmbeddedActionTextAttachment(t *testing.T) { + got := ToText(``) + if got != "Inside" { + t.Errorf("ToText = %q, want %q", got, "Inside") + } +} + func TestToTextTrixFigure(t *testing.T) { got := ToText(`

Before

After

`) if !strings.Contains(got, "[photo.png]") { @@ -204,6 +218,13 @@ func TestExtractImageURLsInsideEmbeddedHTMLAttachment(t *testing.T) { } } +func TestExtractImageURLsInsideEmbeddedActionTextAttachment(t *testing.T) { + urls := ExtractImageURLs(``) + if len(urls) != 1 || urls[0] != "https://example.com/logo.png" { + t.Errorf("ExtractImageURLs = %v, want the image inside the embedded body", urls) + } +} + func TestExtractAttachmentsSkipsEmbeddedHTMLAttachment(t *testing.T) { attachments := ExtractAttachments(`
`) if len(attachments) != 0 { diff --git a/internal/htmlutil/markdown.go b/internal/htmlutil/markdown.go index 158fb310b..779f33e0c 100644 --- a/internal/htmlutil/markdown.go +++ b/internal/htmlutil/markdown.go @@ -736,10 +736,16 @@ func (m *markdownizer) embedded(content string) { // dimensions, decoration beside the text that already says who or what they show. A // caption names an image its missing filename does not — the alt text of the // the node used to be. +// actionTextAttachment renders an unnamed attachment's inline content as embedded HTML +// and a named one as a file attachment. func (m *markdownizer) actionTextAttachment(n *html.Node) { if isImageContentType(getAttr(n, "content-type")) && isDecorativeImage(getAttr(n, "width"), getAttr(n, "height")) { return } + if content := embeddedActionTextContent(n); content != "" { + m.embedded(content) + return + } filename := getAttr(n, "filename") if filename == "" { filename = getAttr(n, "caption") diff --git a/internal/htmlutil/markdown_test.go b/internal/htmlutil/markdown_test.go index 2e1bc2e6e..4a5f8ac38 100644 --- a/internal/htmlutil/markdown_test.go +++ b/internal/htmlutil/markdown_test.go @@ -529,6 +529,24 @@ func TestToMarkdownEmbeddedHTMLAttachment(t *testing.T) { } } +// A nested action-text-attachment carries its HTML in a content attribute. +func TestToMarkdownEmbeddedActionTextAttachment(t *testing.T) { + got := toMarkdown(`
`) + + want := "Dear customer,\n\nPlease [sign the document](https://example.com/sign)." + if got != want { + t.Errorf("ToMarkdown = %q, want %q", got, want) + } +} + +func TestToMarkdownNamedActionTextAttachmentIgnoresInlineContent(t *testing.T) { + got := toMarkdown(``) + want := "📎 report.pdf" + if got != want { + t.Errorf("ToMarkdown = %q, want %q", got, want) + } +} + func TestToMarkdownEmbeddedContentStopsRecursing(t *testing.T) { nested := "

innermost

" for range embeddedContentDepthLimit + 1 { From 91d61fcb487fd645c93b9ad8e287406ae402a8f4 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 27 Oct 2026 00:17:52 -0400 Subject: [PATCH 2/3] Render nested attachment content only when it is HTML HEY renders an action-text-attachment's content attribute as HTML only when its content type is HTML (ActionText's ContentAttachment). Any other attachment is shown as itself. Rendering every unnamed attachment's content showed text HEY never displays, offered it as clip source text, and fetched an image out of the content in place of the attachment's own. Require text/html, as findAttachments already does. Update the named-PDF expectation for the document icon #532 introduced, and cover the depth limit for a chain of nested attachments. --- internal/htmlutil/htmlutil.go | 5 +++- internal/htmlutil/htmlutil_test.go | 44 ++++++++++++++++++++++++++++++ internal/htmlutil/markdown.go | 5 ++-- internal/htmlutil/markdown_test.go | 25 ++++++++++++++++- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/internal/htmlutil/htmlutil.go b/internal/htmlutil/htmlutil.go index a81dc5324..f710bdb49 100644 --- a/internal/htmlutil/htmlutil.go +++ b/internal/htmlutil/htmlutil.go @@ -313,8 +313,11 @@ func parseTrixAttachment(n *html.Node) *trixAttachment { return &att } +// embeddedActionTextContent returns the markup an unnamed text/html attachment carries +// in its content attribute. HEY renders any other attachment as itself, whatever +// content it carries. func embeddedActionTextContent(n *html.Node) string { - if getAttr(n, "filename") != "" { + if getAttr(n, "filename") != "" || !isHTMLContentType(getAttr(n, "content-type")) { return "" } return getAttr(n, "content") diff --git a/internal/htmlutil/htmlutil_test.go b/internal/htmlutil/htmlutil_test.go index ebc096503..cd5340aae 100644 --- a/internal/htmlutil/htmlutil_test.go +++ b/internal/htmlutil/htmlutil_test.go @@ -225,6 +225,50 @@ func TestExtractImageURLsInsideEmbeddedActionTextAttachment(t *testing.T) { } } +func embeddedActionTextAttachment(content string) string { + return `` +} + +func TestEmbeddedActionTextAttachmentStopsRecursing(t *testing.T) { + withinLimit := `

innermost

` + for range embeddedContentDepthLimit { + withinLimit = embeddedActionTextAttachment(withinLimit) + } + beyondLimit := embeddedActionTextAttachment(withinLimit) + + for name, text := range map[string]func(string) string{"ToText": ToText, "MessageSourceText": MessageSourceText} { + if got := text(withinLimit); !strings.Contains(got, "innermost") { + t.Errorf("%s = %q, want the body at the nesting limit", name, got) + } + if got := text(beyondLimit); strings.Contains(got, "innermost") { + t.Errorf("%s = %q, should stop before the body beyond the nesting limit", name, got) + } + } + if urls := ExtractImageURLs(withinLimit); len(urls) != 1 { + t.Errorf("ExtractImageURLs = %v, want the image at the nesting limit", urls) + } + if urls := ExtractImageURLs(beyondLimit); len(urls) != 0 { + t.Errorf("ExtractImageURLs = %v, should stop before the image beyond the nesting limit", urls) + } +} + +// HEY renders an attachment's content as HTML only when its content type is HTML; any +// other attachment is itself, whatever content it carries. +func TestNonHTMLActionTextAttachmentContentIsNotABody(t *testing.T) { + image := `` + if urls := ExtractImageURLs(image); len(urls) != 1 || urls[0] != "https://example.com/floor-plan.png" { + t.Errorf("ExtractImageURLs = %v, want the attachment's own image", urls) + } + + untyped := `

Visible

` + if got := ToText(untyped); strings.Contains(got, "not the body") { + t.Errorf("ToText = %q, should not render a non-HTML attachment's content", got) + } + if got := strings.Join(strings.Fields(MessageSourceText(untyped)), " "); got != "Visible" { + t.Errorf("MessageSourceText = %q, want only the visible text", got) + } +} + func TestExtractAttachmentsSkipsEmbeddedHTMLAttachment(t *testing.T) { attachments := ExtractAttachments(`
`) if len(attachments) != 0 { diff --git a/internal/htmlutil/markdown.go b/internal/htmlutil/markdown.go index 779f33e0c..a546d6e0c 100644 --- a/internal/htmlutil/markdown.go +++ b/internal/htmlutil/markdown.go @@ -730,14 +730,13 @@ func (m *markdownizer) embedded(content string) { m.depth-- } -// actionTextAttachment writes an attachment node — or nothing, for a decorative image. +// actionTextAttachment writes an attachment node — or nothing, for a decorative image, +// or the markup an unnamed text/html one carries inline, which is an email body. // HEY rewrites an inbound email's tags into these nodes, so a notification // email's avatars and icons arrive as image attachments declaring icon-sized // dimensions, decoration beside the text that already says who or what they show. A // caption names an image its missing filename does not — the alt text of the // the node used to be. -// actionTextAttachment renders an unnamed attachment's inline content as embedded HTML -// and a named one as a file attachment. func (m *markdownizer) actionTextAttachment(n *html.Node) { if isImageContentType(getAttr(n, "content-type")) && isDecorativeImage(getAttr(n, "width"), getAttr(n, "height")) { return diff --git a/internal/htmlutil/markdown_test.go b/internal/htmlutil/markdown_test.go index 4a5f8ac38..9ce687b8b 100644 --- a/internal/htmlutil/markdown_test.go +++ b/internal/htmlutil/markdown_test.go @@ -541,12 +541,35 @@ func TestToMarkdownEmbeddedActionTextAttachment(t *testing.T) { func TestToMarkdownNamedActionTextAttachmentIgnoresInlineContent(t *testing.T) { got := toMarkdown(``) - want := "📎 report.pdf" + want := "📄 report.pdf" if got != want { t.Errorf("ToMarkdown = %q, want %q", got, want) } } +// HEY renders an attachment's content as HTML only when its content type is HTML. +func TestToMarkdownNonHTMLActionTextAttachmentIgnoresInlineContent(t *testing.T) { + got := toMarkdown(``) + want := "![Floor plan](https://example.com/floor-plan.png)" + if got != want { + t.Errorf("ToMarkdown = %q, want %q", got, want) + } +} + +func TestToMarkdownEmbeddedActionTextAttachmentStopsRecursing(t *testing.T) { + withinLimit := "

innermost

" + for range embeddedContentDepthLimit { + withinLimit = embeddedActionTextAttachment(withinLimit) + } + if got := toMarkdown(withinLimit); got != "innermost" { + t.Errorf("ToMarkdown = %q, want the body at the nesting limit", got) + } + + if got := toMarkdown(embeddedActionTextAttachment(withinLimit)); strings.Contains(got, "innermost") { + t.Errorf("ToMarkdown = %q, should stop before the body beyond the nesting limit", got) + } +} + func TestToMarkdownEmbeddedContentStopsRecursing(t *testing.T) { nested := "

innermost

" for range embeddedContentDepthLimit + 1 { From c8b403bbaf2e2849914b17291fba3ee46aa69571 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 27 Oct 2026 01:45:24 -0400 Subject: [PATCH 3/3] Cover a named HTML attachment as a file The only named-file test used application/pdf, which the HTML type check rejects before the filename is consulted. Deleting the filename condition left every htmlutil test passing, so nothing held the rule that a named attachment stays a file even when it is text/html. Assert a named text/html attachment, with an image in its content, renders as its file row in Markdown and its filename in text, gives no message source text, and yields no image URL from its content. --- internal/htmlutil/htmlutil_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/htmlutil/htmlutil_test.go b/internal/htmlutil/htmlutil_test.go index cd5340aae..2966f2039 100644 --- a/internal/htmlutil/htmlutil_test.go +++ b/internal/htmlutil/htmlutil_test.go @@ -269,6 +269,24 @@ func TestNonHTMLActionTextAttachmentContentIsNotABody(t *testing.T) { } } +// Only an unnamed HTML attachment is an email body; a named one is a file, even when it is HTML. +func TestNamedHTMLActionTextAttachmentIsAFile(t *testing.T) { + file := `` + + if got := toMarkdown(file); got != "📄 itinerary.html" { + t.Errorf("ToMarkdown = %q, want the file row", got) + } + if got := ToText(file); got != "[itinerary.html]" { + t.Errorf("ToText = %q, want the filename", got) + } + if got := MessageSourceText(file); got != "" { + t.Errorf("MessageSourceText = %q, want no source text from a file", got) + } + if urls := ExtractImageURLs(file); len(urls) != 0 { + t.Errorf("ExtractImageURLs = %v, want no image from inside a file", urls) + } +} + func TestExtractAttachmentsSkipsEmbeddedHTMLAttachment(t *testing.T) { attachments := ExtractAttachments(`
`) if len(attachments) != 0 {