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 SetIndexes
");
+ // (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,