Skip to content

Cap body images at their container width (bd-images-no-max-width-e5ywgnma) - #644

Merged
gordonwoodhull merged 1 commit into
mainfrom
bugfix/bd-images-no-max-width-e5ywgnma-images-no-max-width
Sep 2, 2026
Merged

Cap body images at their container width (bd-images-no-max-width-e5ywgnma)#644
gordonwoodhull merged 1 commit into
mainfrom
bugfix/bd-images-no-max-width-e5ywgnma-images-no-max-width

Conversation

@gordonwoodhull

Copy link
Copy Markdown
Member

An image with no author-supplied size lays out at its intrinsic pixel width, because q2 emits every body image with no class at all. A 2680×1720 screenshot in an 850px content column renders 2680px wide: it overflows the column, overruns the margin, and puts a horizontal scrollbar on the page. On the Positron docs site this hits 63 images across 20 pages, median intrinsic width 2470px. The images that look fine only escape because their author happened to write an explicit width=, which is why the bug hides in a real corpus — about half of any given page is fine and only the unsized images blow out.

.img-fluid { max-width: 100%; height: auto } is already in the compiled theme; it arrives with Bootstrap, byte-identical to Quarto 1's. Nothing else in that stylesheet constrains a figure image, and there's no bare img { max-width } fallback. The only thing missing was the pass that applies the class.

This adds ResponsiveImageTransform, a port of Quarto 1's quarto-post/responsive.lua. It runs in the Finalization phase beside TableBootstrapClassTransform, its sibling in intent — both inject the Bootstrap classes the theme is keyed off, and both run late enough that crossref-rendered figures are tagged too. Chrome emitted as raw HTML rather than AST nodes (listing thumbnails, navbar logos, page-footer images) stays untagged, as in Quarto 1.

Per-image, both of Quarto 1's exclusions are preserved: an explicit height, since img-fluid carries height: auto and would silently override the size the author asked for; and data-no-responsive, honored for any value. An explicit width is deliberately not an exclusion — max-width: 100% still lets a width=450 image shrink inside a narrower column, so the class refines the author's width rather than fighting it.

The document-level default is per-format

fig-responsive is the switch, but its default is not simply "on for HTML". Quarto 1's filter tests isHtmlOutput() and reads param('fig-responsive'), whose default is set per format: createHtmlPresentationFormat sets it false for every HTML presentation format, so revealjs is not tagged even though isHtmlOutput() is true there, and format-html.ts does the same for minimal: true. Both are suppliers of last resort, so an explicit fig-responsive: true still wins.

Verified against the real Quarto 1 binary on all five cases — plain, revealjs, minimal: true, theme: none, and each with an explicit override. Two details worth knowing if you touch the gate:

  • Reveal is matched on target_format, not the format identifier. Every live deck arrives as revealjs or — since the bd-vwp4y5ku convergence rewrote format: revealjs on the way into preview — as q2-slides, which resolves to FormatIdentifier::Html. An identifier test would tag deck images in preview and not in render.
  • The minimal check reads the raw metadata flag rather than is_minimal_html, which also returns true for theme: none / theme: pandoc — where both engines do tag.

llms.txt

img-fluid is stripped in the llms.txt sanitizer. The transform runs immediately before LlmsCaptureTransform, so without the strip every page's .md mirror would read ![A caption](wide.png){.img-fluid} — Bootstrap presentation escaping into output whose whole purpose is clean semantic markdown. docs/ sets llms-txt: true and carries 76 image references, so this is the difference between a clean mirror and 76 pieces of noise in q2 docs llms.

Verification

Against the shared acceptance fixture (q2-positron-docs/llms-info/repros/images-no-max-width) — a plain image, one with width=, one with height=, and one inline. Quarto 1 tags three of four; q2 now tags the same three:

== q2 ==                                    == Quarto 1 ==
1  <img src="wide.png" class="img-fluid" />        class="img-fluid figure-img"
2  <img ... class="img-fluid" width="450" />       class="img-fluid figure-img" width="450"
3  <img src="wide.png" height="100" />             height="100" class="figure-img"
4  <img src="wide.png" class="img-fluid" />        class="img-fluid"
   img-fluid: 3 of 4                                  img-fluid: 3 of 4

Full cargo xtask verify passes (all 14 steps, WASM and hub-client included). Workspace suite: 13578 passed, 199 skipped — +28 against main, exactly the 28 tests added here.

The preview path is covered separately, since q2 preview and hub-client hand serialized AST to a React renderer rather than emitting HTML: three tests against render_qmd_to_preview_ast pin that q2-preview tags, honors the height exclusion, and q2-slides does not tag. The rest of that chain was traced rather than assumed — Image.tsx copies AST classes onto the <img> via className, and the preview iframe loads the compiled Bootstrap theme through <link data-q2-theme>.

Not included

The figure-img class Quarto 1 also puts on figure images. Despite sitting one class away in the output it comes from elsewhere — the DOM postprocessor in format-html-bootstrap.ts that sweeps blockquote, figure and figure img in a single pass — and it is cosmetic (margin-bottom: .5rem; line-height: 1). Per the repo's no-DOM-postprocessor rule that sweep needs its own AST transform, and splitting one of its three classes off into this one would be worse than deferring all three together. A follow-up should also fold in Quarto 1's third img-fluid site (format-html-bootstrap.ts:333-341), which force-tags margin-column images, deliberately overriding the height exclusion.

Also spotted but out of scope: TableBootstrapClassTransform leaks {.caption-top .table} into llms companions the same way img-fluid did. That predates this branch.

Fixes bd-images-no-max-width-e5ywgnma

…gnma)

An image with no author-supplied size laid out at its intrinsic pixel
width, because q2 emitted every body image with no class at all. A
2680x1720 screenshot in an 850px content column rendered 2680px wide,
overrunning the margin and putting a horizontal scrollbar on the page.
`.img-fluid{max-width:100%;height:auto}` was already in the compiled
theme -- only the pass that applies it was missing.

Add ResponsiveImageTransform, a port of Quarto 1's
quarto-post/responsive.lua, in the Finalization phase beside
TableBootstrapClassTransform. Per-image it keeps Q1's two exclusions:
an explicit `height`, which `height:auto` would override, and
`data-no-responsive`. An explicit `width` is deliberately not an
exclusion -- `max-width:100%` refines a fixed width rather than
fighting it.

The document switch is `fig-responsive`, and its default is not simply
"on for HTML". Q1's filter tests isHtmlOutput() *and* reads
param('fig-responsive'), whose default is per-format:
createHtmlPresentationFormat sets it false for HTML presentations, so
revealjs is untagged despite isHtmlOutput() being true, and
format-html.ts does the same for `minimal: true`. Both are suppliers of
last resort, so an explicit `fig-responsive: true` still wins. The
`minimal` check reads the raw flag, not is_minimal_html, which also
fires on `theme: none` where both engines do tag.

Also strip `img-fluid` in the llms.txt sanitizer. The pass runs
immediately before LlmsCaptureTransform, so without it every markdown
companion carried `![cap](x.png){.img-fluid}` -- Bootstrap presentation
in output whose purpose is clean semantic markdown.

Not ported: the `figure-img` class Q1 also puts on figure images. It
comes from a DOM postprocessor in format-html-bootstrap.ts that sweeps
blockquote/figure/figure-img together, and is cosmetic; porting one of
its three classes alone would be worse than deferring the sweep.
@posit-snyk-bot

posit-snyk-bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@gordonwoodhull
gordonwoodhull merged commit 9ad1d44 into main Sep 2, 2026
10 checks passed
@gordonwoodhull
gordonwoodhull deleted the bugfix/bd-images-no-max-width-e5ywgnma-images-no-max-width branch September 2, 2026 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants