feat(publisher): derive <img sizes> from the layout — remove the manual Sizes field#45
Merged
Merged
Conversation
…nts-only srcset Lighthouse kept reporting huge PNG downloads despite a full WebP srcset. Root cause: buildMediaSrcset appended the ORIGINAL file as the largest srcset candidate, and the variant ladder capped at 2048w. With sizes="1280px", any display with DPR >= 1.7 needs > 2048 device px, so the browser's only candidate big enough was the original — a 7.4 MB PNG in the reported case (122 KB as its 2048w WebP). Lighthouse mobile emulates DPR 2.625, so it selected the PNG on every image, every run. The fix, in three parts: - srcset is built from VARIANTS ONLY — the original never appears as a candidate. Applied to the publisher (buildMediaSrcset) and to the admin twin (buildVariantSrcset: editor canvas, media viewer, video poster preview), whose fallthrough now also prefers the largest variant over the original. The original survives in `src` only, which width-descriptor srcsets reserve for non-srcset browsers. - the variant ladder gains one rung AT the source's intrinsic width, so the srcset's top candidate is a full-quality WebP and dropping the original costs no quality ceiling. The rung is clamped to WebP's hard 16383px output cap (a 900x17000 screenshot encodes a clamped rung instead of failing the whole job), and images smaller than every target width keep zero variants — small pixel-art icons still publish as pixel-exact plain `src`, never force-re-encoded to lossy WebP. The Tier-3 delegate path intentionally emits declared widths only: the host must not synthesize URLs outside the plugin's schema-bounded (16..8192) widths contract. - lazy images with the default sizes='auto' now emit `sizes="auto, <ancestor-cap-or-100vw>"` — Chrome 121+ selects by the image's actual rendered width (grids/cards stop over-fetching); everyone else parses past the unknown keyword to the fallback. The spec restricts `auto` to lazy images, so eager images emit the fallback alone, and author-supplied sizes stay verbatim. Verified live end-to-end: published markup is WebP-only srcset topped by the intrinsic rung with sizes="auto, 100vw", and a DPR-2 browser at 1280px viewport downloaded the 20 KB intrinsic WebP — the PNG was never requested. Existing assets (uploaded before this change) have no intrinsic rung; their srcset now tops at the 2048w WebP, which is still strictly better than the original on every display. Re-upload to get the full-resolution top rung. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…xact math The publisher generates the site's CSS, so it can compute what the browser will lay out instead of asking the author to predict it. The image module's "Sizes" text field is gone; the resolver always runs. The v1 resolver only understood ancestor PIXEL caps — a 3-column grid in a 1280px container resolved to sizes="1280px", over-fetching ~3x on every non-Chromium browser (and on eager images everywhere). The new model (src/core/publisher/sizesResolver.ts) walks root→image and expresses each node's width as min(...) over LINEAR functions of the viewport (a·vw + b px), plus a px floor from min-width emitted as max(...): px/%/vw width+maxWidth, px paddings (border-box), and grid column tracks (px/%/fr, literal repeat(), column gaps) all compose exactly — the same grid emits sizes="auto, min(33.33vw - 16px, 410.67px)" and a DPR-2 browser downloads the 6KB w1024 rung instead of the 2048w one. Cascade fidelity (each verified against real browser layout during the adversarial review of this change): - classes merge in styleRule.order — the published stylesheet's source order — not classIds order; node.inlineStyles merge last (they ship as a literal style attribute and outrank every class) - percentage grid tracks resolve against the FULL content box (gaps overflow, they don't shrink % tracks) - grid placement counts only RENDERED siblings (hidden nodes occupy no cell); unequal track lists bail to the container width when placement is unpredictable (sibling gridColumn spans, base.loop grids whose copies round-robin the tracks); equal track lists are exact regardless - min-width floors emit max(Npx, ...) instead of being ignored; a non-px floor skips that node's narrowing - tier collapse only under uniformly-nested media queries (all max-width or all min-width) — mixed directions cover disjoint ranges where dropping an equal-valued tier falls through to the wrong value - degenerate inputs (overflowing % tracks) can no longer emit negative lengths, and candidate sets cap at 4 min() terms (dropping terms only ever over-estimates) Everything the model can't express degrades in the safe direction — the estimate may only grow (heavier download), never shrink (blurry render). Flex rows stay content-driven and keep the container width. Removing the sizes prop: nothing else wrote it (HTML import strips it as a module-generated attribute), the editor field was a raw HTML attribute string no visual-editor user could fill confidently, and stored values are ignored harmlessly. Lazy images keep the `auto, <resolved>` form so Chromium selects by true rendered width even where the estimate bailed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…izes # Conflicts: # docs/features/media.md # src/__tests__/modules/imageResponsiveAttrs.test.ts # src/modules/base/image/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
sizesbecomes fully automatic and invisible to the user, as it should be in a visual editor: the publisher generates the site's CSS, so it computes the rendered width itself instead of asking the author to predict it. The "Sizes" text field is removed from the image module settings (nothing else wrote that prop — HTML import strips it as module-generated; stored values are ignored harmlessly).How
The new resolver (
src/core/publisher/sizesResolver.ts) walks root→image and models each node's width asmin(…)over linear functions of the viewport (a·vw + b px) plus amax()floor formin-width. Pixel caps,%/vwwidths, px paddings, and grid column tracks (px/%/fr with gaps) all stay linear, so the emitted value is exact CSS math, per breakpoint tier:Live measured: the same 3-col grid page at DPR 2 / 1280px viewport downloaded the 6 KB w1024 WebP. The chain for that layout: original bug → multi-MB PNG; #40 → ~122 KB (2048w); this PR → 6 KB. Lazy images keep the
auto,prefix so Chromium selects by true rendered width even where the estimate had to bail.Safety invariant
Anything the model can't express degrades by over-estimating (heavier download, never blurry): flex rows, auto-fit/minmax grids, sibling
gridColumnspans,base.loopgrids with unequal tracks, non-px paddings/floors.The multi-agent adversarial review of the first draft found 9 real defects, 6 of them violations of exactly that invariant — each one empirically reproduced by the verifiers against real browser layout, and all fixed + regression-tested here:
%grid tracks resolve against the full content box (gaps overflow; they don't shrink % tracks)styleRule.order(the published stylesheet's source order), not classIds ordernode.inlineStyles(a literalstyleattribute in published HTML) now outrank classes in the model toomin-widthfloors emitmax(Npx, …)instead of being silently ignoredmin()terms (multi-KB attributes were reproducible from deep %-cap chains)Verification
bun test(5375 pass / 0 fail),bun run build,bun run lintgreen.🤖 Generated with Claude Code