diff --git a/docs/content/docs/guides/sequences.mdx b/docs/content/docs/guides/sequences.mdx index 5f409df9..8f67b6b0 100644 --- a/docs/content/docs/guides/sequences.mdx +++ b/docs/content/docs/guides/sequences.mdx @@ -52,7 +52,7 @@ Cold email from a real person rarely has images. An image-heavy body reads as a #### Writing the HTML yourself -The `` button on the right of the toolbar swaps the body for its HTML. What you type there is byte for byte what the step sends, merge fields and conditions included, so a template built elsewhere sends exactly as its designer wrote it. +The `` button on the right of the toolbar swaps the body for its HTML. What you type there is stored exactly as you wrote it, merge fields and conditions included, so a template built elsewhere keeps its markup. The only thing that changes on the way out is what the send path adds or resolves: merge fields fill in, a `

hi

") + if !strings.Contains(out, "@weird-at-rule-we-do-not-know") { + t.Errorf("unreadable text was deleted once another rule inlined:\n%s", out) + } + if !strings.Contains(out, `style="color: red"`) { + t.Errorf("the readable rule did not inline:\n%s", out) + } +} + +// The editor promises inlining from this finding, so it must not claim it for +// a sheet InlineCSS will leave exactly as written. +func TestLintPromisesInliningOnlyForSheetsThatGetIt(t *testing.T) { + has := func(html string) bool { + for _, f := range Lint(html, 500) { + if f.Code == "stylesheet_inlined" { + return true + } + } + return false + } + if !has(`

x

`) { + t.Error("an ordinary stylesheet is inlined and should say so") + } + if has(`

x

`) { + t.Error("a print stylesheet is never inlined") + } + if has(`

x

`) { + t.Error("an opted-out stylesheet is never inlined") + } +} diff --git a/internal/pkg/warmlint/lint.go b/internal/pkg/warmlint/lint.go index 92ae66c8..619fe17f 100644 --- a/internal/pkg/warmlint/lint.go +++ b/internal/pkg/warmlint/lint.go @@ -60,7 +60,7 @@ func Check(subject, body string, isReply bool) error { if stackedPunct.MatchString(combined) { return fmt.Errorf("stacked punctuation") } - if n := countTriggerTerms(combined); n >= 3 { + if n := countTriggerTerms(withoutURLs(combined)); n >= 3 { return fmt.Errorf("content has %d spam-trigger terms", n) } return nil @@ -106,7 +106,7 @@ func Score(subject, bodyHTML, bodyPlain string) ScoreResult { if stackedPunct.MatchString(combined) { deduct(10, "warn", "stacked_punctuation", "Stacked punctuation (e.g. !!! or ?!) reads as promotional.") } - if n := countTriggerTerms(combined); n > 0 { + if n := countTriggerTerms(withoutURLs(combined)); n > 0 { d := n * 8 if d > 40 { d = 40 @@ -177,6 +177,14 @@ func ScoreWithAttachments(subject, bodyHTML, bodyPlain string, attachments int) // It renders rather than strips tags: a regex left a

Hi Ana, ten minutes on Thursday?

`, "") + if styled.Score != clean.Score { + t.Errorf("a stylesheet changed the content score: %d vs %d (%v)", styled.Score, clean.Score, styled.Issues) + } +} diff --git a/internal/tasks/preview.go b/internal/tasks/preview.go index c9383af9..00bea8a8 100644 --- a/internal/tasks/preview.go +++ b/internal/tasks/preview.go @@ -70,8 +70,12 @@ func (s *tasksService) PreviewEmail(ctx context.Context, orgID uuid.UUID, in Ema out := &EmailPreview{TemplatePreview: previewTemplatesWith(in.Subject, in.BodyHTML, in.BodyPlain, in.Contact, unsubURL)} out.BodyHTML, out.BodyPlain = finishBody(out.BodyHTML, out.BodyPlain, textOnly, in.Account, optOut, unsubURL) // Linted on what the author wrote, sized on what ships: the findings have - // to name the markup they can go and fix, but Gmail measures the wire. - out.HTMLFindings = mailhtml.Lint(in.BodyHTML, len(out.BodyHTML)) + // to name the markup they can go and fix, but Gmail measures the wire. A + // plain-text campaign sends no HTML part at all, so there is no client + // left to be incompatible with and the notes would only be noise. + if !textOnly { + out.HTMLFindings = mailhtml.Lint(in.BodyHTML, len(out.BodyHTML)) + } if in.Account != nil { out.From = &EmailPreviewFrom{Name: strings.TrimSpace(in.Account.Name), Email: in.Account.Email} diff --git a/web/src/components/app/campaigns/sequences/EmailContentEditor.tsx b/web/src/components/app/campaigns/sequences/EmailContentEditor.tsx index a8e30232..f2eddc74 100644 --- a/web/src/components/app/campaigns/sequences/EmailContentEditor.tsx +++ b/web/src/components/app/campaigns/sequences/EmailContentEditor.tsx @@ -92,6 +92,16 @@ export default function EmailContentEditor({ const code = onBodyCodeChange ? bodyCode : localCode; const setCode = onBodyCodeChange ?? setLocalCode; + // A body can also become a document after mount: applying a template + // replaces it wholesale. Whichever mode the step is in, it has to switch + // before the editor parses that markup through its schema, or the next + // visual edit saves the gutted version. The visual editor cannot produce + // document markup itself, so this only ever fires on a body from outside. + React.useEffect(() => { + if (!code && isDocumentBody(bodyHtml)) setCode(true); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [bodyHtml, code]); + // Preview context: null contact = the built-in sample; the mailbox defaults // to the campaign's first enabled sender once the pool has loaded. const [previewContact, setPreviewContact] = React.useState(null); diff --git a/web/src/components/app/campaigns/sequences/RichTextEditor.tsx b/web/src/components/app/campaigns/sequences/RichTextEditor.tsx index aec42481..6a2b0d53 100644 --- a/web/src/components/app/campaigns/sequences/RichTextEditor.tsx +++ b/web/src/components/app/campaigns/sequences/RichTextEditor.tsx @@ -381,14 +381,18 @@ function prettyHTML(html: string): string { // The tags the visual editor's schema can hold. Anything else in HTML mode is // dropped the moment the editor parses it, so the user is told which ones -// before that happens rather than after. Tables, divs and styled spans are in -// the schema now; what is left is the machinery of a whole document, which no -// editor schema can be faithful to. +// before that happens rather than after. +// +// This list has to match the extensions actually mounted above, or the warning +// stays silent while the switch destroys something. Headings are configured to +// levels 2 and 3, so h1 and h4-h6 become paragraphs. Nothing mounted parses +// font or center. The table extensions know only table, tr, td and th: thead +// and tfoot lose their section, colgroup and col are dropped, and a caption +// comes back as an extra row. const SCHEMA_TAGS = new Set([ "p", "br", "strong", "b", "em", "i", "u", "s", "strike", "del", - "h1", "h2", "h3", "h4", "h5", "h6", "ul", "ol", "li", "a", "img", - "span", "div", "font", "center", - "table", "thead", "tbody", "tfoot", "tr", "td", "th", "caption", "colgroup", "col", + "h2", "h3", "ul", "ol", "li", "a", "img", "span", "div", + "table", "tbody", "tr", "td", "th", ]); function unsupportedTags(html: string): string[] { diff --git a/web/src/components/app/campaigns/sequences/pasteHtml.ts b/web/src/components/app/campaigns/sequences/pasteHtml.ts index d6ca3063..5860ed40 100644 --- a/web/src/components/app/campaigns/sequences/pasteHtml.ts +++ b/web/src/components/app/campaigns/sequences/pasteHtml.ts @@ -109,6 +109,21 @@ const DEFAULT_TEXT_COLOURS = new Set([ "rgb(0,0,0)", "rgb(17,17,17)", "rgb(34,34,34)", "rgb(51,51,51)", ]); +// isColourValue accepts a single colour token: a name, a hex code, or one +// functional form. Anything with a second top-level token is a shorthand +// carrying more than a colour. +function isColourValue(value: string): boolean { + const v = value.trim(); + if (!v || /url\(|gradient/i.test(v)) return false; + let depth = 0; + for (const ch of v) { + if (ch === "(") depth++; + else if (ch === ")") depth = Math.max(0, depth - 1); + else if (/\s/.test(ch) && depth === 0) return false; + } + return true; +} + function isDefaultColour(value: string): boolean { return DEFAULT_TEXT_COLOURS.has(value.replace(/\s+/g, "").toLowerCase()); } @@ -124,9 +139,11 @@ function keptInlineStyle(el: Element): string { const value = decl.slice(at + 1).trim(); if (!prop || !value) continue; if (prop === "color" && isDefaultColour(value)) continue; - // A background shorthand holding an image or a gradient is not a - // highlight, and the schema has nowhere to put it. - if (prop === "background-color" && /url\(|gradient/i.test(value)) continue; + // The background shorthand is only a highlight when it is nothing but + // a colour. Word and Outlook paste "background: yellow none repeat + // scroll 0% 0%", and copying that whole value into the longhand writes + // a declaration every client drops, losing the highlight entirely. + if (prop === "background-color" && !isColourValue(value)) continue; kept.set(prop, value); } // says the same thing in the older spelling.