Fix line_height() crash on calc() values - #2831
Merged
Merged
Conversation
line_height() did `elif not value.unit:`, but a calc() value is a FunctionBlock with no `unit` attribute, so laying out text with `line-height: calc(...)` raised `AttributeError: 'FunctionBlock' object has no attribute 'unit'`. Add a check_math() branch (mirroring length() and vertical_align()) that resolves the calc() to a length or number, with percentages referring to the font size like the existing percentage branch. Fixes Kozea#2812.
Computed values are currently solved later for other properties, and we do that for line-height for consistency purpose. We should solve some of them here (including line-height), where the inherited value is the computed length and not the percentage. But it should be done in a consistent way, not for each property. See https://www.w3.org/TR/css-values-4/#calc-computed-value.
Member
|
Thanks for the pull request. I did the resolution later, not in the computed value, for consistency purpose. |
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
2 participants
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.
Summary
line-height: calc(...)crashes when laying out text. This is the same class of bug as #2673 inlength(), but inline_height(), which was missed by the fix in #2675.A
calc()value is aFunctionBlock, which has no.unitattribute, soelif not value.unit:raisesAttributeError. The sibling functionslength()andvertical_align()already guard withcheck_math(value);line_height()did not.Fix
weasyprint/css/computed_values.py— add acheck_math(value)branch toline_height(), mirroringlength()andvertical_align(). Becauseline_height()must return a resolved('NUMBER', …)/('PIXELS', …)value (its result is unpacked instrut()), the branch resolves thecalc()viaresolve_math()with percentages referring to the font size, matching the existing percentage branch. The resolved value then flows through the normal number / percentage / length handling, socalc()line-heights compute identically to their literal equivalents (e.g.calc(1.2em + 2px)→ 14px atfont-size: 10px, andcalc(1.5 * 1)→ the number 1.5).Tests
Added
test_math_functions_line_heightintests/css/test_math.py, parametrized over calc line-heights containing percentages, font units, lengths, and bare numbers. Each renders a paragraph with text (soline_height()is exercised during layout viastrut()). The test fails onmainwith theAttributeErrorabove and passes with the fix.pytest tests/css/— all passruff check— cleanDisclosure: prepared with AI assistance; reviewed and verified locally.