Skip to content

Commit

Permalink
Support mixed percentage and length/number arguments in CSS step func…
Browse files Browse the repository at this point in the history
…tions

https://bugs.webkit.org/show_bug.cgi?id=259714
rdar://problem/113234898

Reviewed by Tim Nguyen.

We shouldn't just check that the two numeric argumnets to round() etc.
have the same value category, but if one is Percent then we should
support Length or Number as the other, and produce an overall
LengthPercent or NumberPercent category.

Fix another bug in this code, where we currently check that the numeric
arguments aren't isRoundOperation(), but this should be
isRoundConstant(). Otherwise values like round(round(10, 5), 2) are
rejected.

* LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed-expected.txt:
* Source/WebCore/css/calc/CSSCalcOperationNode.cpp:
(WebCore::resolvedTypeForStep):
(WebCore::CSSCalcOperationNode::createStep):
(WebCore::CSSCalcOperationNode::createRound):
(WebCore::validateRoundChildren): Deleted.

Canonical link: https://commits.webkit.org/267072@main
  • Loading branch information
heycam committed Aug 19, 2023
1 parent 31e3bbe commit f452d44
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,32 +116,32 @@ PASS rem(10deg,6deg) should be used-value-equivalent to 4deg
PASS rem(10grad,6grad) should be used-value-equivalent to 4grad
PASS rem(10rad,6rad) should be used-value-equivalent to 4rad
PASS rem(10turn,6turn) should be used-value-equivalent to 4turn
FAIL round(10%,1px) should be used-value-equivalent to 8px assert_not_equals: round(10%,1px) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
FAIL round(10%,5px) should be used-value-equivalent to 10px assert_not_equals: round(10%,5px) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
PASS round(10%,1px) should be used-value-equivalent to 8px
PASS round(10%,5px) should be used-value-equivalent to 10px
PASS round(2rem,5px) should be used-value-equivalent to 30px
PASS round(100px,1rem) should be used-value-equivalent to 96px
PASS round(10s,6000ms) should be used-value-equivalent to 12s
PASS round(10000ms,6s) should be used-value-equivalent to 12s
FAIL mod(10%,1px) should be used-value-equivalent to 0.5px assert_not_equals: mod(10%,1px) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
FAIL mod(10%,5px) should be used-value-equivalent to 2.5px assert_not_equals: mod(10%,5px) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
PASS mod(10%,1px) should be used-value-equivalent to 0.5px
PASS mod(10%,5px) should be used-value-equivalent to 2.5px
PASS mod(2rem,5px) should be used-value-equivalent to 2px
PASS mod(100px,1rem) should be used-value-equivalent to 4px
PASS mod(10s,6000ms) should be used-value-equivalent to 4s
PASS mod(10000ms,6s) should be used-value-equivalent to 4s
FAIL mod(18px,100% / 15) should be used-value-equivalent to 3px assert_not_equals: mod(18px,100% / 15) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
FAIL mod(-18px,100% / 10) should be used-value-equivalent to 4.5px assert_not_equals: mod(-18px,100% / 10) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
PASS mod(18px,100% / 15) should be used-value-equivalent to 3px
PASS mod(-18px,100% / 10) should be used-value-equivalent to 4.5px
PASS mod(18%,5%) should be used-value-equivalent to 3%
PASS mod(-19%,5%) should be used-value-equivalent to 1%
PASS mod(18vw,5vw) should be used-value-equivalent to 3vw
PASS mod(-18vw,5vw) should be used-value-equivalent to 2vw
FAIL rem(10%,1px) should be used-value-equivalent to 0.5px assert_not_equals: rem(10%,1px) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
FAIL rem(10%,5px) should be used-value-equivalent to 2.5px assert_not_equals: rem(10%,5px) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
PASS rem(10%,1px) should be used-value-equivalent to 0.5px
PASS rem(10%,5px) should be used-value-equivalent to 2.5px
PASS rem(2rem,5px) should be used-value-equivalent to 2px
PASS rem(100px,1rem) should be used-value-equivalent to 4px
PASS rem(10s,6000ms) should be used-value-equivalent to 4s
PASS rem(10000ms,6s) should be used-value-equivalent to 4s
FAIL rem(18px,100% / 15) should be used-value-equivalent to 3px assert_not_equals: rem(18px,100% / 15) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
FAIL rem(-18px,100% / 15) should be used-value-equivalent to -3px assert_not_equals: rem(-18px,100% / 15) isn't valid in 'margin-left'; got the default value instead. got disallowed value "0px"
PASS rem(18px,100% / 15) should be used-value-equivalent to 3px
PASS rem(-18px,100% / 15) should be used-value-equivalent to -3px
PASS rem(18vw,5vw) should be used-value-equivalent to 3vw
PASS rem(-18vw,5vw) should be used-value-equivalent to -3vw
PASS calc(round(1px + 0%, 1px + 0%)) should be used-value-equivalent to 1px
Expand Down
73 changes: 41 additions & 32 deletions Source/WebCore/css/calc/CSSCalcOperationNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,23 @@ static CalculationCategory resolvedTypeForMinOrMaxOrClamp(CalculationCategory ca
return CalculationCategory::Other;
}

static std::optional<CalculationCategory> resolvedTypeForStep(CalculationCategory a, CalculationCategory b)
{
if (a == b)
return a;

if (a == CalculationCategory::Percent)
std::swap(a, b);

if (a == CalculationCategory::Length)
return CalculationCategory::PercentLength;

if (a == CalculationCategory::Number)
return CalculationCategory::PercentNumber;

return { };
}

static bool isSamePair(CalculationCategory a, CalculationCategory b, CalculationCategory x, CalculationCategory y)
{
return (a == x && b == y) || (a == y && b == x);
Expand Down Expand Up @@ -536,46 +553,38 @@ RefPtr<CSSCalcOperationNode> CSSCalcOperationNode::createStep(CalcOperator op, V
if (values.size() != 2)
return nullptr;

if (values[0]->category() != values[1]->category()) {
LOG_WITH_STREAM(Calc, stream << "Failed to create stepped value node because unable to determine category from " << prettyPrintNodes(values));
return nullptr;
}
return adoptRef(new CSSCalcOperationNode(values[0]->category(), op, WTFMove(values)));
}
if (auto category = resolvedTypeForStep(values[0]->category(), values[1]->category()))
return adoptRef(new CSSCalcOperationNode(*category, op, WTFMove(values)));

static bool validateRoundChildren(Vector<Ref<CSSCalcExpressionNode>>& values)
{
// for 3 children 1st node must be round constant
if (values.size() == 3) {
if (!is<CSSCalcOperationNode>(values[0]) || !(downcast<CSSCalcOperationNode>(values[0].get()).isRoundOperation()))
return false;
}
// for 2 children should not have round constant anywhere but first node of 3
for (size_t i = values.size() == 2 ? 0 : 1; i < values.size(); i++) {
if (is<CSSCalcOperationNode>(values[i])) {
if (downcast<CSSCalcOperationNode>(values[i].get()).isRoundConstant())
return false;
}
}
// check that two categories of numerical values are the same
return values.rbegin()[1]->category() == values.rbegin()[0]->category();

LOG_WITH_STREAM(Calc, stream << "Failed to create stepped value node because unable to determine category from " << prettyPrintNodes(values));
return nullptr;
}

RefPtr<CSSCalcOperationNode> CSSCalcOperationNode::createRound(Vector<Ref<CSSCalcExpressionNode>>&& values)
{
if (values.size() != 2 && values.size() != 3)
return nullptr;

if (!validateRoundChildren(values)) {
LOG_WITH_STREAM(Calc, stream << "Failed to create round node because unable to determine category from " << prettyPrintNodes(values));
return nullptr;

auto asRoundOperation = [](Ref<CSSCalcExpressionNode>& node) -> std::optional<CalcOperator> {
if (auto value = dynamicDowncast<CSSCalcOperationNode>(node.get()); value && value->isRoundConstant())
return value->calcOperator();
return { };
};

auto roundOperation = CalcOperator::Nearest;

if (values.size() == 3) {
if (auto operation = asRoundOperation(values[0])) {
roundOperation = *operation;
values.remove(0);
} else
return nullptr;
}
CalcOperator roundType = values.size() == 2 ? CalcOperator::Nearest : downcast<CSSCalcOperationNode>(values[0].get()).calcOperator();
if (values.size() == 3)
values.remove(0);
return adoptRef(new CSSCalcOperationNode(values.rbegin()[0]->category(), roundType, WTFMove(values)));

if (asRoundOperation(values[0]) || asRoundOperation(values[1]))
return nullptr;

return createStep(roundOperation, WTFMove(values));
}

RefPtr<CSSCalcOperationNode> CSSCalcOperationNode::createRoundConstant(CalcOperator op)
Expand Down

0 comments on commit f452d44

Please sign in to comment.