From bdbfc60e5c2773410ac32227af0839408194f469 Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Fri, 4 Sep 2026 11:19:10 -0400 Subject: [PATCH] TOC: descend through a wrapper div into the section it holds Fixes bd-toc-skips-headings-in-id-div-1jorg679. A heading inside a fenced div that carries an id never reached the table of contents. The heading rendered, its
was anchorable, and a reader could scroll to it, but there was no way to navigate to it. No diagnostic; the render exited 0. Quarto 1 lists it. The id is the trigger. sectionize_blocks absorbs an anonymous wrapper into the section it holds, so a div with no attributes or with only a class is already a plain section by the time the TOC is built. An id-bearing wrapper cannot be absorbed -- its id would collide with the section's -- so the section stays one level down, and collect_toc_entries ended the walk at the first non-section Div it met. The walk now sees through a wrapper whose sole content is another Div, which is pandoc's rule (toTOCTree, Text/Pandoc/Chunks.hs, jgm/pandoc#8402). It recurses rather than unwrapping one level, so a section under stacked wrappers is reached too -- Quarto 1 lists those. A wrapper holding anything besides a lone Div still ends the walk, which is where filter-built chrome stops -- though not always at its outermost Div. A titled callout and a resolved tabset each put two blocks side by side, header beside body and nav Plain beside pane container, so the walk stops at the outer Div. An untitled callout does not: build_untitled_content emits a lone .callout-body.d-flex, so the walk descends one level and stops on the icon container beside the body container. pandoc descends that same level, so Quarto 1 stops in the same place. Both stops rest on a sibling block another transform is free to move, so each is now pinned by a test, and panel_tabset_resolve.rs records that its nav Plain is load-bearing for the TOC walk. Headings inside a tab pane, a callout, or a blockquote therefore stay out of the TOC, as does a heading with prose or a sibling section beside it inside the same wrapper -- all matching both pandoc and Quarto 1. TESTS New pipeline-level tests in tests/integration/test_toc_wrapper_divs.rs drive markdown through readers::qmd::read -> sectionize_blocks -> generate_toc, because the bug was an interaction between the absorb rule and the walk and toc.rs's unit tests hand-build the sectionized AST on both sides of it. They pin the eight-case contract, stacked wrappers, and the two ways a wrapper can hold more than one block (prose before the heading, and two sibling sections). Confirmed to fail without the new arm. test_non_section_div_terminates_the_walk pinned the old behaviour with a synthetic tabset shape (.panel-tabset directly wrapping a .tab-pane) that PanelTabsetResolveTransform does not emit -- its own unit test asserts the outer Div holds two blocks. It is now test_resolved_tabset_pane_is_not_reached and uses the real shape. test_untitled_callout_body_is_not_reached is new and pins the other stop. test_blockquote_heading_is_not_collected (bd-8yjvs3bj) is untouched and green. Workspace: 13685 passed, 199 skipped (+6, all added here). VERIFIED END TO END The eight-case repro fixture: q2 now agrees with Quarto 1 on every case, including both negative controls. Callout, tabset and single-tab-tabset shapes rendered through the binary produce TOCs identical to Quarto 1's. On the Positron website's download page, which wraps its hero in ::: {#download-hero .download-hero}, the "Recommended for you" entry is present and the page's TOC matches Quarto 1's entry for entry. extract_outline shares generate_toc and runs pre-transform, so a single-child id-bearing callout or tabset now reaches profile.outline though the rendered TOC still excludes it. That is the already-filed bd-ca17fck0 divergence, extended from anonymous wrappers to id-bearing ones; noted on that strand. --- crates/pampa/src/toc.rs | 266 +++++++++++++++--- crates/pampa/tests/integration/main.rs | 1 + .../integration/test_toc_wrapper_divs.rs | 154 ++++++++++ .../src/transforms/panel_tabset_resolve.rs | 8 + 4 files changed, 392 insertions(+), 37 deletions(-) create mode 100644 crates/pampa/tests/integration/test_toc_wrapper_divs.rs diff --git a/crates/pampa/src/toc.rs b/crates/pampa/src/toc.rs index 1f7f9b114..4a011b9d4 100644 --- a/crates/pampa/src/toc.rs +++ b/crates/pampa/src/toc.rs @@ -44,22 +44,32 @@ //! //! The function detects sectionized structure and extracts headers accordingly. //! -//! ## The walk stops at a non-section Div +//! ## What the walk descends through //! -//! The table of contents *is* the section tree. A `Div` that is not a -//! section ends the walk, with no recursion past it — this is pandoc's -//! rule, whose `sectionToListItem` matches only `Div(_, _, Header:rest)` -//! and yields nothing for anything else. +//! The table of contents *is* the section tree, and the walk reaches it +//! two ways. It collects a section `Div` and recurses into it. It also +//! descends through a *wrapper* — a `Div` whose sole content is another +//! `Div` — collecting nothing on the way, which is pandoc's rule +//! (`toTOCTree`, `Text/Pandoc/Chunks.hs`, jgm/pandoc#8402). That descent +//! recurses, so stacked wrappers are all traversed. Anything else ends +//! the walk. //! -//! This is not a loss of reach, because `sectionize_blocks` *absorbs* a -//! transparent wrapper — an empty-id `Div` around a single header-led -//! run — into the section itself, so a heading inside `::: {.column-margin}` -//! or a plain `:::` block is a section by the time we get here. What stays -//! wrapped in a plain `Div` is filter-built chrome: a callout body, a -//! tabset pane. Quarto 1 does not list those either, and listing them was -//! actively harmful — the entries were indistinguishable from one another -//! and every one past the first pointed at `display: none` content -//! (bd-tabset-headings-in-toc-t04ie7f7). +//! The wrapper clause exists for a `Div` the author gave an id. +//! `sectionize_blocks` *absorbs* an anonymous wrapper — an empty-id +//! `Div` around a single header-led run — into the section itself, so a +//! heading inside `::: {.column-margin}` or a plain `:::` block is +//! already an ordinary section by the time we get here. An id-bearing +//! wrapper it cannot absorb, since that would put two ids on one +//! element, so `::: {#download-hero}` leaves its section nested a level +//! down and the walk has to descend to reach it +//! (bd-toc-skips-headings-in-id-div-1jorg679). +//! +//! What ends the walk is filter-built chrome — though not always at its +//! outermost `Div`; the `_` arm of `collect_toc_entries` records where +//! each shape actually stops. Quarto 1 does not list those headings +//! either, and listing them was actively harmful: the entries were +//! indistinguishable from one another and every one past the first +//! pointed at `display: none` content (bd-tabset-headings-in-toc-t04ie7f7). //! //! **Precondition:** callers that want headings nested in Divs to appear //! must pass blocks that have been through `sectionize_blocks`. See @@ -367,10 +377,6 @@ fn collect_toc_entries(blocks: &[Block], max_depth: i32) -> Vec { for block in blocks { match block { - // Only section Divs are walked. A Div that is not a section - // ends the walk, with no recursion past it — pandoc's - // `sectionToListItem` matches `Div(_, _, Header:rest)` and - // returns nothing for anything else. Block::Div(div) if is_section_div(div) => { if let Some(entry) = extract_entry_from_section(div, max_depth) { entries.push(entry); @@ -378,6 +384,19 @@ fn collect_toc_entries(blocks: &[Block], max_depth: i32) -> Vec { // Recurse into section content for nested sections entries.extend(collect_toc_entries(&div.content, max_depth)); } + // A wrapper Div whose sole content is another Div is + // transparent to the walk — pandoc's `toTOCTree` descends + // through it (`Text/Pandoc/Chunks.hs`, jgm/pandoc#8402). + // The case that matters is a wrapper the author gave an id: + // `sectionize_blocks` absorbs an anonymous wrapper into the + // section it holds, but an id-bearing one it cannot, since + // that would put two ids on one element. The heading inside + // is an ordinary section a reader expects to navigate to. + // Recursion, not a single unwrap, so stacked wrappers are + // all descended through. + Block::Div(div) if wraps_exactly_one_div(div) => { + entries.extend(collect_toc_entries(&div.content, max_depth)); + } Block::Header(header) => { // Direct header (non-sectionized document) if let Some(entry) = extract_entry_from_header(header, max_depth) { @@ -385,10 +404,32 @@ fn collect_toc_entries(blocks: &[Block], max_depth: i32) -> Vec { } } _ => { - // Nothing else carries a section. In particular - // `BlockQuote`: `sectionize_blocks` does not descend - // into one (matching pandoc's `makeSections`), so a - // quoted heading is never a section and is never + // Nothing else carries a section, and nothing else is + // descended through. + // + // Filter-built chrome ends the walk, but not always at + // its outermost Div. A *titled* callout and a resolved + // tabset each put two blocks side by side — header + // beside body, nav `Plain` beside pane container — so + // the walk stops at the outer Div. An *untitled* + // callout does not: `build_untitled_content` + // (`quarto-core/src/transforms/callout_resolve.rs`) + // emits a lone `.callout-body.d-flex`, so the walk + // descends one level and stops there instead, on the + // icon container beside the body container. pandoc + // descends that same level, so Quarto 1 stops in the + // same place. + // + // Both stops therefore rest on a *sibling* block: the + // tabset's nav `Plain`, and the callout's icon + // container, which `callout_resolve` emits + // unconditionally for Q1 parity. Dropping either would + // quietly put chrome headings back in the TOC + // (bd-tabset-headings-in-toc-t04ie7f7). + // + // `BlockQuote` likewise — `sectionize_blocks` does not + // descend into one (matching pandoc's `makeSections`), + // so a quoted heading is never a section and is never // listed. bd-8yjvs3bj. } } @@ -403,6 +444,17 @@ fn is_section_div(div: &Div) -> bool { classes.iter().any(|c| c == "section") } +/// Check if a Div is one the walk should see through: one whose content +/// is exactly one other Div, matching pandoc's `toTOCTree` clause +/// `go (Div _ [d@Div{}]) = go d`. +/// +/// Deliberately not called "transparent": `sectionize_blocks` uses that +/// word for the anonymous wrapper it *absorbs*, which is the opposite +/// shape — the one that never reaches this walk at all. +fn wraps_exactly_one_div(div: &Div) -> bool { + matches!(div.content.as_slice(), [Block::Div(_)]) +} + /// Extract the heading level from a section Div's classes. fn get_section_level(div: &Div) -> Option { let (_, classes, _) = &div.attr; @@ -724,14 +776,24 @@ mod tests { assert_eq!(toc.entries[0].children[0].id, "sub"); } - // ── the walk stops at a non-section Div ──────────────────────── + // ── what the walk descends through, and what ends it ─────────── + // + // pandoc's `toTOCTree` matches a section — `Div(_, _, Header:rest)` + // — and, since #8402, a wrapper Div whose sole content is another + // Div, which it descends through. Anything else ends the walk. // - // pandoc's `toTableOfContents` matches only `Div(_, _, Header:rest)` - // — a section — so a Div that is not one ends the walk with no - // recursion past it. Everything a reader expects to see in the TOC - // has already been *absorbed* into the section tree by - // `sectionize_blocks`; what remains wrapped in a plain Div is filter - // chrome (a callout, a tabset pane), and Quarto 1 does not list it. + // A wrapper the author left anonymous is already gone by this + // point: `sectionize_blocks` absorbs a Div with an empty id whose + // content is exactly one section. What survives to be descended + // through is a wrapper the author gave an id — that one cannot be + // absorbed, because its id would collide with the section's. + // + // What survives to end the walk is filter-built chrome — though not + // always at its outermost Div: an untitled callout is a lone + // `.callout-body.d-flex`, so the walk descends one level and stops + // on the icon container beside the body. The tests below pin both + // stops, because each rests on a sibling block another transform is + // free to move. /// Every id in the tree, depth-first. Checking only `toc.entries` /// would miss a leaked entry, which `build_hierarchy` nests *under* @@ -754,6 +816,19 @@ mod tests { } } + /// A `Plain` holding one `RawInline`, the shape filter-built chrome + /// leaves alongside its container Divs. + fn make_raw_plain(html: &str) -> Block { + Block::Plain(quarto_pandoc_types::block::Plain { + content: vec![Inline::RawInline(quarto_pandoc_types::inline::RawInline { + format: "html".to_string(), + text: html.to_string(), + source_info: dummy_source_info(), + })], + source_info: dummy_source_info(), + }) + } + fn make_plain_div(id: &str, classes: Vec<&str>, content: Vec) -> Block { Block::Div(Div { attr: ( @@ -767,20 +842,31 @@ mod tests { }) } + /// The shape `PanelTabsetResolveTransform` leaves behind: the + /// nav-tabs list as a `Plain`, then the pane container. The + /// `.panel-tabset` Div therefore holds two blocks, not one Div, so + /// the walk ends there and the section inside a pane stays out of + /// the TOC — which is where Quarto 1 leaves it. + /// bd-tabset-headings-in-toc-t04ie7f7. #[test] - fn test_non_section_div_terminates_the_walk() { - // The shape a resolved tabset leaves behind: a section, real and - // correct, buried under two plain Divs. + fn test_resolved_tabset_pane_is_not_reached() { let blocks = vec![ make_section(2, "configuration", vec![], "Configuration", vec![]), make_plain_div( "", vec!["panel-tabset"], - vec![make_plain_div( - "tabset-1-1", - vec!["tab-pane"], - vec![make_section(4, "in-a-tab", vec![], "In a tab", vec![])], - )], + vec![ + make_raw_plain("
    …
"), + make_plain_div( + "", + vec!["tab-content"], + vec![make_plain_div( + "tabset-1-1", + vec!["tab-pane", "active"], + vec![make_section(4, "in-a-tab", vec![], "In a tab", vec![])], + )], + ), + ], ), make_section(2, "next-steps", vec![], "Next steps", vec![]), ]; @@ -792,6 +878,112 @@ mod tests { ); } + /// The shape `build_untitled_content` leaves behind: the outer + /// `.callout` Div holds a *lone* `.callout-body.d-flex`, so unlike + /// a titled callout the walk does descend into it — and stops one + /// level down, because the icon container sits beside the body + /// container. pandoc stops in the same place, so Quarto 1 does too. + /// The icon container is emitted unconditionally for Q1 parity + /// (`callout_resolve::icon_container_div`); this test is what would + /// catch it going away. bd-tabset-headings-in-toc-t04ie7f7. + #[test] + fn test_untitled_callout_body_is_not_reached() { + let blocks = vec![ + make_section(2, "before", vec![], "Before", vec![]), + make_plain_div( + "", + vec!["callout", "callout-style-simple", "callout-note"], + vec![make_plain_div( + "", + vec!["callout-body", "d-flex"], + vec![ + make_plain_div( + "", + vec!["callout-icon-container"], + vec![make_raw_plain("")], + ), + make_plain_div( + "", + vec!["callout-body-container"], + vec![make_section( + 3, + "in-a-callout", + vec![], + "In a callout", + vec![], + )], + ), + ], + )], + ), + ]; + let toc = generate_toc(&blocks, &deep_config()); + assert_eq!(all_ids(&toc.entries), vec!["before"]); + } + + /// An author's `::: {#someid}` around a heading. The wrapper cannot + /// be absorbed by `sectionize_blocks` — its id would collide with + /// the section's — so the section sits one level down. pandoc and + /// Quarto 1 both list it. bd-toc-skips-headings-in-id-div-1jorg679. + #[test] + fn test_id_bearing_wrapper_is_descended_through() { + let blocks = vec![ + make_section(2, "before", vec![], "Before", vec![]), + make_plain_div( + "someid", + vec![], + vec![make_section(2, "wrapped", vec![], "Wrapped", vec![])], + ), + make_section(2, "after", vec![], "After", vec![]), + ]; + let toc = generate_toc(&blocks, &deep_config()); + assert_eq!(all_ids(&toc.entries), vec!["before", "wrapped", "after"]); + } + + /// `::: {#outer}` around `::: {#inner}`. pandoc's clause recurses, + /// so the descent iterates instead of unwrapping exactly one level. + #[test] + fn test_nested_wrappers_are_descended_through() { + let blocks = vec![make_plain_div( + "outer", + vec![], + vec![make_plain_div( + "inner", + vec![], + vec![make_section( + 2, + "doubly-nested", + vec![], + "Doubly nested", + vec![], + )], + )], + )]; + let toc = generate_toc(&blocks, &deep_config()); + assert_eq!(all_ids(&toc.entries), vec!["doubly-nested"]); + } + + /// The negative control. When the wrapper holds prose *before* the + /// heading its content is not a single Div, and neither pandoc nor + /// Quarto 1 lists the heading. Descending unconditionally into + /// every Div would list it and break parity in the other direction. + #[test] + fn test_wrapper_with_content_before_the_section_ends_the_walk() { + let blocks = vec![ + make_section(2, "before", vec![], "Before", vec![]), + make_plain_div( + "someid", + vec![], + vec![ + make_para("Prose before the heading."), + make_section(2, "wrapped", vec![], "Wrapped", vec![]), + ], + ), + ]; + let toc = generate_toc(&blocks, &deep_config()); + assert_eq!(all_ids(&toc.entries), vec!["before"]); + } + #[test] fn test_section_nested_in_a_section_is_still_collected() { // The walk must still recurse through *sections*, or the TOC diff --git a/crates/pampa/tests/integration/main.rs b/crates/pampa/tests/integration/main.rs index d209acda4..8cbcbaca4 100644 --- a/crates/pampa/tests/integration/main.rs +++ b/crates/pampa/tests/integration/main.rs @@ -82,6 +82,7 @@ pub mod test_shortcode_separator_diagnostics; pub mod test_smart_typography_positions; pub mod test_task_list; pub mod test_template_integration; +pub mod test_toc_wrapper_divs; pub mod test_trailing_linebreak_commonmark; pub mod test_treesitter_coverage; pub mod test_treesitter_refactoring; diff --git a/crates/pampa/tests/integration/test_toc_wrapper_divs.rs b/crates/pampa/tests/integration/test_toc_wrapper_divs.rs new file mode 100644 index 000000000..be6482444 --- /dev/null +++ b/crates/pampa/tests/integration/test_toc_wrapper_divs.rs @@ -0,0 +1,154 @@ +/* + * test_toc_wrapper_divs.rs + * Copyright (c) 2026 Posit, PBC + * + * Pipeline-level tests for which wrapped headings reach the TOC. + */ + +//! What reaches the table of contents when a heading is wrapped in a +//! fenced div, driven from markdown rather than from a hand-built AST. +//! +//! `toc.rs`'s unit tests construct sectionized blocks directly, so they +//! pin `collect_toc_entries` in isolation. This bug was an *interaction*: +//! `sectionize_blocks` absorbs an anonymous wrapper into the section it +//! holds but cannot absorb an id-bearing one (its id would collide with +//! the section's), and the walk then refused to enter what was left. +//! Either half can change without the other's tests noticing, so these +//! run the real pair — `readers::qmd::read` → `sectionize_blocks` → +//! `generate_toc` — over the eight wrapper shapes that pin the contract. +//! +//! Every expectation here was measured against `pandoc 3.8.1 +//! --toc --section-divs` and against Quarto 1, which agree with each +//! other on all eight. bd-toc-skips-headings-in-id-div-1jorg679. + +use pampa::toc::{TocConfig, TocEntry, generate_toc}; +use pampa::transforms::sectionize_blocks; + +/// The eight cases in one document, matching the out-of-repo repro +/// fixture case for case so the two cannot drift apart silently. +const FIXTURE: &str = r#"## A top level + +body + +::: {} +## B no attributes +::: + +::: {.someclass} +## C class only +::: + +::: {#someid} +## D id only +::: + +::: {#someid2 .someclass2} +## E id and class +::: + +::: {#someid3} +## [F span heading]{#span-id} +::: + +::: {#someid4} +Prose before the heading. + +## G not the sole child +::: + +::: {#outer} +::: {#inner} +## H doubly nested +::: +::: +"#; + +fn toc_ids(markdown: &str) -> Vec { + let (pandoc, _context, _warnings) = pampa::readers::qmd::read( + markdown.as_bytes(), + false, + "", + &mut std::io::sink(), + true, + None, + ) + .expect("fixture must parse"); + let sectionized = sectionize_blocks(pandoc.blocks); + let toc = generate_toc( + §ionized, + &TocConfig { + depth: 6, + title: None, + }, + ); + let mut ids = Vec::new(); + collect_ids(&toc.entries, &mut ids); + ids +} + +fn collect_ids(entries: &[TocEntry], out: &mut Vec) { + for entry in entries { + out.push(entry.id.clone()); + collect_ids(&entry.children, out); + } +} + +/// The whole contract in one assertion. G is absent and everything else +/// is present, in document order. +#[test] +fn wrapped_headings_reach_the_toc_except_when_the_wrapper_holds_more() { + assert_eq!( + toc_ids(FIXTURE), + vec![ + "a-top-level", + "b-no-attributes", + "c-class-only", + "d-id-only", + "e-id-and-class", + "f-span-heading", + "h-doubly-nested", + ], + "G must stay out — pandoc and Quarto 1 both exclude a wrapper \ + whose content is not a single Div — and every other case must \ + come in" + ); +} + +/// The case the bug report was filed for, on its own so a failure names +/// it. An id on the wrapper is the entire trigger: C, one line away in +/// the fixture above, differs only in carrying a class instead. +#[test] +fn an_id_on_the_wrapper_does_not_hide_the_heading() { + assert_eq!(toc_ids("::: {#someid}\n## Wrapped\n:::\n"), vec!["wrapped"]); +} + +/// Depth: Quarto 1 descends through arbitrarily many wrappers, so the +/// descent has to iterate rather than unwrap one level. +#[test] +fn stacked_wrappers_are_all_descended_through() { + assert_eq!( + toc_ids("::: {#a}\n::: {#b}\n::: {#c}\n## Deep\n:::\n:::\n:::\n"), + vec!["deep"] + ); +} + +/// The negative control, isolated. Prose ahead of the heading means the +/// wrapper holds two blocks, and neither pandoc nor Quarto 1 lists it. +#[test] +fn prose_before_the_heading_keeps_it_out() { + assert!( + toc_ids("::: {#someid}\nProse first.\n\n## Wrapped\n:::\n").is_empty(), + "listing this would diverge from Quarto 1 in the other direction" + ); +} + +/// The other way to hold more than one block: two sibling sections. The +/// existing negative control fails the predicate on block *kind*; this +/// one fails it on block *count*. Verified against pandoc 3.8.1. +#[test] +fn two_sibling_sections_in_one_wrapper_stay_out() { + assert!( + toc_ids("::: {#someid}\n## One\n\n## Two\n:::\n").is_empty(), + "a wrapper holding two sections is not a transparent wrapper" + ); +} diff --git a/crates/quarto-core/src/transforms/panel_tabset_resolve.rs b/crates/quarto-core/src/transforms/panel_tabset_resolve.rs index 592d3a6e2..ea2712f75 100644 --- a/crates/quarto-core/src/transforms/panel_tabset_resolve.rs +++ b/crates/quarto-core/src/transforms/panel_tabset_resolve.rs @@ -352,6 +352,14 @@ mod tests { panic!("expected resolved Div"); }; assert!(outer.attr.1.contains(&"panel-tabset".to_string())); + // The nav `Plain` sitting beside the tab-content Div is + // load-bearing beyond the markup: `collect_toc_entries` + // (`pampa/src/toc.rs`) descends through a Div whose *sole* + // content is another Div, so this second block is what stops the + // TOC walk at the tabset. Move the nav into the template and + // `.tab-content` becomes a lone Div — and for a single-tab + // tabset its pane too — putting tab headings back in the TOC + // with nothing else failing (bd-tabset-headings-in-toc-t04ie7f7). assert_eq!(outer.content.len(), 2, "nav Plain + tab-content Div"); let Block::Plain(nav) = &outer.content[0] else {