Commit 07fae6a4 authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Fix decision of whether to drop y value for 2d translate

Spec requires dropping y value for 2d translate if it's zero (*). The
current implementation contains bug that:

1. It rounds the value before checking, which results in incorrect
   dropping of '0.1px'

2. It calls DoubleValue() on calc() without type checking, while
   DoubleValue() on calc() is meaningless when the calc() doesn't
   resolve to a simple numeric value (e.g., 1px - 1%). This results in
   incorrect dropping of some calc().

Both are fixed by this patch.

This is also a preparation for adding DCHECK in DoubleValue() to ensure
that it's called only when calc() can be resolved into a simple numeric
value.

Bug: 979895
Change-Id: Ie8b846729b91e55006485a25fab0b65533983eac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1692202Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675708}
parent 2126ad77
...@@ -144,4 +144,10 @@ double CSSMathFunctionValue::ClampToPermittedRange(double value) const { ...@@ -144,4 +144,10 @@ double CSSMathFunctionValue::ClampToPermittedRange(double value) const {
return IsNonNegative() && value < 0 ? 0 : value; return IsNonNegative() && value < 0 ? 0 : value;
} }
bool CSSMathFunctionValue::IsZero() const {
if (expression_->ResolvedUnitType() == UnitType::kUnknown)
return false;
return expression_->IsZero();
}
} // namespace blink } // namespace blink
...@@ -40,6 +40,8 @@ class CORE_EXPORT CSSMathFunctionValue : public CSSPrimitiveValue { ...@@ -40,6 +40,8 @@ class CORE_EXPORT CSSMathFunctionValue : public CSSPrimitiveValue {
return IsNonNegative() ? kValueRangeNonNegative : kValueRangeAll; return IsNonNegative() ? kValueRangeNonNegative : kValueRangeAll;
} }
bool IsZero() const;
// TODO(crbug.com/979895): Make sure these functions are called only when // TODO(crbug.com/979895): Make sure these functions are called only when
// the math expression resolves into a double value. // the math expression resolves into a double value.
double DoubleValue() const; double DoubleValue() const;
......
...@@ -18,6 +18,8 @@ class CORE_EXPORT CSSNumericLiteralValue : public CSSPrimitiveValue { ...@@ -18,6 +18,8 @@ class CORE_EXPORT CSSNumericLiteralValue : public CSSPrimitiveValue {
CSSNumericLiteralValue(double num, UnitType type); CSSNumericLiteralValue(double num, UnitType type);
bool IsZero() const { return !DoubleValue(); }
double DoubleValue() const { return num_; } double DoubleValue() const { return num_; }
double ComputeSeconds() const; double ComputeSeconds() const;
double ComputeDegrees() const; double ComputeDegrees() const;
......
...@@ -301,6 +301,11 @@ double CSSPrimitiveValue::GetDoubleValue() const { ...@@ -301,6 +301,11 @@ double CSSPrimitiveValue::GetDoubleValue() const {
: To<CSSNumericLiteralValue>(this)->DoubleValue(); : To<CSSNumericLiteralValue>(this)->DoubleValue();
} }
bool CSSPrimitiveValue::IsZero() const {
return IsCalculated() ? To<CSSMathFunctionValue>(this)->IsZero()
: To<CSSNumericLiteralValue>(this)->IsZero();
}
CSSPrimitiveValue::UnitType CSSPrimitiveValue::CanonicalUnitTypeForCategory( CSSPrimitiveValue::UnitType CSSPrimitiveValue::CanonicalUnitTypeForCategory(
UnitCategory category) { UnitCategory category) {
// The canonical unit type is chosen according to the way // The canonical unit type is chosen according to the way
......
...@@ -228,6 +228,8 @@ class CORE_EXPORT CSSPrimitiveValue : public CSSValue { ...@@ -228,6 +228,8 @@ class CORE_EXPORT CSSPrimitiveValue : public CSSValue {
// Converts to a Length (Fixed, Percent or Calculated) // Converts to a Length (Fixed, Percent or Calculated)
Length ConvertToLength(const CSSToLengthConversionData&) const; Length ConvertToLength(const CSSToLengthConversionData&) const;
bool IsZero() const;
// TODO(crbug.com/979895): Make sure this functions are called only when // TODO(crbug.com/979895): Make sure this functions are called only when
// applicable: on a numeric literal, or on a calc() that can be resolved into // applicable: on a numeric literal, or on a calc() that can be resolved into
// a single value without extra context (e.g., CSSToLengthConversionData). // a single value without extra context (e.g., CSSToLengthConversionData).
......
...@@ -6685,7 +6685,7 @@ const CSSValue* Translate::ParseSingleValue( ...@@ -6685,7 +6685,7 @@ const CSSValue* Translate::ParseSingleValue(
if (translate_y) { if (translate_y) {
CSSValue* translate_z = css_property_parser_helpers::ConsumeLength( CSSValue* translate_z = css_property_parser_helpers::ConsumeLength(
range, context.Mode(), kValueRangeAll); range, context.Mode(), kValueRangeAll);
if (translate_y->GetIntValue() == 0 && !translate_z) if (translate_y->IsZero() && !translate_z)
return list; return list;
list->Append(*translate_y); list->Append(*translate_y);
......
...@@ -18,7 +18,9 @@ test_valid_value("translate", "0px"); ...@@ -18,7 +18,9 @@ test_valid_value("translate", "0px");
test_valid_value("translate", "100%"); test_valid_value("translate", "100%");
test_valid_value("translate", "100px 0px", "100px"); test_valid_value("translate", "100px 0px", "100px");
test_valid_value("translate", "100px 0.1px", "100px 0.1px");
test_valid_value("translate", "100px 0%", "100px"); test_valid_value("translate", "100px 0%", "100px");
test_valid_value("translate", "100px calc(10px - 10%)", "100px calc(10px - 10%)");
test_valid_value("translate", "100px 200%"); test_valid_value("translate", "100px 200%");
test_valid_value("translate", "100% 200px"); test_valid_value("translate", "100% 200px");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment