From a6e04988e0cf9c8fadb4f02d41d8259892ad5f13 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 28 Oct 2025 11:42:17 +0100 Subject: [PATCH 1/4] Add test to reproduce #13616 --- tests/docs/smoke-all/2025/10/27/13616/13616.qmd | 15 +++++++++++++++ tests/docs/smoke-all/2025/10/27/13616/_quarto.yml | 9 +++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/docs/smoke-all/2025/10/27/13616/13616.qmd create mode 100644 tests/docs/smoke-all/2025/10/27/13616/_quarto.yml diff --git a/tests/docs/smoke-all/2025/10/27/13616/13616.qmd b/tests/docs/smoke-all/2025/10/27/13616/13616.qmd new file mode 100644 index 0000000000..fc91c33359 --- /dev/null +++ b/tests/docs/smoke-all/2025/10/27/13616/13616.qmd @@ -0,0 +1,15 @@ +--- +title: "Raw tex in content-visible div (#13616)" +_quarto: + tests: + pdf: + noErrors: true +--- + +::: {.content-visible when-format="pdf"} + +```{=tex} +\textbf{Hello World} +``` + +::: diff --git a/tests/docs/smoke-all/2025/10/27/13616/_quarto.yml b/tests/docs/smoke-all/2025/10/27/13616/_quarto.yml new file mode 100644 index 0000000000..2539ccd82a --- /dev/null +++ b/tests/docs/smoke-all/2025/10/27/13616/_quarto.yml @@ -0,0 +1,9 @@ +project: + type: manuscript + +manuscript: + article: 13616.qmd + +format: + pdf: + pdf-engine: xelatex From 71fcf85591e06742307d7b02a9859f43a886e364 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 28 Oct 2025 17:13:30 +0100 Subject: [PATCH 2/4] Fix ConditionalBlock crash in manuscript projects In manuscript.lua, using pandoc.List() to build block collections and assigning to divEl.content changed the content type from "Blocks" to "List". When ConditionalBlock returned el.content, it returned a List without a .walk() method, causing crashes during AST traversal. Changed to pandoc.Blocks({}) to maintain proper type throughout the filter chain. Fixes #13616 --- src/resources/filters/layout/manuscript.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resources/filters/layout/manuscript.lua b/src/resources/filters/layout/manuscript.lua index a0a3f18cc4..ab21d42e13 100644 --- a/src/resources/filters/layout/manuscript.lua +++ b/src/resources/filters/layout/manuscript.lua @@ -57,10 +57,10 @@ function manuscript() -- If this is a notebook embed cell, 'lift' the contents of any child divs -- up (unroll their contents), this will help us avoid -- labeling divs marked as `cells` more than once - local blocks = pandoc.List() + local blocks = pandoc.Blocks({}) for _, childBlock in ipairs(divEl.content) do if is_regular_node(childBlock, "Div") then - tappend(blocks, childBlock.content) + blocks:extend(childBlock.content) else blocks:insert(childBlock) end From 5e8020a324c456014e04feb3456bb68c973ca780 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 28 Oct 2025 17:13:57 +0100 Subject: [PATCH 3/4] use simpler version of the test as it has nothing to do with RawBlocks --- tests/docs/smoke-all/2025/10/27/13616/.gitignore | 2 ++ tests/docs/smoke-all/2025/10/27/13616/13616.qmd | 6 ++---- 2 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 tests/docs/smoke-all/2025/10/27/13616/.gitignore diff --git a/tests/docs/smoke-all/2025/10/27/13616/.gitignore b/tests/docs/smoke-all/2025/10/27/13616/.gitignore new file mode 100644 index 0000000000..ad293093b0 --- /dev/null +++ b/tests/docs/smoke-all/2025/10/27/13616/.gitignore @@ -0,0 +1,2 @@ +/.quarto/ +**/*.quarto_ipynb diff --git a/tests/docs/smoke-all/2025/10/27/13616/13616.qmd b/tests/docs/smoke-all/2025/10/27/13616/13616.qmd index fc91c33359..85e7938dae 100644 --- a/tests/docs/smoke-all/2025/10/27/13616/13616.qmd +++ b/tests/docs/smoke-all/2025/10/27/13616/13616.qmd @@ -1,5 +1,5 @@ --- -title: "Raw tex in content-visible div (#13616)" +title: "Conditional Block in manuscript article" _quarto: tests: pdf: @@ -8,8 +8,6 @@ _quarto: ::: {.content-visible when-format="pdf"} -```{=tex} -\textbf{Hello World} -``` +Something ::: From 6e74c59be9a84edeb352d2b74b478ec08cc10432 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 30 Oct 2025 15:16:16 +0000 Subject: [PATCH 4/4] TO REMOVE : plan doc for claude --- .../plans/pandoc-list-vs-blocks-analysis.md | 256 ++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 .claude/docs/plans/pandoc-list-vs-blocks-analysis.md diff --git a/.claude/docs/plans/pandoc-list-vs-blocks-analysis.md b/.claude/docs/plans/pandoc-list-vs-blocks-analysis.md new file mode 100644 index 0000000000..f5daa300cb --- /dev/null +++ b/.claude/docs/plans/pandoc-list-vs-blocks-analysis.md @@ -0,0 +1,256 @@ +# pandoc.List() vs pandoc.Blocks() Bug Analysis + +## Executive Summary + +The bug fixed in manuscript.lua (PR #13624) is **NOT ConditionalBlock-specific**. It's a type mismatch bug where `pandoc.List()` is used instead of `pandoc.Blocks()` when building block collections that are assigned to element `.content` properties. ConditionalBlock merely **exposed** this bug, but the bug could be triggered by any code path that walks the AST of an element with wrongly-typed content. + +## The Core Bug: Type Mismatch + +### What's the Difference? + +From the Lua type definitions in quarto-cli: + +**pandoc.Blocks** (`src/resources/pandoc/blocks.lua:27-29`): +```lua +-- List of Block elements, a special case of a pandoc.List. +-- pandoc.Blocks is a type alias for pandoc.List, but it also +-- supports a `walk` method. +``` + +**pandoc.List** (`src/resources/pandoc/List.lua`): +- Standard Lua List implementation +- Has methods like `:insert()`, `:extend()`, `:map()`, etc. +- Does NOT have a `.walk()` method in the standard implementation + +### The Bug Pattern + +```lua +-- BUGGY CODE +local blocks = pandoc.List() +for _, child in ipairs(divEl.content) do + blocks:insert(child) +end +divEl.content = blocks -- BUG: Assigns List to .content + +-- Later in the filter chain... +_quarto.ast.walk(divEl, {...}) -- CRASH: tries to walk divEl.content +``` + +When you assign a `pandoc.List()` to `.content`, any subsequent AST walking operations that expect `pandoc.Blocks` (with its `.walk()` method) will fail. + +### Why It Crashes + +The crash happens when: +1. Code assigns `pandoc.List()` to an element's `.content` property +2. That element is returned from a filter or passed to another filter +3. Some code tries to walk the AST, either: + - Directly via `_quarto.ast.walk(element, ...)` + - Indirectly when a filter returns `element.content` and Pandoc's filter system tries to process it + - When custom filters traverse the AST + +The error manifests as attempting to call a method that doesn't exist on `pandoc.List`. + +## ConditionalBlock's Role: Exposing, Not Causing + +### What ConditionalBlock Does + +From `src/resources/filters/quarto-pre/content-hidden.lua:63`: +```lua +function conditionalBlock(show) + return function(el) + if show then + return el.content -- Returns the Blocks directly + else + return {} + end + end +end +``` + +When `show` is true, ConditionalBlock returns `el.content` **as-is**. This propagates whatever type `.content` is up the filter chain. + +### How It Exposed the Bug + +1. manuscript.lua assigns `pandoc.List()` to `divEl.content` +2. That `divEl` contains a ConditionalBlock +3. ConditionalBlock's filter runs and returns `el.content` (the wrongly-typed List) +4. Later code tries to walk this returned content +5. CRASH: `pandoc.List` doesn't have the expected methods + +**But ConditionalBlock is just the messenger!** The bug was already there when `pandoc.List()` was assigned to `.content`. ConditionalBlock just exposed it by propagating the wrong type upward. + +## Other Potential Triggers (Without ConditionalBlock) + +The bug could be triggered by: + +### 1. Direct AST Walking +```lua +local blocks = pandoc.List() +divEl.content = blocks +_quarto.ast.walk(divEl, {...}) -- CRASH: even without ConditionalBlock +``` + +### 2. Custom Filters That Return .content +Any custom filter extension that does: +```lua +function MyFilter(el) + return el.content -- If content is wrongly-typed List +end +``` + +### 3. Filter Chain Interactions +When filters compose and one expects `pandoc.Blocks` but receives `pandoc.List`. + +### 4. Format-Specific Processing +Some output formats may do additional AST traversal that would trigger the crash. + +## Analysis of Each Occurrence + +### manuscript.lua (Line 68) - **BUGGY** ✗ + +**Code**: +```lua +local blocks = pandoc.List() +-- ... build blocks ... +divEl.content = blocks -- Assigns List to .content +``` + +**Why it's buggy**: +- Assigns `pandoc.List()` to `divEl.content` +- Later code walks the AST (line 138: `_quarto.ast.walk(divEl, ...)`) +- Exposed by ConditionalBlock, but would crash on any AST walk + +**Fix applied**: Changed to `pandoc.Blocks({})` + +### outputs.lua (Line 41) - **SAFE** ✓ + +**Code**: +```lua +local blocks = pandoc.List() +-- ... build blocks ... +return blocks -- Returns directly from filter +``` + +**Why it's safe**: +- Returns `pandoc.List()` directly from the filter +- Does NOT assign to `.content` +- Pandoc's filter system can handle returned Lists +- The returned blocks are used to replace content, not stored as-is + +**No fix needed**: Return pattern is safe. + +### jats.lua quarto-post (Line 57) - **SAFE** ✓ + +**Code**: +```lua +function unrollDiv(div, fnSkip) + local blocks = pandoc.List() + -- ... build blocks ... + return blocks -- Returns directly +end +``` + +**Why it's safe**: Same as outputs.lua - returns directly, never assigns to `.content`. + +**No fix needed**: Return pattern is safe. + +### jats.lua quarto-post (Line 175) - **BUGGY** ✗ + +**Code**: +```lua +local orderedContents = pandoc.List() +orderedContents:extend(otherEls) +orderedContents:extend(outputEls) +div.content = orderedContents -- Assigns List to .content +``` + +**Why it's buggy**: +- Assigns `pandoc.List()` to `div.content` +- Same pattern as manuscript.lua +- Could crash if this div is later walked or returned by a filter like ConditionalBlock + +**Needs fix**: Change line 172 to `pandoc.Blocks({})` + +### dashboard.lua (Lines 248, 296) - **SAFE** ✓ + +**Code**: +```lua +local results = pandoc.List() +-- ... build results ... +return pandoc.Blocks(results) -- Wraps in pandoc.Blocks before return +``` + +**Why it's safe**: +- Uses `pandoc.List()` internally (fine for building) +- Wraps in `pandoc.Blocks()` before returning +- Returns the correct type + +**No fix needed**: Already handles type correctly. + +## The Fix Pattern + +### When to Use pandoc.Blocks() + +Use `pandoc.Blocks({})` when: +1. Building a collection that will be assigned to `.content` +2. Building a collection that will be part of the document AST +3. Building blocks that need to support `.walk()` operations + +```lua +-- CORRECT +local blocks = pandoc.Blocks({}) +blocks:extend(childContent) +element.content = blocks +``` + +### When pandoc.List() is OK + +Use `pandoc.List()` when: +1. Building a temporary collection that will be returned directly (filter system handles it) +2. Building a collection that will be wrapped in `pandoc.Blocks()` before use +3. Building non-Block collections (like List of Inlines, table rows, etc.) + +```lua +-- CORRECT: Direct return +local blocks = pandoc.List() +blocks:insert(something) +return blocks -- Filter system handles conversion + +-- CORRECT: Explicit wrap +local blocks = pandoc.List() +blocks:insert(something) +return pandoc.Blocks(blocks) + +-- CORRECT: Non-block collection +local inlines = pandoc.List() -- Building Inlines, not Blocks +``` + +## Root Cause Summary + +The bug is fundamentally about **type safety in Lua's Pandoc AST**: + +1. `.content` properties expect `pandoc.Blocks` (which has `.walk()`) +2. Using `pandoc.List()` creates wrong type without required methods +3. Crash occurs when AST traversal tries to use `.walk()` on the List +4. ConditionalBlock exposed the bug by returning wrongly-typed content +5. But ANY AST walking or content propagation could trigger it + +## Recommended Actions + +1. **Fix jats.lua line 172**: Change `pandoc.List()` to `pandoc.Blocks({})` +2. **Test verification**: Create tests for both manuscript and JATS formats with ConditionalBlock +3. **Pattern search**: Search codebase for `.content = .*pandoc.List` pattern +4. **Documentation**: Consider adding a code comment in blocks.lua about when to use which type + +## Testing Strategy + +Tests should verify: +1. Documents with ConditionalBlock in manuscript format (already exists) +2. Documents with ConditionalBlock in JATS format (needs creation) +3. Direct AST walking on elements with `.content` assigned (stress test) +4. Filter composition scenarios + +The tests should confirm that: +- Fixed code works with ConditionalBlock +- Fixed code works with direct AST walking +- Safe patterns (outputs.lua, dashboard.lua) continue working