Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[CSS-Typed-OM] Cannot set z-index property to -3.14
https://bugs.webkit.org/show_bug.cgi?id=249788

Reviewed by Antti Koivisto.

We couldn't set the `z-index` property to -3.14 using the CSS Typed OM API. We
would expect -3 as computed value, however, WebKit was returning 0.

* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/background-size-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/font-weight-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/order-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/z-index-expected.txt:
Rebaseline WPT tests now that more checks are passing.

* Source/WebCore/css/CSSToStyleMap.cpp:
(WebCore::CSSToStyleMap::mapAnimationDuration):
Make sure we clamp the duration to [0, inf] to avoid hitting an assertion later
on. We can end up with a negative duration when using calc().

* Source/WebCore/css/calc/CSSCalcValue.h:
Fix confusingly named parameter. It did the exact opposite of that it claimed.

* Source/WebCore/css/typedom/CSSUnitValue.cpp:
(WebCore::CSSUnitValue::toCSSValueWithProperty const):
When we wrap the out of range value in a calc(), make sure we allow negative
values, like we were trying to do (but the parameter's name didn't match the
actual behavior).

Canonical link: https://commits.webkit.org/258265@main
  • Loading branch information
cdumez committed Dec 22, 2022
1 parent c639ec2 commit c4ae09b
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 14 deletions.
Expand Up @@ -9,7 +9,7 @@ PASS Can set 'background-size' to a length: -3.14em
PASS Can set 'background-size' to a length: 3.14cm
PASS Can set 'background-size' to a length: calc(0px + 0em)
PASS Can set 'background-size' to a percent: 0%
FAIL Can set 'background-size' to a percent: -3.14% assert_approx_equals: expected -3.14 +/- 0.000001 but got 0
PASS Can set 'background-size' to a percent: -3.14%
PASS Can set 'background-size' to a percent: 3.14%
PASS Can set 'background-size' to a percent: calc(0% + 0%)
PASS Can set 'background-size' to the 'auto' keyword: auto
Expand Down
Expand Up @@ -9,7 +9,7 @@ PASS Can set 'font-weight' to the 'bold' keyword: bold
PASS Can set 'font-weight' to the 'bolder' keyword: bolder
PASS Can set 'font-weight' to the 'lighter' keyword: lighter
PASS Can set 'font-weight' to a number: 0
FAIL Can set 'font-weight' to a number: -3.14 assert_approx_equals: expected -3.14 +/- 0.000001 but got 0
FAIL Can set 'font-weight' to a number: -3.14 assert_approx_equals: expected -3.14 +/- 0.000001 but got -3
FAIL Can set 'font-weight' to a number: 3.14 assert_approx_equals: expected 3.14 +/- 0.000001 but got 3
PASS Can set 'font-weight' to a number: calc(2 + 3)
PASS Setting 'font-weight' to a length: 0px throws TypeError
Expand Down
Expand Up @@ -5,7 +5,7 @@ PASS Can set 'order' to CSS-wide keywords: unset
PASS Can set 'order' to CSS-wide keywords: revert
PASS Can set 'order' to var() references: var(--A)
PASS Can set 'order' to a number: 0
FAIL Can set 'order' to a number: -3.14 assert_approx_equals: expected -3 +/- 0.000001 but got 0
PASS Can set 'order' to a number: -3.14
PASS Can set 'order' to a number: 3.14
PASS Can set 'order' to a number: calc(2 + 3)
PASS Setting 'order' to a length: 0px throws TypeError
Expand Down
Expand Up @@ -6,7 +6,7 @@ PASS Can set 'z-index' to CSS-wide keywords: revert
PASS Can set 'z-index' to var() references: var(--A)
PASS Can set 'z-index' to the 'auto' keyword: auto
PASS Can set 'z-index' to a number: 0
FAIL Can set 'z-index' to a number: -3.14 assert_approx_equals: expected -3 +/- 0.000001 but got 0
PASS Can set 'z-index' to a number: -3.14
PASS Can set 'z-index' to a number: 3.14
PASS Can set 'z-index' to a number: calc(2 + 3)
PASS Setting 'z-index' to a length: 0px throws TypeError
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/css/CSSToStyleMap.cpp
Expand Up @@ -340,7 +340,8 @@ void CSSToStyleMap::mapAnimationDuration(Animation& animation, const CSSValue& v
if (!is<CSSPrimitiveValue>(value))
return;

animation.setDuration(downcast<CSSPrimitiveValue>(value).computeTime<double, CSSPrimitiveValue::Seconds>());
auto duration = std::max<double>(downcast<CSSPrimitiveValue>(value).computeTime<double, CSSPrimitiveValue::Seconds>(), 0);
animation.setDuration(duration);
}

void CSSToStyleMap::mapAnimationFillMode(Animation& layer, const CSSValue& value)
Expand Down
12 changes: 6 additions & 6 deletions Source/WebCore/css/calc/CSSCalcValue.cpp
Expand Up @@ -272,10 +272,10 @@ static RefPtr<CSSCalcExpressionNode> createCSS(const Length& length, const Rende
return nullptr;
}

CSSCalcValue::CSSCalcValue(Ref<CSSCalcExpressionNode>&& expression, bool shouldClampToNonNegative)
CSSCalcValue::CSSCalcValue(Ref<CSSCalcExpressionNode>&& expression, ShouldClampToNonNegative shouldClampToNonNegative)
: CSSValue(CalculationClass)
, m_expression(WTFMove(expression))
, m_shouldClampToNonNegative(shouldClampToNonNegative)
, m_shouldClampToNonNegative(shouldClampToNonNegative == ShouldClampToNonNegative::Yes)
{
}

Expand Down Expand Up @@ -397,7 +397,7 @@ RefPtr<CSSCalcValue> CSSCalcValue::create(CSSValueID function, const CSSParserTo
auto expression = parser.parseCalc(tokens, function, allowsNegativePercentage);
if (!expression)
return nullptr;
auto result = adoptRef(new CSSCalcValue(expression.releaseNonNull(), range != ValueRange::All));
auto result = adoptRef(new CSSCalcValue(expression.releaseNonNull(), range != ValueRange::All ? ShouldClampToNonNegative::Yes : ShouldClampToNonNegative::No));
LOG_WITH_STREAM(Calc, stream << "CSSCalcValue::create " << *result);
return result;
}
Expand All @@ -415,14 +415,14 @@ RefPtr<CSSCalcValue> CSSCalcValue::create(const CalculationValue& value, const R

auto simplifiedExpression = CSSCalcOperationNode::simplify(expression.releaseNonNull());

auto result = adoptRef(new CSSCalcValue(WTFMove(simplifiedExpression), value.shouldClampToNonNegative()));
auto result = adoptRef(new CSSCalcValue(WTFMove(simplifiedExpression), value.shouldClampToNonNegative() ? ShouldClampToNonNegative::Yes : ShouldClampToNonNegative::No));
LOG_WITH_STREAM(Calc, stream << "CSSCalcValue::create from CalculationValue: " << *result);
return result;
}

RefPtr<CSSCalcValue> CSSCalcValue::create(Ref<CSSCalcExpressionNode>&& node, bool allowsNegativePercentage)
RefPtr<CSSCalcValue> CSSCalcValue::create(Ref<CSSCalcExpressionNode>&& node, ShouldClampToNonNegative shouldClampToNonNegative)
{
return adoptRef(*new CSSCalcValue(WTFMove(node), allowsNegativePercentage));
return adoptRef(*new CSSCalcValue(WTFMove(node), shouldClampToNonNegative));
}

TextStream& operator<<(TextStream& ts, const CSSCalcValue& value)
Expand Down
6 changes: 4 additions & 2 deletions Source/WebCore/css/calc/CSSCalcValue.h
Expand Up @@ -48,12 +48,14 @@ enum class CSSUnitType : uint8_t;
enum class CalculationCategory : uint8_t;
enum class ValueRange : uint8_t;

enum class ShouldClampToNonNegative : bool { No, Yes };

class CSSCalcValue final : public CSSValue {
public:
static RefPtr<CSSCalcValue> create(CSSValueID function, const CSSParserTokenRange&, CalculationCategory destinationCategory, ValueRange, const CSSCalcSymbolTable&, bool allowsNegativePercentage = false);
static RefPtr<CSSCalcValue> create(CSSValueID function, const CSSParserTokenRange&, CalculationCategory destinationCategory, ValueRange);
static RefPtr<CSSCalcValue> create(const CalculationValue&, const RenderStyle&);
static RefPtr<CSSCalcValue> create(Ref<CSSCalcExpressionNode>&&, bool allowsNegativePercentage = false);
static RefPtr<CSSCalcValue> create(Ref<CSSCalcExpressionNode>&&, ShouldClampToNonNegative = ShouldClampToNonNegative::No);
~CSSCalcValue();

CalculationCategory category() const;
Expand All @@ -79,7 +81,7 @@ class CSSCalcValue final : public CSSValue {
const CSSCalcExpressionNode& expressionNode() const { return m_expression; }

private:
CSSCalcValue(Ref<CSSCalcExpressionNode>&&, bool shouldClampToNonNegative);
CSSCalcValue(Ref<CSSCalcExpressionNode>&&, ShouldClampToNonNegative);

double clampToPermittedRange(double) const;

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/css/typedom/CSSUnitValue.cpp
Expand Up @@ -271,7 +271,7 @@ RefPtr<CSSValue> CSSUnitValue::toCSSValueWithProperty(CSSPropertyID propertyID)
auto sumNode = CSSCalcOperationNode::createSum(Vector { node.releaseNonNull() });
if (!sumNode)
return nullptr;
return CSSPrimitiveValue::create(CSSCalcValue::create(sumNode.releaseNonNull(), true /* allowsNegativePercentage */));
return CSSPrimitiveValue::create(CSSCalcValue::create(sumNode.releaseNonNull()));
}
return toCSSValue();
}
Expand Down

0 comments on commit c4ae09b

Please sign in to comment.