border-image-width computed values should be a calc() value if it contains a percentage

• Component: CSS
• Status: RESOLVED FIXED

[Antoine Quint]

border-image-width computed values should be a calc() value if it contains a percentage

[Antoine Quint]

Created attachment 425713 [details]
Patch

[Sam Weinig]

Comment on attachment 425713 [details]
Patch

The spec has some odd language here: (https://drafts.csswg.org/css-values-3/#calc-computed-value)

"Where percentages are not resolved at computed-value time, they are not resolved in calc() expressions, e.g. calc(100% - 100% + 1em) resolves to calc(1em + 0%), not to 1em. If there are special rules for computing percentages in a value (e.g. the height property), they apply whenever a calc() expression contains percentages."

"For example, whereas font-size computes percentage values at computed value time so that font-relative length units can be computed, background-position has layout-dependent behavior for percentage values, and thus does not resolve percentages until used-value time.

Due to this, background-position computation preserves the percentage in a calc() whereas font-size will compute such expressions directly into a length."

From that, it kind of sounds like this check needs to be a bit more involved to differentiate between the different variations here.

[Antoine Quint]

(In reply to Sam Weinig from comment #2)
> Comment on attachment 425713 [details]
> Patch
> 
> The spec has some odd language here:
> (https://drafts.csswg.org/css-values-3/#calc-computed-value)
> 
> "Where percentages are not resolved at computed-value time, they are not
> resolved in calc() expressions, e.g. calc(100% - 100% + 1em) resolves to
> calc(1em + 0%), not to 1em. If there are special rules for computing
> percentages in a value (e.g. the height property), they apply whenever a
> calc() expression contains percentages."
> 
> "For example, whereas font-size computes percentage values at computed value
> time so that font-relative length units can be computed, background-position
> has layout-dependent behavior for percentage values, and thus does not
> resolve percentages until used-value time.
> 
> Due to this, background-position computation preserves the percentage in a
> calc() whereas font-size will compute such expressions directly into a
> length."
> 
> From that, it kind of sounds like this check needs to be a bit more involved
> to differentiate between the different variations here.

I haven't looked into it that much, but the computed value tests for font-size and background-position have PASS results for calc() values. The only WPT test that changes behavior with this change is the border-image-width one. Maybe I'm mistaken, but it looks like such checks either happen higher up the chain, or they are sufficient.

[Antoine Quint]

In the case of font-size, for instance, we do this:

static Ref<CSSPrimitiveValue> fontSizeFromStyle(const RenderStyle& style)
{
    return zoomAdjustedPixelValue(style.fontDescription().computedSize(), style);
}

My guess is that properties that ought not to serialize as `calc()` simply remove the calculated Length from the equation when returning the style. When we do have a calc() value, such as in the border-image-width scenario, we merely need to ensure we preserve percentage values in calc() operations and not resolve them.

[Antti Koivisto]

Comment on attachment 425713 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425713&action=review

> Source/WebCore/css/CSSPrimitiveValue.cpp:555
>  template<> Length CSSPrimitiveValue::computeLength(const CSSToLengthConversionData& conversionData) const
>  {
> +    if (primitiveUnitType() == CSSUnitType::CSS_CALC) {
> +        auto& cssCalcValue = *m_value.calc;
> +        auto calcCategory = cssCalcValue.category();
> +        if (calcCategory == CalculationCategory::Percent
> +            || calcCategory == CalculationCategory::PercentNumber
> +            || calcCategory == CalculationCategory::PercentLength)
> +            return Length(cssCalcValue.createCalculationValue(conversionData));
> +    }
> +
>      return Length(clampTo<float>(computeLengthDouble(conversionData), minValueForCssLength, maxValueForCssLength), LengthType::Fixed);
>  }

It seems very random to have this test in one version of computeLength. Surely there is some more generic problem here?

[Antoine Quint]

Looking at background-position-x, which serializes values the same way as `border-image-width`, we end up calling this code under `BuilderConverter::convertLength(const BuilderState& builderState, const CSSValue& value)`:

    if (primitiveValue.isCalculatedPercentageWithLength())
        return Length(primitiveValue.cssCalcValue()->createCalculationValue(conversionData));

We need to do something similar in `LengthBox CSSToStyleMap::mapNinePieceImageQuad(CSSValue& value)`.

[Antoine Quint]

Created attachment 425767 [details]
Patch

[EWS]

Committed r275834 (236403@main): <https://commits.webkit.org/236403@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425767 [details].

[Radar WebKit Bug Importer]

<rdar://problem/76552120>