LibWeb/CSS: Fix crash resolving calc with only percentages

[Jaycadox]

Fixes #3006 , (relates to) #648

From some analysis, it seems as if this issue only occurs with a only percentages in a calc.

• calc(10%) crashes.
• calc(10% + 10%) crashes.
• calc(10% + 0px) doesn't crash.
• calc(10vw) doesn't crash.

The resolve function returns a Length in the general case, but returns a Percentage if only percentages are provided.

As a sanity check, I looked at the code before it was changed (commit before change was here), and it only handled the case of percentages and lengths, as I do in this commit.

The old code being:

Optional<Length> CalculatedStyleValue::resolve_length_percentage(Length::ResolutionContext const& resolution_context, Length const& percentage_basis) const
{
    auto result = m_calculation->resolve(resolution_context, percentage_basis);

    return result.value().visit(
        [&](Length const& length) -> Optional<Length> {
            return length;
        },
        [&](Percentage const& percentage) -> Optional<Length> {
            return percentage_basis.percentage_of(percentage);
        },
        [&](auto const&) -> Optional<Length> {
            return {};
        });
}

Not sure if this is perfect, very open to constructive criticism.

[gmta]

Hi @Jaycadox, thanks for looking into this! Could you also add a test to expose the original issue?

[AtkinsSJ]

This isn't correct. If a percentage gets returned (eg, 35%) it'll return that number as a length (35px).

A more correct thing to do would be to return the percentage applied to the percentage_basis in that case.

The question is why this isn't getting converted to a length earlier on. NumericCalculationNode::resolve() should be doing that.

[Jaycadox]

From my analysis, I believe it would actually return the correct resolved pixel value (as percentage_basis was being called on it in NumericCalculationNode::resolve), the issue was purely that it was being reported as the wrong type, and I do think that my original patch, did fix the specific issue, but agree that it didn't fix the underlying problem.

[Jaycadox]

I had a look again at the issue, it seems that NumericCalculationNode::resolve, when given a percentage, would convert it to a pixel value (via percentage_of) but not update the CSSNumericType, so it would stay as Percentage. When putting the expression as a sum, SumCalculationNode would call add_the_types, and I think this would see the expression "Percentage" + Length, and set the output type to Length, thus fixing the bug.

In my new patch, I visit all the variants of the percentage basis and set the CSSNumericType explicitly, which fixes the bug, though I'm not sure if it makes sense to allow all of these types to be a percentage basis. Still open for feedback.

As the original code (which I put in my original message) only handles the case of Length and Percentage, and returns an empty Option if those weren't provided, I would think that it would make sense to only allow those two cases, though it would require some more code modification.

[Jaycadox]

I don't know what CSSNumericType should occur in the case of Number, I assume there isn't a set one and it can change. But this code should probably be corrected if there is one.