fix(render): measure page height from content, not the body box - #131
Merged
Conversation
pixelshot clamped the page height to `body.getBoundingClientRect().bottom`,
which only bounds the document when the page lets the body size to its
content. Sites that pin it to the viewport — `html, body { height: 100% }`,
as Wikipedia's Vector 2022 skin does — leave the content overflowing a
one-viewport box, so a 19,951px article measured 1,568px and captured a
single tile with `complete: true` (issue #124).
The rect was also read as a document coordinate while being
viewport-relative. A URL with a `#fragment` loads already scrolled, where
the bottom edge is negative and `Math.max(bottom, 1)` floors the whole page
to 1px.
Measure the lowest edge among the body and its element children instead, and
add the scroll offset back. That reads the same as before on a self-sizing
body, so the clamp still drops the blank tail an inflated
`documentElement.scrollHeight` would otherwise buy (root padding, trailing
margin), while surviving a pinned body.
Both capture paths carried their own copy of the probe; the snippet now
lives in one module so they can't drift apart.
Verified in stock Chrome, standard and turbo probes agreeing on every page:
page before after scrollHeight
wikipedia (#124) 1568 19951 19951
wikipedia #fragment 1 19951 19951
simonwillison.net 18252 18252 18252
ourworldindata.org 8713 8713 8713
Closes #124
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
|
I verified the core #124 regression on rulin. What I ran: .venv/bin/python -m pytest tests/test_render.py::test_page_taller_than_a_viewport_bounded_body_is_fully_tiled -q --tb=shortResult: That test exercises the important failure mode: One caveat: the full |
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 breaks
pixelshotclamps the measured page height tobody.getBoundingClientRect().bottom.That bound holds only when the page lets the body size to its content. Wikipedia's
Vector 2022 skin pins
html, bodytoheight: 100%and lets the article overflowvisibly, so a 19,951px page measures 1,568px and captures one tile — with
complete: trueset regardless (#124).The same expression also reads a viewport-relative rect as a document coordinate.
A URL with a
#fragmentloads already scrolled to the anchor, where the bottom edgeis negative and
Math.max(bottom, 1)floors the entire page to 1px.Fix
Measure the lowest edge among the body and its element children, and add the scroll
offset back. On a self-sizing body this reads the same as before, so the clamp still
drops the blank tail an inflated
documentElement.scrollHeightwould otherwise buy(root padding, trailing margin).
Both capture paths carried their own copy of the probe. The snippet now lives in
page_metrics.pyso they can't drift.Verification
Standard and turbo probes, stock Chrome and the patched
headless_shell:The 1553 is the number from the issue report — that path needs the patched
headless_shell, which is linux-x64 only.tests/test_render.pygains a regression case: a page whose body is pinned to theviewport must tile in full.
Closes #124