fix: image alt on HTML-only translations and on markdown that starts with a tag

Blank content means content_html is the body (no 422 for
content_html-only translations, a regression from the markdown alt fix);
any other content is handled as markdown, so a body opening with a tag
gets its inline image label and keeps its code samples.

quick-261104-xae, review WR-02.
This commit is contained in:
Jakub Zych
2026-11-05 00:19:28 +01:00
parent a41d537e52
commit cf13c606f8
3 changed files with 104 additions and 16 deletions

View File

@@ -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. - **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. - **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. - **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`, 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 `<img>` 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. - **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`. `content` is read as markdown, as the renderer reads it (an HTML body included, whose `<img>` is a raw image outside code), and gets the alt in its inline image `![alt](src)` (with or without a title, outside code; `classes.SetMarkdownImageAlt`) or a raw `<img>` outside code (`classes.SetImageAltOutsideCode`), also when the markdown starts with an HTML tag, so the next content save keeps the alt; a locale whose `content` is blank (a translation that carries only `content_html`) gets the alt in `content_html` alone. 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 ### Related posts

View File

@@ -97,12 +97,13 @@ var errForbiddenPost = errors.New("forbidden post")
// [{post_id, locale, src, alt}]}: it sets the alt attribute of the img whose // [{post_id, locale, src, alt}]}: it sets the alt attribute of the img whose
// src equals src in the post's content_html, in the default locale when // 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 // 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 // decorative image. The post's content gets it too, read as markdown (an
// holds the same img; a markdown body in its inline image ![alt](src) or a // HTML body included): its inline image ![alt](src) or a raw img outside
// raw img outside code (classes.SetMarkdownImageAlt, // code (classes.SetMarkdownImageAlt, classes.SetImageAltOutsideCode), so
// classes.SetImageAltOutsideCode), so the next content save keeps it. An // the next content save keeps it. An image the markdown does not hold that
// image the markdown does not hold that way (a reference-style image, for // way (a reference-style image, for example) refuses the change with 422
// example) refuses the change with 422 changes.N.src. It needs // changes.N.src. A locale whose content is blank (a translation that
// carries only content_html) gets the alt in content_html alone. It needs
// golem15.journal.access_posts and may edit only the posts the caller may // 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 // 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 // not in the post refuses it with 422 (errors keyed changes.N.field). It
@@ -169,15 +170,22 @@ func SetImageAlt(app *backpack.App) http.HandlerFunc {
verrs = addSettingsError(verrs, fmt.Sprintf("changes.%d.src", i), "The image is not in the post.") verrs = addSettingsError(verrs, fmt.Sprintf("changes.%d.src", i), "The image is not in the post.")
continue continue
} }
if classes.LooksLikeHTML(text.content) { switch {
// An HTML body (the rich editor's) holds the same img. case strings.TrimSpace(text.content) == "":
if content, m, _ := classes.SetImageAlt(text.content, c.src, c.alt); m > 0 { // content_html is the body (a translation that holds
text.content = content // only content_html, or a row without content): nothing
} // else to keep in sync (33-REVIEW-GAPFIX WR-02).
} else { default:
// A markdown body: the next save derives content_html // content is read as markdown, as FormatHTML renders it
// from it again, so the alt must land in the markdown // on every API path, and the next save derives
// (L1 WR-05): an inline image, or a raw img outside code. // content_html from it again, so the alt must land in it
// (L1 WR-05): an inline image, or a raw img outside
// code. An HTML body (the rich editor's) is HTML blocks
// to goldmark, so its img is a raw img outside code and
// gets the alt here too. The first character no longer
// picks the branch, so markdown that opens with a tag
// keeps its code samples and is never answered changed
// and reverted later.
content, m, _ := classes.SetMarkdownImageAlt(text.content, c.src, c.alt) content, m, _ := classes.SetMarkdownImageAlt(text.content, c.src, c.alt)
content, r, _ := classes.SetImageAltOutsideCode(content, c.src, c.alt) content, r, _ := classes.SetImageAltOutsideCode(content, c.src, c.alt)
if m+r == 0 { if m+r == 0 {

View File

@@ -723,6 +723,86 @@ func TestSetImageAlt(t *testing.T) {
} }
} }
// TestSetImageAltBodyShapes pins the body rule of images/alt
// (33-REVIEW-GAPFIX WR-02): a locale whose content is blank (a translation
// that carries only content_html) gets the alt in content_html alone, and
// any other content is read as markdown, also when it starts with a tag.
func TestSetImageAltBodyShapes(t *testing.T) {
env := newPostsEnv(t)
editor := env.admin(t, controllers.PermissionAccessPosts, controllers.PermissionAccessOtherPosts)
const a = "https://cdn.example.test/a.jpg"
path := journalAPIPrefix + "/images/alt"
decode := func(rec *httptest.ResponseRecorder) []map[string]any {
var out struct {
Changes []map[string]any `json:"changes"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil {
t.Fatalf("alt body %s: %v", rec.Body.String(), err)
}
return out.Changes
}
t.Run("a translation that holds only content_html", func(t *testing.T) {
post := apiPostID(t, env.expect(t, http.StatusCreated, http.MethodPost, journalAPIPrefix+"/posts", editor, map[string]any{
"title": "HTML translation", "content": "Intro\n\n![](" + a + ")",
"translations": map[string]any{"pl": map[string]any{"title": "Obrazy", "content_html": `<p><img src="` + a + `"></p>`}},
}))
changes := decode(env.expect(t, http.StatusOK, http.MethodPost, path, editor, map[string]any{
"changes": []any{
map[string]any{"post_id": post, "src": a, "alt": "Grzyb"},
map[string]any{"post_id": post, "locale": "pl", "src": a, "alt": "Lew"},
},
}))
if len(changes) != 2 || changes[1]["locale"] != "pl" || changes[1]["changed"] != true || changes[1]["old_alt"] != nil {
t.Fatalf("answer = %#v", changes)
}
pl := translatedAttrs(t, env, post, "pl")
if html, _ := pl["content_html"].(string); !strings.Contains(html, `alt="Lew"`) {
t.Fatalf("pl = %#v", pl)
}
if c, has := pl["content"]; has {
if s, _ := c.(string); strings.TrimSpace(s) != "" {
t.Fatalf("pl content was written: %#v", pl)
}
}
if got := storedPost(t, env, post).Content; got != "Intro\n\n![Grzyb]("+a+")" {
t.Fatalf("default content = %q", got)
}
})
t.Run("markdown that starts with a tag", func(t *testing.T) {
md := "<p class=\"lead\">Intro</p>\n\n![](" + a + ")\n\n```html\n<img src=\"" + a + "\">\n```\n"
post := apiPostID(t, env.expect(t, http.StatusCreated, http.MethodPost, journalAPIPrefix+"/posts", editor, map[string]any{"title": "Tag lead", "content": md}))
env.expect(t, http.StatusOK, http.MethodPost, path, editor, map[string]any{
"changes": []any{map[string]any{"post_id": post, "src": a, "alt": "Grzyb"}},
})
want := "<p class=\"lead\">Intro</p>\n\n![Grzyb](" + a + ")\n\n```html\n<img src=\"" + a + "\">\n```\n"
stored := storedPost(t, env, post)
if stored.Content != want || !strings.Contains(stored.ContentHTML, `alt="Grzyb"`) {
t.Fatalf("after the alt = %q / %q", stored.Content, stored.ContentHTML)
}
env.expect(t, http.StatusOK, http.MethodPut, fmt.Sprintf("%s/posts/%d", journalAPIPrefix, post), editor, map[string]any{"content": stored.Content})
if again := storedPost(t, env, post); !strings.Contains(again.ContentHTML, `alt="Grzyb"`) {
t.Fatalf("the next content save lost the alt: %q", again.ContentHTML)
}
})
t.Run("markdown that starts with a tag and holds the image by reference", func(t *testing.T) {
ref := "<p>Lead</p>\n\n![old][r]\n\n[r]: " + a + "\n"
post := apiPostID(t, env.expect(t, http.StatusCreated, http.MethodPost, journalAPIPrefix+"/posts", editor, map[string]any{"title": "Tag lead reference", "content": ref}))
before := storedPost(t, env, post)
rec := env.expect(t, http.StatusUnprocessableEntity, http.MethodPost, path, editor, map[string]any{
"changes": []any{map[string]any{"post_id": post, "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 answer %s", rec.Body.String())
}
if after := storedPost(t, env, post); after.Content != before.Content || after.ContentHTML != before.ContentHTML {
t.Fatalf("a refused change wrote: %q / %q", after.Content, after.ContentHTML)
}
})
}
// TestBlogEditorRequestOptions (J1, UI-SPEC C28): the post and category // TestBlogEditorRequestOptions (J1, UI-SPEC C28): the post and category
// editor fields serve headingLevels and genericAnchors per form schema // editor fields serve headingLevels and genericAnchors per form schema
// request, following the H1 setting and the settings' generic link texts. // request, following the H1 setting and the settings' generic link texts.