LibWeb: Crash on CSS calc() with multiplication/division on dimensions

[sw]

Crashing website: https://www.oev-info.ch/de

I'm new to the project and have been trying to investigate this, so below is my speculation:

The CSS somewhere has calc(18em / 18em), which is parsed to ProductCalculationNode{18em, InvertCalculationNode{18em}}. This then causes a verification failure in CSSMathValue::CalculationResult::multiply_by.

https://www.w3.org/TR/css-values-4/#calc-simplification says about simplification:

7. If root is an Invert node:
   1. If root’s child is a number (not a percentage or dimension) return the reciprocal of the child’s value.
   2. If root’s child is an Invert node, return the child’s child.
   3. Return root.

I'm interpreting this to mean that InvertCalculationNode should only be used on Integer or Number.

But https://drafts.css-houdini.org/css-typed-om-1/ has a more sophisticated approach where you have to keep track of the exponent of the type. It looks like CSSNumericType supports these exponents, but CSSMathValue doesn't.

[AtkinsSJ]

Hi there, thanks for the report!

I'm interpreting this to mean that InvertCalculationNode should only be used on Integer or Number.

I don't think that's the correct interpretation. It checks for integers and numbers in step i. If it's not one, it carries on to step ii. Dimensions and percentages will reach step iii and be returned directly.

Basically, you don't attempt to simplify a dimension value directly, but do so in step 9.4. (All invert nodes end up inside product nodes during parsing, IIRC.)

As for the crash, CalculationResult::multiply_by is just wrong, basically. The spec rules about what is allowed in various places in a calc() changed over time (you previously could only divide by a number/integer) and we haven't updated everything to match. A little bit of work has been done to implement that typed-oom type exponents like you saw, but we've got a way to go. :sweat_smile: (We don't do simplification at all, it's on my list to get too soon.)

It's worth noting that the w3.org specs are often a bit outdated, so I'd recommend looking at https://drafts.csswg.org/css-values/#calc-simplification instead.

[sw]

The crash on https://www.oev-info.ch/de no longer occurs beginning with 69d0646, though that didn't touch CalculationResult::multiply_by. ~There is however a new crash in GridFormattingContext on that site introduced with 32c467c by @kalenikaliaksandr.~ Fixed by 11e10d0

[shlyakpavel]

So, as the crash no longer occurs, can we close the issue?

[sw]

According to @AtkinsSJ, CalculationResult::multiply_by needs updating, so maybe keep this issue open for that?

Playing around with the mentioned web site, I found another crash with fd3f8b7, see attached files crash.html.txt crash.txt. With a lot of changes to the codebase going on, is it useful if I open more issues for these kind of bugs?

[shlyakpavel]

is it useful if I open more issues for these kind of bugs?

Yes, especially if they include minimal reproductions. This helps me and other contributors identify and address issues more easily.

With a lot of changes to the codebase going on

Absolutely, core team members are aware of this. However, casual contributors might not be as informed. Having dedicated issues (such as bitmaps not rendering or page crashes) makes it easier for them to contribute.

[AtkinsSJ]

I think as the crash itself is gone this can be closed.