From 7662d54ab3fea8965f5678245ff7e5fd2f3a286f Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 4 Nov 2026 23:20:02 +0100 Subject: [PATCH] fix: image alt texts on markdown posts are written into the markdown POST /_journal/api/v1/images/alt writes the alt of a markdown post into its source: the inline image ![alt](src) outside code (classes.SetMarkdownImageAlt) or a raw img outside code (classes.SetImageAltOutsideCode), so the next content save keeps it. An image the markdown does not hold that way is refused with 422 changes.N.src and nothing is written. --- README.md | 2 +- classes/image_alt.go | 335 +++++++++++++++++++++++++++++++++ classes/image_alt_test.go | 55 +++++- controllers/api/maintenance.go | 33 +++- headings_test.go | 30 +++ mcp.go | 2 +- 6 files changed, 449 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index bc5c8f9..12f0656 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ Journal decides per site whether its rich text may hold an H1 (2026-10-28, D-50) - **The API flag.** `GET /_journal/api/v1/posts/{slug}` and every category row of `GET /_journal/api/v1/categories` carry `content_h1_to_h2` (boolean, an added key), so a frontend reads the setting from the Journal and not from its own option. - **Generic link texts.** The `generic_anchors` setting (one per line; the English and Polish seed lists when empty, `classes.DefaultGenericAnchors`) is the list the editor's outline and link form warn about. - **Settings API.** `GET` and `PUT /_journal/api/v1/settings` read and write every settings column for `golem15.journal.manage_settings` (`403` without it); `PUT` checks the submitted columns against `models.Settings.Rules` and ignores other keys. -- **Image alt.** `POST /_journal/api/v1/images/alt` `{dry_run, changes: [{post_id, locale, src, alt}]}` sets the alt of the image whose `src` equals `src` in the post's `content_html` (and in `content` when that holds the same image), in the default locale or in `locale`; `alt: ""` marks it decorative. It needs `golem15.journal.access_posts` and edit rights on each post, refuses the whole batch with `422` when one image is not in its post, answers `{changes: [{post_id, locale, src, old_alt, alt, changed}], dry_run}` and fires a `RecordSaved` per changed post. Alt text is post content, so the bulk alt tool is Journal's (`journal_set_image_alt`), not the SEO plugin's. +- **Image alt.** `POST /_journal/api/v1/images/alt` `{dry_run, changes: [{post_id, locale, src, alt}]}` sets the alt of the image whose `src` equals `src` in the post's `content_html` and in `content`, in the default locale or in `locale`: an HTML body when it holds the same image; a markdown body in its inline image `![alt](src)` (with or without a title, outside code; `classes.SetMarkdownImageAlt`) or a raw `` outside code (`classes.SetImageAltOutsideCode`), so the next content save keeps the alt. An image the markdown does not hold that way (a reference-style image) refuses the change with `422` `changes.N.src` ("The image is not an inline image of the post's markdown; edit the post."); `alt: ""` marks it decorative. It needs `golem15.journal.access_posts` and edit rights on each post, refuses the whole batch with `422` when one image is not in its post, answers `{changes: [{post_id, locale, src, old_alt, alt, changed}], dry_run}` and fires a `RecordSaved` per changed post. Alt text is post content, so the bulk alt tool is Journal's (`journal_set_image_alt`), not the SEO plugin's. ### Related posts diff --git a/classes/image_alt.go b/classes/image_alt.go index 9f604c4..3a94de5 100644 --- a/classes/image_alt.go +++ b/classes/image_alt.go @@ -2,8 +2,12 @@ package classes import ( "bytes" + "sort" "strings" + "github.com/yuin/goldmark/ast" + gmtext "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" xhtml "golang.org/x/net/html" ) @@ -94,3 +98,334 @@ func setAlt(tok *xhtml.Token, alt string) *string { tok.Attr = attrs return prev } + +// SetMarkdownImageAlt sets the alt text of every inline markdown image of +// md whose destination is src (P33-65): ![label](dest) and +// ![label](dest "title"), outside fenced and indented code blocks, code +// spans and HTML blocks (the ranges come from goldmark's AST, the engine of +// FormatHTML). An image matches when its destination equals src as written, +// or as goldmark's HTML renderer escapes it into the img src attribute. The +// label is replaced by alt with \, [ and ] backslash-escaped and line breaks +// folded to spaces, so an alt can neither close the image nor add a link; +// every other byte of md is kept. It returns the new markdown, the number of +// matching images and the label the first match had. A reference-style +// image (![label][ref]) is not matched. +func SetMarkdownImageAlt(md, src, alt string) (string, int, *string) { + if src == "" || !strings.Contains(md, "![") { + return md, 0, nil + } + skip, images := markdownLayout(md) + if len(images) == 0 { + return md, 0, nil + } + label := escapeMarkdownLabel(alt) + var b strings.Builder + b.Grow(len(md) + len(label)) + matches := 0 + var old *string + last := 0 + for i := 0; i+1 < len(md); i++ { + if md[i] != '!' || md[i+1] != '[' || (i > 0 && md[i-1] == '\\') || inRanges(skip, i) { + continue + } + img, ok := scanInlineImage(md, i) + if !ok { + continue + } + dest := md[img.destStart:img.destEnd] + escaped := string(util.URLEscape([]byte(dest), true)) + if !images[dest] || (dest != src && escaped != src) { + i = img.end - 1 + continue + } + if matches == 0 { + v := md[img.labelStart:img.labelEnd] + old = &v + } + matches++ + b.WriteString(md[last:img.labelStart]) + b.WriteString(label) + last = img.labelEnd + i = img.end - 1 + } + if matches == 0 { + return md, 0, nil + } + b.WriteString(md[last:]) + return b.String(), matches, old +} + +// SetImageAltOutsideCode applies SetImageAlt to the parts of markdown md +// outside its fenced and indented code blocks and code spans, so a raw +// in the markdown gets the alt while an shown in a code sample +// keeps its bytes. +func SetImageAltOutsideCode(md, src, alt string) (string, int, *string) { + if src == "" || !containsFold(md, " 0 && matches == 0 { + old = prev + } + matches += n + b.WriteString(out) + } + for _, r := range code { + apply(md[last:r[0]]) + b.WriteString(md[r[0]:r[1]]) + last = r[1] + } + apply(md[last:]) + if matches == 0 { + return md, 0, nil + } + return b.String(), matches, old +} + +// escapeMarkdownLabel is alt as an image label: \, [ and ] escaped, line +// breaks folded to spaces. +func escapeMarkdownLabel(alt string) string { + alt = strings.NewReplacer("\r\n", " ", "\n", " ", "\r", " ").Replace(alt) + return strings.NewReplacer(`\`, `\\`, "[", `\[`, "]", `\]`).Replace(alt) +} + +// inlineImage is the byte layout of one ![label](dest "title") in a text. +type inlineImage struct { + labelStart, labelEnd int + destStart, destEnd int + end int +} + +// scanInlineImage reads the inline image starting at md[i] ("!["), as +// goldmark's link parser reads one: a label with balanced brackets and +// backslash escapes, "(", optional spaces, a destination in <...> or +// without spaces with balanced parentheses, an optional title in quotes or +// parentheses, and ")". +func scanInlineImage(md string, i int) (inlineImage, bool) { + var img inlineImage + img.labelStart = i + 2 + depth := 0 + j := img.labelStart + for ; j < len(md); j++ { + c := md[j] + if c == '\\' && j+1 < len(md) { + j++ + continue + } + if c == '[' { + depth++ + } else if c == ']' { + if depth == 0 { + break + } + depth-- + } + } + if j >= len(md) || j+1 >= len(md) || md[j+1] != '(' { + return img, false + } + img.labelEnd = j + k := skipLinkSpace(md, j+2) + if k >= len(md) { + return img, false + } + if md[k] == '<' { + e := k + 1 + for ; e < len(md) && md[e] != '>' && md[e] != '\n'; e++ { + if md[e] == '\\' && e+1 < len(md) { + e++ + } + } + if e >= len(md) || md[e] != '>' { + return img, false + } + img.destStart, img.destEnd = k+1, e + k = e + 1 + } else { + opened := 0 + e := k + for ; e < len(md); e++ { + c := md[e] + if c == '\\' && e+1 < len(md) { + e++ + continue + } + if c == '(' { + opened++ + } else if c == ')' { + if opened == 0 { + break + } + opened-- + } else if c == ' ' || c == '\t' || c == '\n' || c == '\r' { + break + } + } + if e == k { + return img, false + } + img.destStart, img.destEnd = k, e + k = e + } + k = skipLinkSpace(md, k) + if k < len(md) && (md[k] == '"' || md[k] == '\'' || md[k] == '(') { + closer := md[k] + if closer == '(' { + closer = ')' + } + e := k + 1 + for ; e < len(md) && md[e] != closer; e++ { + if md[e] == '\\' && e+1 < len(md) { + e++ + } + } + if e >= len(md) { + return img, false + } + k = skipLinkSpace(md, e+1) + } + if k >= len(md) || md[k] != ')' { + return img, false + } + img.end = k + 1 + return img, true +} + +// skipLinkSpace skips spaces, tabs and at most one line break. +func skipLinkSpace(md string, k int) int { + newline := false + for k < len(md) { + switch md[k] { + case ' ', '\t': + case '\n', '\r': + if newline { + return k + } + newline = true + default: + return k + } + k++ + } + return k +} + +// markdownLayout parses md as FormatHTML does and answers the byte ranges +// an image is never read from (code blocks, code spans and HTML blocks) +// and the destinations of its inline images. +func markdownLayout(md string) ([][2]int, map[string]bool) { + images := map[string]bool{} + ranges := markdownRanges(md, true, func(n ast.Node) { + if img, ok := n.(*ast.Image); ok { + images[string(img.Destination)] = true + } + }) + return ranges, images +} + +// markdownCodeRanges is the byte ranges of md's code blocks and code +// spans, in order. +func markdownCodeRanges(md string) [][2]int { + return markdownRanges(md, false, nil) +} + +// markdownRanges walks md's goldmark AST (parsed from the trimmed text, as +// FormatHTML converts it) and answers the sorted, merged byte ranges of its +// fenced and indented code blocks, its code spans and, with html, its HTML +// blocks; visit sees every node. +func markdownRanges(md string, html bool, visit func(ast.Node)) [][2]int { + trimmed := strings.TrimSpace(md) + lead := strings.Index(md, trimmed) + if trimmed == "" || lead < 0 { + return nil + } + source := []byte(trimmed) + doc := markdownEngine.Parser().Parse(gmtext.NewReader(source)) + var ranges [][2]int + lines := func(n ast.Node) { + l := n.Lines() + for i := 0; i < l.Len(); i++ { + s := l.At(i) + ranges = append(ranges, [2]int{lead + s.Start, lead + s.Stop}) + } + } + _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { + if !entering { + return ast.WalkContinue, nil + } + if visit != nil { + visit(n) + } + switch v := n.(type) { + case *ast.FencedCodeBlock: + lines(v) + if v.Info != nil { + ranges = append(ranges, [2]int{lead + v.Info.Segment.Start, lead + v.Info.Segment.Stop}) + } + return ast.WalkSkipChildren, nil + case *ast.CodeBlock: + lines(v) + return ast.WalkSkipChildren, nil + case *ast.HTMLBlock: + if html { + lines(v) + } + return ast.WalkSkipChildren, nil + case *ast.CodeSpan: + start, stop := -1, -1 + for c := v.FirstChild(); c != nil; c = c.NextSibling() { + if t, ok := c.(*ast.Text); ok { + if start < 0 || t.Segment.Start < start { + start = t.Segment.Start + } + stop = max(stop, t.Segment.Stop) + } + } + if start >= 0 { + // The backticks around the span belong to it. + for start > 0 && source[start-1] == '`' { + start-- + } + for stop < len(source) && source[stop] == '`' { + stop++ + } + ranges = append(ranges, [2]int{lead + start, lead + stop}) + } + return ast.WalkSkipChildren, nil + } + return ast.WalkContinue, nil + }) + sort.Slice(ranges, func(i, j int) bool { return ranges[i][0] < ranges[j][0] }) + var merged [][2]int + for _, r := range ranges { + if r[0] >= r[1] { + continue + } + if n := len(merged); n > 0 && r[0] <= merged[n-1][1] { + merged[n-1][1] = max(merged[n-1][1], r[1]) + continue + } + merged = append(merged, r) + } + return merged +} + +// inRanges reports whether i lies in one of the sorted ranges. +func inRanges(ranges [][2]int, i int) bool { + for _, r := range ranges { + if i < r[0] { + return false + } + if i < r[1] { + return true + } + } + return false +} diff --git a/classes/image_alt_test.go b/classes/image_alt_test.go index b12f81d..add98af 100644 --- a/classes/image_alt_test.go +++ b/classes/image_alt_test.go @@ -1,6 +1,9 @@ package classes -import "testing" +import ( + "strings" + "testing" +) // TestSetImageAlt pins the alt rewrite: only the matching img tag is // rewritten from its token, the alt is escaped, a missing alt is added and @@ -54,3 +57,53 @@ func TestSetImageAltEdges(t *testing.T) { t.Errorf("match with an unfinished tail = %q %d", out, n) } } + +// TestSetMarkdownImageAlt pins P33-65 (L1 WR-05): inline images with or +// without a title get the alt; a destination written unescaped matches the +// src goldmark renders for it; an alt with ] and a line break is escaped +// and folded; the same image in a fence or a code span is left alone; a +// reference-style image is not matched. +func TestSetMarkdownImageAlt(t *testing.T) { + out, n, old := SetMarkdownImageAlt("A ![](/a.jpg) and ![old](/a.jpg \"t\") and ![x](/b.jpg).", "/a.jpg", "Lion's mane") + if n != 2 || old == nil || *old != "" || out != "A ![Lion's mane](/a.jpg) and ![Lion's mane](/a.jpg \"t\") and ![x](/b.jpg)." { + t.Fatalf("plain = %q %d %v", out, n, old) + } + html, err := FormatHTML("![x]()") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(html, `src="/a%20b.jpg"`) { + t.Fatalf("goldmark src = %s", html) + } + if out, n, _ := SetMarkdownImageAlt("![x]()", "/a%20b.jpg", "y"); n != 1 || out != "![y]()" { + t.Fatalf("escaped src = %q %d", out, n) + } + if out, n, _ := SetMarkdownImageAlt("![x](/ż.jpg)", "/%C5%BC.jpg", "y"); n != 1 || out != "![y](/ż.jpg)" { + t.Fatalf("non-ASCII src = %q %d", out, n) + } + if out, _, _ := SetMarkdownImageAlt("![x](/a.jpg)", "/a.jpg", "a ] b\nc [d] \\e"); out != `![a \] b c \[d\] \\e](/a.jpg)` { + t.Fatalf("escaped alt = %q", out) + } + code := "```\n![x](/a.jpg)\n```\n\nInline `![x](/a.jpg)` and\n\n ![x](/a.jpg)\n" + if out, n, _ := SetMarkdownImageAlt(code, "/a.jpg", "y"); n != 0 || out != code { + t.Fatalf("code = %q %d", out, n) + } + ref := "![x][a]\n\n[a]: /a.jpg\n" + if out, n, _ := SetMarkdownImageAlt(ref, "/a.jpg", "y"); n != 0 || out != ref { + t.Fatalf("reference = %q %d", out, n) + } + // The rendered alt follows the markdown. + out, _, _ = SetMarkdownImageAlt("Text\n\n![](/a.jpg)\n", "/a.jpg", "Grzyb [leśny]") + if html, _ := FormatHTML(out); !strings.Contains(html, `alt="Grzyb [leśny]"`) { + t.Fatalf("rendered = %s", html) + } + + // SetImageAltOutsideCode: a raw img gets the alt, one in a code sample + // keeps its bytes. + raw := "\n\n```html\n\n```\n\nand ``\n" + got, n, _ := SetImageAltOutsideCode(raw, "/a.jpg", "z") + want := "\"z\"\n\n```html\n\n```\n\nand ``\n" + if n != 1 || got != want { + t.Fatalf("outside code = %q %d", got, n) + } +} diff --git a/controllers/api/maintenance.go b/controllers/api/maintenance.go index 5b6d08b..69ec16f 100644 --- a/controllers/api/maintenance.go +++ b/controllers/api/maintenance.go @@ -86,14 +86,23 @@ type imageAltText struct { oldContent, oldHTML string } +// errMarkdownImage refuses an alt for an image a markdown post's source +// does not hold as an inline image or a raw img. +const errMarkdownImage = "The image is not an inline image of the post's markdown; edit the post." + // errForbiddenPost stops a batch that names a post the caller may not edit. var errForbiddenPost = errors.New("forbidden post") // SetImageAlt is POST /_journal/api/v1/images/alt {dry_run, changes: // [{post_id, locale, src, alt}]}: it sets the alt attribute of the img whose -// src equals src in the post's content_html (and in content when that holds -// the same image), in the default locale when locale is empty and in that -// translation otherwise; alt "" marks a decorative image. It needs +// src equals src in the post's content_html, in the default locale when +// locale is empty and in that translation otherwise; alt "" marks a +// decorative image. The post's content gets it too: an HTML body when it +// holds the same img; a markdown body in its inline image ![alt](src) or a +// raw img outside code (classes.SetMarkdownImageAlt, +// classes.SetImageAltOutsideCode), so the next content save keeps it. An +// image the markdown does not hold that way (a reference-style image, for +// example) refuses the change with 422 changes.N.src. It needs // golem15.journal.access_posts and may edit only the posts the caller may // edit (403). The batch is all or nothing: a missing post or an src that is // not in the post refuses it with 422 (errors keyed changes.N.field). It @@ -160,10 +169,24 @@ func SetImageAlt(app *backpack.App) http.HandlerFunc { verrs = addSettingsError(verrs, fmt.Sprintf("changes.%d.src", i), "The image is not in the post.") continue } - text.contentHTML = html - if content, m, _ := classes.SetImageAlt(text.content, c.src, c.alt); m > 0 { + if classes.LooksLikeHTML(text.content) { + // An HTML body (the rich editor's) holds the same img. + if content, m, _ := classes.SetImageAlt(text.content, c.src, c.alt); m > 0 { + text.content = content + } + } else { + // A markdown body: the next save derives content_html + // from it again, so the alt must land in the markdown + // (L1 WR-05): an inline image, or a raw img outside code. + content, m, _ := classes.SetMarkdownImageAlt(text.content, c.src, c.alt) + content, r, _ := classes.SetImageAltOutsideCode(content, c.src, c.alt) + if m+r == 0 { + verrs = addSettingsError(verrs, fmt.Sprintf("changes.%d.src", i), errMarkdownImage) + continue + } text.content = content } + text.contentHTML = html var oldAlt any if old != nil { oldAlt = *old diff --git a/headings_test.go b/headings_test.go index ea818ba..99a1908 100644 --- a/headings_test.go +++ b/headings_test.go @@ -662,6 +662,36 @@ func TestSetImageAlt(t *testing.T) { if !found { t.Fatalf("no RecordSaved with the default and pl content_html: %#v", records.savedEvents()) } + + // L1 WR-05: on a markdown post the alt lands in the markdown, outside + // code, and survives the next save of the same content. + sample := "```\n![](" + a + ")\n```\n" + md := "Intro\n\n![](" + a + ")\n\n" + sample + mdPost := apiPostID(t, env.expect(t, http.StatusCreated, http.MethodPost, journalAPIPrefix+"/posts", editor, map[string]any{"title": "Markdown images", "content": md})) + env.expect(t, http.StatusOK, http.MethodPost, path, editor, map[string]any{ + "changes": []any{map[string]any{"post_id": mdPost, "src": a, "alt": "Grzyb [leśny]"}}, + }) + stored := storedPost(t, env, mdPost) + if stored.Content != "Intro\n\n![Grzyb \\[leśny\\]]("+a+")\n\n"+sample || !strings.Contains(stored.ContentHTML, `alt="Grzyb [leśny]"`) { + t.Fatalf("markdown post after the alt = %q / %q", stored.Content, stored.ContentHTML) + } + env.expect(t, http.StatusOK, http.MethodPut, fmt.Sprintf("%s/posts/%d", journalAPIPrefix, mdPost), editor, map[string]any{"content": stored.Content}) + if again := storedPost(t, env, mdPost); !strings.Contains(again.ContentHTML, `alt="Grzyb [leśny]"`) { + t.Fatalf("the next content save lost the alt: %q", again.ContentHTML) + } + // A reference-style image is not rewritten: 422, nothing written. + ref := "![old][r]\n\n[r]: " + a + "\n" + refPost := apiPostID(t, env.expect(t, http.StatusCreated, http.MethodPost, journalAPIPrefix+"/posts", editor, map[string]any{"title": "Reference image", "content": ref})) + before := storedPost(t, env, refPost) + rec = env.expect(t, http.StatusUnprocessableEntity, http.MethodPost, path, editor, map[string]any{ + "changes": []any{map[string]any{"post_id": refPost, "src": a, "alt": "new"}}, + }) + if !strings.Contains(rec.Body.String(), "changes.0.src") || !strings.Contains(rec.Body.String(), "not an inline image") { + t.Fatalf("reference-style answer %s", rec.Body.String()) + } + if after := storedPost(t, env, refPost); after.Content != before.Content || after.ContentHTML != before.ContentHTML { + t.Fatalf("a refused markdown change wrote: %q / %q", after.Content, after.ContentHTML) + } } // TestBlogEditorRequestOptions (J1, UI-SPEC C28): the post and category diff --git a/mcp.go b/mcp.go index 4abf7b9..cfc3788 100644 --- a/mcp.go +++ b/mcp.go @@ -229,7 +229,7 @@ func (p *Plugin) MCP() pact.MCP { { Name: "set_image_alt", Title: "Set image alt texts", - Description: "Sets the alt text of images in post content, several at once; an empty alt marks an image as decorative.", + Description: "Sets the alt text of images in post content, several at once; an empty alt marks an image as decorative. In a markdown post the alt is written into the markdown source (an inline image ![alt](src), or a raw img tag outside code); an image the markdown does not hold that way (a reference-style image) is refused with changes.N.src: edit that post instead.", Guard: mcpGuard, Method: http.MethodPost, Path: "/_journal/api/v1/images/alt",