Bug 1518954 - Three-value position syntax uses calc() as its computed value representation. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 11 Jan 2019 01:07:22 +0100
changeset 510483 472bcde2c2d1142a518cbe0ba6f06ba28896b7a9
parent 510481 36be88b03e11661c75387b773e8df82af9ef68cd
child 510484 fbe6548db11ded24b5221180719ce66e785dc3c6
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1518954
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518954 - Three-value position syntax uses calc() as its computed value representation. r=heycam This restores the previous behavior of using calc(). Note that background-position / object-position, which test this, weren't hitting the assertion because they use another codepath. I didn't add more extensive tests for this because it's well tested for those two properties, and because this is legacy anyway, see the comment in the test. I did add the assertion to the codepath those two properties hit. Differential Revision: https://phabricator.services.mozilla.com/D16176
layout/style/test/property_database.js
servo/components/style/gecko/conversions.rs
servo/components/style/values/specified/position.rs
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -222,16 +222,23 @@ var validGradientAndElementValues = [
   "repeating-radial-gradient(farthest-side circle at 45px, red, blue)",
   "repeating-radial-gradient(ellipse closest-side at 50%, red, blue)",
   "repeating-radial-gradient(circle farthest-corner at 4em, red, blue)",
 
   "repeating-radial-gradient(30% 40% at top left, red, blue)",
   "repeating-radial-gradient(50px 60px at 15% 20%, red, blue)",
   "repeating-radial-gradient(7em 8em at 45px, red, blue)",
 
+  // FIXME(emilio): We should not be allowing 3-value positions anywhere else
+  // than on `background-position`, see
+  // https://github.com/w3c/csswg-drafts/issues/2140.
+  //
+  // When that happens this should be moved to the `invalid` list.
+  "repeating-radial-gradient(circle closest-side at left bottom 7in, hsl(2,2%,5%), rgb(1,6,0))",
+
   "-moz-image-rect(url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), 2, 10, 10, 2)",
   "-moz-image-rect(url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), 10%, 50%, 30%, 0%)",
   "-moz-image-rect(url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), 10, 50%, 30%, 0)",
 
   "radial-gradient(at calc(25%) top, red, blue)",
   "radial-gradient(at left calc(25%), red, blue)",
   "radial-gradient(at calc(25px) top, red, blue)",
   "radial-gradient(at left calc(25px), red, blue)",
--- a/servo/components/style/gecko/conversions.rs
+++ b/servo/components/style/gecko/conversions.rs
@@ -17,29 +17,34 @@ use crate::gecko_bindings::structs::{sel
 use crate::gecko_bindings::structs::{nsStyleImage, nsresult, SheetType};
 use crate::gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
 use crate::stylesheets::{Origin, RulesMutateError};
 use crate::values::computed::image::LineDirection;
 use crate::values::computed::transform::Matrix3D;
 use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::{Angle, Gradient, Image};
 use crate::values::computed::{Integer, LengthPercentage};
+use crate::values::computed::{Length, Percentage, TextAlign};
 use crate::values::computed::{LengthPercentageOrAuto, NonNegativeLengthPercentageOrAuto};
-use crate::values::computed::{Percentage, TextAlign};
 use crate::values::generics::box_::VerticalAlign;
 use crate::values::generics::grid::{TrackListValue, TrackSize};
 use crate::values::generics::image::{CompatMode, GradientItem, Image as GenericImage};
 use crate::values::generics::rect::Rect;
 use crate::values::generics::NonNegative;
 use app_units::Au;
 use std::f32::consts::PI;
 use style_traits::values::specified::AllowedNumericType;
 
 impl From<LengthPercentage> for nsStyleCoord_CalcValue {
     fn from(other: LengthPercentage) -> nsStyleCoord_CalcValue {
+        debug_assert!(
+            other.was_calc ||
+                other.percentage.is_none() ||
+                other.unclamped_length() == Length::zero()
+        );
         let has_percentage = other.percentage.is_some();
         nsStyleCoord_CalcValue {
             mLength: other.unclamped_length().to_i32_au(),
             mPercent: other.percentage.map_or(0., |p| p.0),
             mHasPercent: has_percentage,
         }
     }
 }
--- a/servo/components/style/values/specified/position.rs
+++ b/servo/components/style/values/specified/position.rs
@@ -258,21 +258,22 @@ impl<S: Side> ToComputedValue for Positi
             PositionComponent::Side(ref keyword, None) => {
                 let p = Percentage(if keyword.is_start() { 0. } else { 1. });
                 ComputedLengthPercentage::new_percent(p)
             },
             PositionComponent::Side(ref keyword, Some(ref length)) if !keyword.is_start() => {
                 let length = length.to_computed_value(context);
                 let p = Percentage(1. - length.percentage());
                 let l = -length.unclamped_length();
+                // We represent `<end-side> <length>` as `calc(100% - <length>)`.
                 ComputedLengthPercentage::with_clamping_mode(
                     l,
                     Some(p),
                     length.clamping_mode,
-                    length.was_calc,
+                    /* was_calc = */ true,
                 )
             },
             PositionComponent::Side(_, Some(ref length)) |
             PositionComponent::Length(ref length) => length.to_computed_value(context),
         }
     }
 
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {