fix(16.2): WR-04 serve content_html for markdown that starts with a tag

LooksLikeHTML decided by the first character alone, so a markdown post
opening with a hero <img>, a <div> or a comment was served as raw
markdown and its first edit stored ## and ** markers as HTML in both
columns. Load now serves such a value as HTML only when content_html is
empty or missing, or holds that same HTML (the value or SanitizeHTML of
it, white space at the ends ignored); otherwise it serves content_html.

classes/blog_editor.go

@@ -65,9 +65,10 @@ var (
 // one. Both render BlogEditorElement. Decode stores what the HTML policy
 // keeps (SanitizeHTMLReport) and reports what it removed as notices; it
 // never refuses a string because of its markup. Load serves HTML content as
-// it is and a markdown value (one that does not start with '<') as the
-// rendered HTML of the htmlColumn option, and a blank value as the
-// htmlColumn attribute when that holds a body. Upload stores an image the editor
+// it is and a markdown value as the rendered HTML of the htmlColumn option,
+// and a blank value as the htmlColumn attribute when that holds a body; a
+// value that starts with '<' counts as HTML only when the htmlColumn
+// attribute is empty or holds that same HTML (loadBlogHTML). Upload stores an image the editor
 // inserts through StoreMedia, in the media bucket the app publishes. app is
 // asked for the booted application at request time; it may be nil in tests,
 // where Upload then fails.
@@ -107,9 +108,11 @@ func BlogEditorTypes(app func() *backpack.App) []pact.FormFieldType {
 	}
 }
 
-// LooksLikeHTML reports whether s is stored HTML rather than markdown: its
-// first character that is not white space is '<' (an HTML comment
-// included). Phase 16.2 assumption A1.
+// LooksLikeHTML reports whether s may be stored HTML rather than markdown:
+// its first character that is not white space is '<' (an HTML comment
+// included). Phase 16.2 assumption A1. Markdown can start with a tag too,
+// so Load also compares the value with its htmlColumn attribute before it
+// serves the value as HTML.
 func LooksLikeHTML(s string) bool {
 	t := strings.TrimLeftFunc(s, unicode.IsSpace)
 	return strings.HasPrefix(t, "<")
@@ -198,38 +201,68 @@ func removalNames(names []string, cut bool) string {
 	return joined
 }
 
-// loadBlogHTML serves a stored value to the editor (D-07). HTML content is
-// served as it is. A markdown value is served as its rendered HTML: the
-// htmlColumn attribute in the same locale, or FormatHTML of the value when
-// that is empty or cannot be read (the value itself when rendering fails).
-// A blank value is served as the htmlColumn attribute in the same locale
-// when that is not empty, so a locale that carries only content_html (an
-// API-written translation, FormatTranslation) opens with the body the site
-// renders instead of an empty editor whose first edit would replace it; it
-// is served as stored otherwise.
+// loadBlogHTML serves a stored value to the editor (D-07). Without the
+// htmlColumn option every value is served as stored. With it:
+//
+//   - A blank value is served as the htmlColumn attribute in the same locale
+//     when that is not empty, so a locale that carries only content_html (an
+//     API-written translation, FormatTranslation) opens with the body the
+//     site renders instead of an empty editor whose first edit would replace
+//     it; it is served as stored otherwise.
+//   - A value that looks like HTML (LooksLikeHTML) is served as it is when
+//     the htmlColumn attribute is empty or missing, or holds the same HTML:
+//     the value itself or the policy's output for it (SanitizeHTML), white
+//     space at either end ignored. Otherwise the value is markdown that
+//     merely starts with a tag (a hero <img>, a leading <div> or comment),
+//     and it is served as the htmlColumn attribute, so the first edit does
+//     not store raw markdown as HTML.
+//   - Any other value is markdown, served as the htmlColumn attribute, or as
+//     FormatHTML of the value when that is empty or cannot be read (the
+//     value itself when rendering fails).
 func loadBlogHTML(_ context.Context, in pact.FormFieldValue) (any, error) {
 	text := storedText(in.Value)
 	column, _ := in.Options["htmlColumn"].(string)
-	if column == "" || LooksLikeHTML(text) {
+	if column == "" {
 		return text, nil
 	}
+	html := ""
 	if in.Translated != nil {
-		html, found, err := in.Translated(column)
+		value, found, err := in.Translated(column)
 		if err != nil {
 			return nil, err
 		}
-		if found && html != "" {
-			return html, nil
+		if found {
+			html = value
 		}
 	}
-	if strings.TrimSpace(text) == "" {
+	switch {
+	case strings.TrimSpace(text) == "":
+		if html != "" {
+			return html, nil
+		}
 		return text, nil
+	case LooksLikeHTML(text):
+		if html == "" || sameStoredHTML(text, html) {
+			return text, nil
+		}
+		return html, nil
+	case html != "":
+		return html, nil
 	}
-	html, err := FormatHTML(text)
+	rendered, err := FormatHTML(text)
 	if err != nil {
 		return text, nil
 	}
-	return html, nil
+	return rendered, nil
+}
+
+// sameStoredHTML reports whether rendered, a stored htmlColumn value, is the
+// HTML text itself: equal to text or to the policy's output for it, with
+// white space at either end ignored. It is false when rendered came from
+// rendering text as markdown in a way that changed it.
+func sameStoredHTML(text, rendered string) bool {
+	rendered = strings.TrimSpace(rendered)
+	return rendered == strings.TrimSpace(text) || rendered == strings.TrimSpace(SanitizeHTML(text))
 }
 
 // storedText is a stored column value as text: a string, a *string (nil is

classes/blog_editor_test.go

@@ -57,15 +57,81 @@ func TestBlogEditorLoad(t *testing.T) {
 			"\n<!-- imported -->\n<p>both</p>\n",
 			"<style>.x{}</style><p>unsafe markup is served too; Decode strips it on save</p>",
 		} {
+			// content_html missing, empty, the stored HTML itself (an admin
+			// save), the policy's output for it, or either with other white
+			// space at the ends (FormatHTML trims and goldmark appends "\n").
+			for _, tc := range []struct {
+				name  string
+				html  string
+				found bool
+			}{
+				{"missing", "", false},
+				{"empty", "", true},
+				{"equal", stored, true},
+				{"sanitized", SanitizeHTML(stored), true},
+				{"trimmed", strings.TrimSpace(stored) + "\n", true},
+				{"sanitized and trimmed", "\n" + strings.TrimSpace(SanitizeHTML(stored)), true},
+			} {
+				got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{
+					Options: blogOptions, Locale: "pl", Default: true, Value: stored,
+					Translated: translatedReader(new([]string), tc.html, tc.found, nil),
+				})
+				if err != nil || got != stored {
+					t.Fatalf("Load(%q) with content_html %s = %q, %v; want the stored bytes", stored, tc.name, got, err)
+				}
+			}
 			got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{
-				Options: blogOptions, Locale: "pl", Default: true, Value: stored, Translated: noTranslated(t),
+				Options: blogOptions, Locale: "pl", Default: true, Value: stored,
 			})
 			if err != nil || got != stored {
-				t.Fatalf("Load(%q) = %q, %v; want the stored bytes", stored, got, err)
+				t.Fatalf("Load(%q) without a reader = %q, %v; want the stored bytes", stored, got, err)
 			}
 		}
 	})
 
+	// WR-04: markdown whose first character is '<' is not HTML when its
+	// content_html is a different rendering of it; serving the raw markdown
+	// would make the first edit store "##" and "**" as HTML.
+	t.Run("markdown that starts with a tag is served as content_html", func(t *testing.T) {
+		for _, source := range []string{
+			"<img src=\"/storage/hero.png\" alt=\"Hero\">\n\n## Nagłówek\n\nTekst z **pogrubieniem**.",
+			"<div class=\"lead\">Wstęp</div>\n\n## Nagłówek\n\n*kursywa*",
+			"<!-- imported -->\n\n## Heading\n\n**bold**",
+		} {
+			stored, err := FormatHTML(source)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !LooksLikeHTML(source) || sameStoredHTML(source, stored) {
+				t.Fatalf("fixture %q does not exercise WR-04 (content_html %q)", source, stored)
+			}
+			var asked []string
+			got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{
+				Options: blogOptions, Locale: "en", Value: source,
+				Translated: translatedReader(&asked, stored, true, nil),
+			})
+			if err != nil || got != stored {
+				t.Fatalf("Load(%q) = %q, %v; want content_html %q", source, got, err, stored)
+			}
+			if strings.Contains(got.(string), "**") || strings.Contains(got.(string), "## ") {
+				t.Fatalf("Load(%q) served markdown markers: %q", source, got)
+			}
+			if len(asked) != 1 || asked[0] != "content_html" {
+				t.Fatalf("asked %v, want [content_html]", asked)
+			}
+		}
+	})
+
+	t.Run("an HTML value with a failing content_html read is the Load error", func(t *testing.T) {
+		boom := errors.New("read failed")
+		_, err := loadBlogHTML(context.Background(), pact.FormFieldValue{
+			Options: blogOptions, Value: "<p>x</p>", Translated: translatedReader(new([]string), "", false, boom),
+		})
+		if !errors.Is(err, boom) {
+			t.Fatalf("err = %v, want %v", err, boom)
+		}
+	})
+
 	t.Run("markdown content is served as that locale's content_html", func(t *testing.T) {
 		var asked []string
 		got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{

controllers/posts_editor_smoke_test.go

@@ -202,15 +202,26 @@ func TestBlogEditorTracerSmoke(t *testing.T) {
 		}
 		return got
 	}
-	noRead := func(string) (string, bool, error) {
-		t.Fatal("Translated read for an HTML value")
-		return "", false, nil
-	}
-
 	t.Run("load serves HTML content as it is", func(t *testing.T) {
 		const html = "  <!-- lead -->\n<p>Stored <em>HTML</em></p>"
-		if got := load(t, html, noRead); got != html {
-			t.Fatalf("Load(HTML) = %q", got)
+		for _, stored := range []string{"", html, html + "\n"} {
+			if got := load(t, html, func(string) (string, bool, error) { return stored, true, nil }); got != html {
+				t.Fatalf("Load(HTML) with content_html %q = %q", stored, got)
+			}
+		}
+		if got := load(t, html, nil); got != html {
+			t.Fatalf("Load(HTML, no reader) = %q", got)
+		}
+	})
+
+	t.Run("load serves markdown that starts with a tag as content_html", func(t *testing.T) {
+		const source = "<img src=\"/storage/hero.png\" alt=\"Hero\">\n\n## Heading\n\n**bold**"
+		rendered, err := classes.FormatHTML(source)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got := load(t, source, func(string) (string, bool, error) { return rendered, true, nil }); got != rendered {
+			t.Fatalf("Load(markdown starting with <img>) = %q, want content_html %q", got, rendered)
 		}
 	})