From bc03ee2c172f9911e0d3196cc74afd53b1ad49a5 Mon Sep 17 00:00:00 2001 From: Eric Bouchut Date: Mon, 13 Jul 2026 16:35:32 +0200 Subject: [PATCH] fix(course): Demote Markdown headings one level in lesson rendering The lesson page provides the single h1 (the lesson title), so an instructor's "# Heading" produced a second h1 and broke the one-h1, no-skipped-level heading hierarchy that docs/rgaa.md commits to for RGAA 9.1. A custom commonmark-java NodeRenderer now shifts every Markdown heading one HTML level down (h1 becomes h2, capped at h6) while keeping inline formatting inside the heading. The jsoup sanitization and the content-addressed cache of ADR-0013 are unchanged, so existing lessons pick the corrected levels up on their next render with no migration. The decision and its alternatives are recorded in ADR-0014. --- ...e-markdown-headings-in-lesson-rendering.md | 80 +++++++++++++++++++ docs/adr/README.md | 3 +- .../learndev/course/MarkdownRenderer.java | 49 +++++++++++- .../learndev/course/MarkdownRendererTest.java | 34 +++++++- 4 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 docs/adr/0014-demote-markdown-headings-in-lesson-rendering.md diff --git a/docs/adr/0014-demote-markdown-headings-in-lesson-rendering.md b/docs/adr/0014-demote-markdown-headings-in-lesson-rendering.md new file mode 100644 index 0000000..709aab4 --- /dev/null +++ b/docs/adr/0014-demote-markdown-headings-in-lesson-rendering.md @@ -0,0 +1,80 @@ +# Demote Markdown headings one level in lesson rendering + +- Status: accepted +- Date: 2026-07-13 +- Deciders: Eric Bouchut + +## Context and Problem Statement + +The lesson page renders the lesson title as the page `h1`, then emits the +instructor's Markdown as HTML (see +[ADR-0013](0013-render-lesson-markdown-with-commonmark-java.md)). Instructors +naturally start their content with `# Heading`, which CommonMark renders as a +second `h1`. Two `h1` elements on one page break the heading hierarchy that +docs/rgaa.md commits to for RGAA 9.1 (one `h1` per page, no skipped level), +and screen-reader users navigating by headings lose the page outline. How +does instructor-authored Markdown coexist with the page's own `h1`? + +## Decision Drivers + +- RGAA 9.1: exactly one `h1` per page, ordered heading levels +- Instructors should write natural Markdown, not remember a house rule +- The sanitization pipeline of ADR-0013 must stay unchanged +- Already-authored content must keep working without migration + +## Considered Options + +- Demote headings one level at render time (`#` becomes `h2`, capped at `h6`) +- Reject or lint Markdown that contains a level-1 heading at authoring time +- Strip `h1` from the sanitized output (jsoup allowlist change) +- Do nothing and document a "never use #" authoring convention + +## Decision Outcome + +Chosen: "demote headings one level at render time", because it makes the +accessible outcome structural instead of relying on author discipline: +whatever the instructor writes, the rendered fragment slots under the page's +`h1` with a correct hierarchy, and existing content is fixed retroactively +the next time it renders (the cache of ADR-0013 is content-addressed, so no +invalidation is needed). A custom commonmark-java `NodeRenderer` for +`Heading` shifts each level down by one, capping at `h6`; the jsoup +sanitization is untouched. + +Rejecting `#` at authoring time would surprise instructors and does nothing +for content pasted from elsewhere; stripping `h1` in the sanitizer would +silently delete content; a documented convention is the option that RGAA +audits exist to catch. + +### Consequences + +- Good: one `h1` per page holds on lesson pages by construction (RGAA 9.1) +- Good: no authoring rule to teach; pasted Markdown behaves sensibly +- Good: no schema or content migration; re-rendering applies the new levels +- Trade-off: authored `######` (h6) and `#####` (h5) both render as `h6`, + collapsing one distinction at the deepest levels +- Trade-off: the rendered HTML no longer matches what a generic CommonMark + renderer would produce for the same source (a surprise when comparing with + an external preview) + +## Pros and Cons of the Options + +### Demote at render time + +- 👍 Structural guarantee, independent of author discipline +- 👍 Fixes existing and pasted content with no migration +- 👎 Deepest levels collapse (h5 and h6 both become h6) + +### Reject level-1 headings at authoring time + +- 👍 The stored source matches the rendered output +- 👎 Surprising validation error for natural Markdown; no help for existing rows + +### Strip h1 in the sanitizer + +- 👍 One-line allowlist change +- 👎 Silently deletes the author's text, the worst failure mode of the four + +### Authoring convention only + +- 👍 No code +- 👎 Exactly the class of promise automated audits keep finding broken diff --git a/docs/adr/README.md b/docs/adr/README.md index 78c92d5..e479efb 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -43,4 +43,5 @@ NNNN-short-title-in-kebab-case.md | [0010](0010-structure-ci-as-focused-workflows-per-concern.md) | Structure CI as focused workflows per concern, not a monolithic ci.yml | accepted | | [0011](0011-start-ci-quality-checks-as-advisory-reports.md) | Start CI quality checks as advisory reports, gates come later | superseded by ADR-0012 | | [0012](0012-publish-test-coverage-to-codecov.md) | Publish test coverage to Codecov | accepted | -| [0013](0013-render-lesson-markdown-with-commonmark-java.md) | Render lesson Markdown with commonmark-java, sanitized by jsoup | accepted | \ No newline at end of file +| [0013](0013-render-lesson-markdown-with-commonmark-java.md) | Render lesson Markdown with commonmark-java, sanitized by jsoup | accepted | +| [0014](0014-demote-markdown-headings-in-lesson-rendering.md) | Demote Markdown headings one level in lesson rendering | accepted | \ No newline at end of file diff --git a/src/main/java/com/ericbouchut/learndev/course/MarkdownRenderer.java b/src/main/java/com/ericbouchut/learndev/course/MarkdownRenderer.java index e2a6a74..47e38c4 100644 --- a/src/main/java/com/ericbouchut/learndev/course/MarkdownRenderer.java +++ b/src/main/java/com/ericbouchut/learndev/course/MarkdownRenderer.java @@ -1,8 +1,13 @@ package com.ericbouchut.learndev.course; import com.ericbouchut.learndev.common.config.CacheConfig; +import org.commonmark.node.Heading; +import org.commonmark.node.Node; import org.commonmark.parser.Parser; +import org.commonmark.renderer.NodeRenderer; +import org.commonmark.renderer.html.HtmlNodeRendererContext; import org.commonmark.renderer.html.HtmlRenderer; +import org.commonmark.renderer.html.HtmlWriter; import org.jsoup.Jsoup; import org.jsoup.safety.Safelist; import org.springframework.cache.annotation.Cacheable; @@ -12,6 +17,8 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.HexFormat; +import java.util.Map; +import java.util.Set; /** * Converts lesson Markdown to HTML that is safe to serve. @@ -20,6 +27,10 @@ * script, event handler, or frame survives, whatever the Markdown contains. * Results are cached by the SHA-256 of the source, so a lesson is rendered * once per content version (see ADR-0013 in docs/adr/). + *

Markdown headings are demoted one level ({@code #} becomes {@code h2}, + * capped at {@code h6}): the lesson page already provides the single + * {@code h1} (the lesson title), and one {@code h1} per page with no skipped + * level is an RGAA 9.1 commitment (see ADR-0014 and docs/rgaa.md). */ @Service public class MarkdownRenderer { @@ -30,7 +41,9 @@ public class MarkdownRenderer { .addAttributes("code", "class"); private final Parser parser = Parser.builder().build(); - private final HtmlRenderer renderer = HtmlRenderer.builder().build(); + private final HtmlRenderer renderer = HtmlRenderer.builder() + .nodeRendererFactory(DemotedHeadingRenderer::new) + .build(); @Cacheable(cacheNames = CacheConfig.RENDERED_MARKDOWN_CACHE, key = "T(com.ericbouchut.learndev.course.MarkdownRenderer).cacheKey(#markdown)") @@ -57,4 +70,38 @@ public static String cacheKey(String markdown) { throw new IllegalStateException("SHA-256 unavailable", e); } } + + /** + * Renders Markdown headings one HTML level lower than authored, capped + * at {@code h6}, so instructor content slots under the page's own + * {@code h1} without breaking the heading hierarchy. + */ + private static final class DemotedHeadingRenderer implements NodeRenderer { + + private final HtmlNodeRendererContext context; + private final HtmlWriter html; + + private DemotedHeadingRenderer(HtmlNodeRendererContext context) { + this.context = context; + this.html = context.getWriter(); + } + + @Override + public Set> getNodeTypes() { + return Set.of(Heading.class); + } + + @Override + public void render(Node node) { + Heading heading = (Heading) node; + String tag = "h" + Math.min(heading.getLevel() + 1, 6); + html.line(); + html.tag(tag, context.extendAttributes(heading, tag, Map.of())); + for (Node child = heading.getFirstChild(); child != null; child = child.getNext()) { + context.render(child); + } + html.tag("/" + tag); + html.line(); + } + } } diff --git a/src/test/java/com/ericbouchut/learndev/course/MarkdownRendererTest.java b/src/test/java/com/ericbouchut/learndev/course/MarkdownRendererTest.java index eb07cc1..a9fddf9 100644 --- a/src/test/java/com/ericbouchut/learndev/course/MarkdownRendererTest.java +++ b/src/test/java/com/ericbouchut/learndev/course/MarkdownRendererTest.java @@ -26,11 +26,43 @@ void renders_markdown_constructs_to_html() { String html = markdownRenderer.render(markdown); // Assert (Then): structural HTML with the language hint preserved - assertThat(html).contains("

Indexes

"); + // (the heading is demoted one level, see the renderer Javadoc) + assertThat(html).contains("

Indexes

"); assertThat(html).contains("bold"); assertThat(html).contains(""); } + @Test + void demotes_headings_one_level_capped_at_h6() { + // Arrange (Given): authored levels 1 through 6; the lesson page owns + // the only h1 (RGAA 9.1), so authored levels must shift down + String markdown = "# One\n\n## Two\n\n##### Five\n\n###### Six"; + + // Act (When) + String html = markdownRenderer.render(markdown); + + // Assert (Then): every level is one deeper, h6 stays h6 + assertThat(html).contains("

One

"); + assertThat(html).contains("

Two

"); + assertThat(html).contains("
Five
"); + assertThat(html).contains("
Six
"); + assertThat(html).doesNotContain("The for loop is great"); + } + @Test void strips_scripts_and_event_handlers_from_hostile_markdown() { // Arrange (Given): raw HTML in Markdown passes through CommonMark,