Bug 1609737 - Forbid accessing the length and percentage parts of a LengthPercentage separately. r=boris

This is just not a thing you can do if you have min() / max() / etc, as the min
/ max value may depend on the percentage basis.

Differential Revision: https://phabricator.services.mozilla.com/D60168

--HG--
extra : moz-landing-system : lando

accessible/base/StyleInfo.cpp

@@ -37,7 +37,7 @@ void StyleInfo::TextIndent(nsAString& aValue) {
 
   const auto& textIndent = mComputedStyle->StyleText()->mTextIndent;
   if (textIndent.ConvertsToLength()) {
-    aValue.AppendFloat(textIndent.LengthInCSSPixels());
+    aValue.AppendFloat(textIndent.ToLengthInCSSPixels());
     aValue.AppendLiteral("px");
     return;
   }
@@ -46,7 +46,8 @@ void StyleInfo::TextIndent(nsAString& aValue) {
     aValue.AppendLiteral("%");
     return;
   }
-  // FIXME: This doesn't handle calc in any meaningful way?
+  // FIXME: This doesn't handle calc in any meaningful way? Probably should just
+  // use the Servo serialization code...
   aValue.AppendLiteral("0px");
 }
 

layout/style/ServoStyleConstsInlines.h

@@ -563,26 +563,6 @@ LengthPercentage LengthPercentage::FromPercentage(float aPercentage) {
   return l;
 }
 
-CSSCoord LengthPercentage::LengthInCSSPixels() const {
-  if (IsLength()) {
-    return AsLength().ToCSSPixels();
-  }
-  if (IsPercentage()) {
-    return 0;
-  }
-  return AsCalc().length.ToCSSPixels();
-}
-
-float LengthPercentage::Percentage() const {
-  if (IsLength()) {
-    return 0;
-  }
-  if (IsPercentage()) {
-    return AsPercentage()._0;
-  }
-  return AsCalc().percentage._0;
-}
-
 bool LengthPercentage::HasPercent() const {
   return IsPercentage() || (IsCalc() && AsCalc().has_percentage);
 }
@@ -594,6 +574,11 @@ nscoord LengthPercentage::ToLength() const {
   return IsLength() ? AsLength().ToAppUnits() : AsCalc().length.ToAppUnits();
 }
 
+CSSCoord LengthPercentage::ToLengthInCSSPixels() const {
+  MOZ_ASSERT(ConvertsToLength());
+  return IsLength() ? AsLength().ToCSSPixels() : AsCalc().length.ToCSSPixels();
+}
+
 bool LengthPercentage::ConvertsToPercentage() const {
   if (IsPercentage()) {
     return true;
@@ -607,7 +592,10 @@ bool LengthPercentage::ConvertsToPercentage() const {
 
 float LengthPercentage::ToPercentage() const {
   MOZ_ASSERT(ConvertsToPercentage());
-  return Percentage();
+  if (IsPercentage()) {
+    return AsPercentage()._0;
+  }
+  return AsCalc().percentage._0;
 }
 
 bool LengthPercentage::HasLengthAndPercentage() const {
@@ -626,7 +614,14 @@ bool LengthPercentage::IsDefinitelyZero() const {
 }
 
 CSSCoord LengthPercentage::ResolveToCSSPixels(CSSCoord aPercentageBasis) const {
-  return LengthInCSSPixels() + Percentage() * aPercentageBasis;
+  if (IsLength()) {
+    return AsLength().ToCSSPixels();
+  }
+  if (IsPercentage()) {
+    return AsPercentage()._0 * aPercentageBasis;
+  }
+  auto& calc = AsCalc();
+  return calc.length.ToCSSPixels() + calc.percentage._0 * aPercentageBasis;
 }
 
 template <typename T>
@@ -634,7 +629,7 @@ CSSCoord LengthPercentage::ResolveToCSSPixelsWith(T aPercentageGetter) const {
   static_assert(std::is_same<decltype(aPercentageGetter()), CSSCoord>::value,
                 "Should return CSS pixels");
   if (ConvertsToLength()) {
-    return LengthInCSSPixels();
+    return ToLengthInCSSPixels();
   }
   return ResolveToCSSPixels(aPercentageGetter());
 }
@@ -668,15 +663,14 @@ nscoord LengthPercentage::Resolve(nscoord aPercentageBasis) const {
 
 template <typename T>
 nscoord LengthPercentage::Resolve(T aPercentageGetter) const {
-  static_assert(std::is_same<decltype(aPercentageGetter()), nscoord>::value,
-                "Should return app units");
   return Resolve(aPercentageGetter, NSToCoordFloorClamped);
 }
 
 template <typename T>
 nscoord LengthPercentage::Resolve(nscoord aPercentageBasis,
                                   T aPercentageRounder) const {
-  return Resolve([=] { return aPercentageBasis; }, aPercentageRounder);
+  return Resolve([aPercentageBasis] { return aPercentageBasis; },
+                 aPercentageRounder);
 }
 
 void LengthPercentage::ScaleLengthsBy(float aScale) {

layout/style/nsComputedDOMStyle.cpp

@@ -2288,21 +2288,8 @@ void nsComputedDOMStyle::SetValueToLengthPercentage(
     return aValue->SetPercent(result);
   }
 
-  // TODO(emilio): This intentionally matches the serialization of
-  // SetValueToCalc, but goes against the spec. Update this when possible.
-  RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-  nsAutoString tmp, result;
-  result.AppendLiteral("calc(");
-  val->SetAppUnits(CSSPixel::ToAppUnits(aLength.LengthInCSSPixels()));
-  val->GetCssText(tmp);
-  result.Append(tmp);
-  if (aLength.HasPercent()) {
-    result.AppendLiteral(" + ");
-    val->SetPercent(aLength.Percentage());
-    val->GetCssText(tmp);
-    result.Append(tmp);
-  }
-  result.Append(')');
+  nsAutoString result;
+  Servo_LengthPercentage_ToCss(&aLength, &result);
   aValue->SetString(result);
 }
 

layout/style/nsStyleUtil.cpp

@@ -256,18 +256,15 @@ static bool ObjectPositionCoordMightCauseOverflow(
   // Any nonzero length in "object-position" can push us to overflow
   // (particularly if our concrete object size is exactly the same size as the
   // replaced element's content-box).
-  if (aCoord.LengthInCSSPixels() != 0.) {
-    return true;
+  if (!aCoord.ConvertsToPercentage()) {
+    return !aCoord.ConvertsToLength() || aCoord.ToLengthInCSSPixels() != 0.0f;
   }
 
   // Percentages are interpreted as a fraction of the extra space. So,
   // percentages in the 0-100% range are safe, but values outside of that
   // range could cause overflow.
-  if (aCoord.HasPercent() &&
-      (aCoord.Percentage() < 0.0f || aCoord.Percentage() > 1.0f)) {
-    return true;
-  }
-  return false;
+  float percentage = aCoord.ToPercentage();
+  return percentage < 0.0f || percentage > 1.0f;
 }
 
 /* static */

servo/components/style/values/animated/transform.rs

@@ -1168,10 +1168,11 @@ impl ComputeSquaredDistance for ComputedTransformOperation {
                 // However, dropping percentage makes us impossible to compute
                 // the distance for the percentage-percentage case, but Gecko
                 // uses the same formula, so it's fine for now.
-                let fx = fx.length_component().px();
-                let fy = fy.length_component().px();
-                let tx = tx.length_component().px();
-                let ty = ty.length_component().px();
+                let basis = Length::new(0.);
+                let fx = fx.resolve(basis).px();
+                let fy = fy.resolve(basis).px();
+                let tx = tx.resolve(basis).px();
+                let ty = ty.resolve(basis).px();
 
                 Ok(fx.compute_squared_distance(&tx)? +
                     fy.compute_squared_distance(&ty)? +

servo/ports/geckolib/cbindgen.toml

@@ -248,7 +248,7 @@ renaming_overrides_prefixing = true
 "CSSPixelLength" = """
   inline nscoord ToAppUnits() const;
   inline bool IsZero() const;
-  float ToCSSPixels() const { return _0; }
+  CSSCoord ToCSSPixels() const { return _0; }
   inline void ScaleBy(float);
 """
 
@@ -296,11 +296,10 @@ renaming_overrides_prefixing = true
   static inline Self FromPercentage(float);
 
   inline void ScaleLengthsBy(float);
-  inline CSSCoord LengthInCSSPixels() const;
-  inline float Percentage() const;
   inline bool HasPercent() const;
   inline bool ConvertsToLength() const;
   inline nscoord ToLength() const;
+  inline CSSCoord ToLengthInCSSPixels() const;
   inline bool ConvertsToPercentage() const;
   inline bool HasLengthAndPercentage() const;
   inline float ToPercentage() const;

servo/ports/geckolib/glue.rs

@@ -6851,3 +6851,11 @@ pub unsafe extern "C" fn Servo_LoadData_GetLazy(
 ) -> *const url::LoadData {
     source.get()
 }
+
+#[no_mangle]
+pub extern "C" fn Servo_LengthPercentage_ToCss(
+    lp: &computed::LengthPercentage,
+    result: &mut nsAString
+) {
+    lp.to_css(&mut CssWriter::new(result)).unwrap();
+}

testing/web-platform/meta/css/css-logical/parsing/max-block-size-computed.html.ini

@@ -1,4 +0,0 @@
-[max-block-size-computed.html]
-  [Property max-block-size value 'calc(20% + 10px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-logical/parsing/max-inline-size-computed.html.ini

@@ -1,4 +0,0 @@
-[max-inline-size-computed.html]
-  [Property max-inline-size value 'calc(20% + 10px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-logical/parsing/min-block-size-computed.html.ini

@@ -1,4 +0,0 @@
-[min-block-size-computed.html]
-  [Property min-block-size value 'calc(20% + 10px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-logical/parsing/min-inline-size-computed.html.ini

@@ -1,4 +0,0 @@
-[min-inline-size-computed.html]
-  [Property min-inline-size value 'calc(20% + 10px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-position/parsing/bottom-computed.html.ini

@@ -1,4 +0,0 @@
-[bottom-computed.html]
-  [Property bottom value 'calc(50% + 60px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-position/parsing/left-computed.html.ini

@@ -1,4 +0,0 @@
-[left-computed.html]
-  [Property left value 'calc(50% + 60px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-position/parsing/right-computed.html.ini

@@ -1,4 +0,0 @@
-[right-computed.html]
-  [Property right value 'calc(50% + 60px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-position/parsing/top-computed.html.ini

@@ -1,4 +0,0 @@
-[top-computed.html]
-  [Property top value 'calc(50% + 60px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-sizing/parsing/max-height-computed.html.ini

@@ -1,4 +0,0 @@
-[max-height-computed.html]
-  [Property max-height value 'calc(10% + 40px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-sizing/parsing/max-width-computed.html.ini

@@ -1,4 +0,0 @@
-[max-width-computed.html]
-  [Property max-width value 'calc(10% + 40px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-sizing/parsing/min-height-computed.html.ini

@@ -1,4 +0,0 @@
-[min-height-computed.html]
-  [Property min-height value 'calc(10% + 40px)']
-    expected: FAIL
-

testing/web-platform/meta/css/css-sizing/parsing/min-width-computed.html.ini

@@ -1,4 +0,0 @@
-[min-width-computed.html]
-  [Property min-width value 'calc(10% + 40px)']
-    expected: FAIL
-

testing/web-platform/meta/svg/geometry/parsing/sizing-properties-computed.svg.ini

@@ -1,10 +1,4 @@
 [sizing-properties-computed.svg]
-  [resolved value is computed value when display is none]
-    expected: FAIL
-
-  [resolved value is computed value when display is contents]
-    expected: FAIL
-
   [Property width value 'calc(50% + 1.5em)']
     expected: FAIL