Bug 1505156 - Percentage values of translate are serialized as percent for computed values. r=emilio

Basically, we rewrite the type of generics::transform::Translate and its
ToCss to match the spec. Besides, we always serialize Translate by servo,
so we could drop a lot of duplicated code.

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Boris Chiou 2018-11-08 22:41:00 +00:00
parent 0a41baa8d0
commit a2c42ba3de
15 changed files with 143 additions and 140 deletions

View File

@ -107,6 +107,7 @@ SERIALIZED_PREDEFINED_TYPES = [
"Resize",
"Scale",
"TextAlign",
"Translate",
"TimingFunction",
"TransformStyle",
"background::BackgroundSize",

View File

@ -1294,85 +1294,6 @@ ReadIndividualTransformValue(nsCSSValueSharedList* aList,
return val.forget();
}
already_AddRefed<CSSValue>
nsComputedDOMStyle::DoGetTranslate()
{
typedef nsStyleTransformMatrix::TransformReferenceBox TransformReferenceBox;
RefPtr<nsComputedDOMStyle> self(this);
return ReadIndividualTransformValue(StyleDisplay()->mSpecifiedTranslate,
[self](const nsCSSValue::Array* aData, nsString& aResult) {
TransformReferenceBox refBox(self->mInnerFrame, nsSize(0, 0));
// Even though the spec doesn't say to resolve percentage values, Blink
// and Edge do and so until that is clarified we do as well:
//
// https://github.com/w3c/csswg-drafts/issues/2124
switch (nsStyleTransformMatrix::TransformFunctionOf(aData)) {
/* translate : <length-percentage> */
case eCSSKeyword_translatex: {
MOZ_ASSERT(aData->Count() == 2, "Invalid array!");
float tx = ProcessTranslatePart(aData->Item(1),
&refBox,
&TransformReferenceBox::Width);
aResult.AppendFloat(tx);
aResult.AppendLiteral("px");
break;
}
/* translate : <length-percentage> <length-percentage> */
case eCSSKeyword_translate: {
MOZ_ASSERT(aData->Count() == 3, "Invalid array!");
float tx = ProcessTranslatePart(aData->Item(1),
&refBox,
&TransformReferenceBox::Width);
aResult.AppendFloat(tx);
aResult.AppendLiteral("px");
float ty = ProcessTranslatePart(aData->Item(2),
&refBox,
&TransformReferenceBox::Height);
if (ty != 0) {
aResult.AppendLiteral(" ");
aResult.AppendFloat(ty);
aResult.AppendLiteral("px");
}
break;
}
/* translate : <length-percentage> <length-percentage> <length>*/
case eCSSKeyword_translate3d: {
MOZ_ASSERT(aData->Count() == 4, "Invalid array!");
float tx = ProcessTranslatePart(aData->Item(1),
&refBox,
&TransformReferenceBox::Width);
aResult.AppendFloat(tx);
aResult.AppendLiteral("px");
float ty = ProcessTranslatePart(aData->Item(2),
&refBox,
&TransformReferenceBox::Height);
float tz = ProcessTranslatePart(aData->Item(3),
&refBox,
nullptr);
if (ty != 0. || tz != 0.) {
aResult.AppendLiteral(" ");
aResult.AppendFloat(ty);
aResult.AppendLiteral("px");
}
if (tz != 0.) {
aResult.AppendLiteral(" ");
aResult.AppendFloat(tz);
aResult.AppendLiteral("px");
}
break;
}
default:
MOZ_ASSERT_UNREACHABLE("Unexpected CSS keyword.");
}
});
}
already_AddRefed<CSSValue>
nsComputedDOMStyle::DoGetRotate()
{

View File

@ -365,7 +365,6 @@ private:
already_AddRefed<CSSValue> DoGetOverflowClipBoxInline();
already_AddRefed<CSSValue> DoGetTouchAction();
already_AddRefed<CSSValue> DoGetTransform();
already_AddRefed<CSSValue> DoGetTranslate();
already_AddRefed<CSSValue> DoGetRotate();
already_AddRefed<CSSValue> DoGetTransformOrigin();
already_AddRefed<CSSValue> DoGetPerspective();

View File

@ -2102,12 +2102,12 @@ impl Matrix3D {
/// <https://drafts.csswg.org/css-transforms-2/#propdef-rotate>
impl ComputedRotate {
fn resolve(rotate: &ComputedRotate) -> (Number, Number, Number, Angle) {
fn resolve(&self) -> (Number, Number, Number, Angle) {
// According to the spec:
// https://drafts.csswg.org/css-transforms-2/#individual-transforms
//
// If the axis is unspecified, it defaults to "0 0 1"
match *rotate {
match *self {
Rotate::None => (0., 0., 1., Angle::zero()),
Rotate::Rotate3D(rx, ry, rz, angle) => (rx, ry, rz, angle),
Rotate::Rotate(angle) => (0., 0., 1., angle),
@ -2122,8 +2122,7 @@ impl Animate for ComputedRotate {
other: &Self,
procedure: Procedure,
) -> Result<Self, ()> {
let from = ComputedRotate::resolve(self);
let to = ComputedRotate::resolve(other);
let (from, to) = (self.resolve(), other.resolve());
let (mut fx, mut fy, mut fz, fa) =
transform::get_normalized_vector_and_angle(from.0, from.1, from.2, from.3);
@ -2163,24 +2162,17 @@ impl Animate for ComputedRotate {
/// <https://drafts.csswg.org/css-transforms-2/#propdef-translate>
impl ComputedTranslate {
fn resolve(
translate: &ComputedTranslate,
) -> (LengthOrPercentage, LengthOrPercentage, Length) {
fn resolve(&self) -> (LengthOrPercentage, LengthOrPercentage, Length) {
// According to the spec:
// https://drafts.csswg.org/css-transforms-2/#individual-transforms
//
// Unspecified translations default to 0px
match *translate {
match *self {
Translate::None => {
(
LengthOrPercentage::Length(Length::zero()),
LengthOrPercentage::Length(Length::zero()),
Length::zero(),
)
(LengthOrPercentage::zero(), LengthOrPercentage::zero(), Length::zero())
},
Translate::Translate3D(tx, ty, tz) => (tx, ty, tz),
Translate::Translate(tx, ty) => (tx, ty, Length::zero()),
Translate::TranslateX(tx) => (tx, LengthOrPercentage::Length(Length::zero()), Length::zero()),
}
}
}
@ -2192,23 +2184,31 @@ impl Animate for ComputedTranslate {
other: &Self,
procedure: Procedure,
) -> Result<Self, ()> {
let from = ComputedTranslate::resolve(self);
let to = ComputedTranslate::resolve(other);
Ok(Translate::Translate3D(from.0.animate(&to.0, procedure)?,
from.1.animate(&to.1, procedure)?,
from.2.animate(&to.2, procedure)?))
match (self, other) {
(&Translate::None, &Translate::None) => Ok(Translate::None),
(&Translate::Translate3D(_, ..), _) | (_, &Translate::Translate3D(_, ..)) => {
let (from, to) = (self.resolve(), other.resolve());
Ok(Translate::Translate3D(from.0.animate(&to.0, procedure)?,
from.1.animate(&to.1, procedure)?,
from.2.animate(&to.2, procedure)?))
},
(&Translate::Translate(_, ..), _) | (_, &Translate::Translate(_, ..)) => {
let (from, to) = (self.resolve(), other.resolve());
Ok(Translate::Translate(from.0.animate(&to.0, procedure)?,
from.1.animate(&to.1, procedure)?))
},
}
}
}
/// <https://drafts.csswg.org/css-transforms-2/#propdef-scale>
impl ComputedScale {
fn resolve(scale: &ComputedScale) -> (Number, Number, Number) {
fn resolve(&self) -> (Number, Number, Number) {
// According to the spec:
// https://drafts.csswg.org/css-transforms-2/#individual-transforms
//
// Unspecified scales default to 1
match *scale {
match *self {
Scale::None => (1.0, 1.0, 1.0),
Scale::Scale3D(sx, sy, sz) => (sx, sy, sz),
Scale::Scale(sx, sy) => (sx, sy, 1.),
@ -2226,8 +2226,7 @@ impl Animate for ComputedScale {
match (self, other) {
(&Scale::None, &Scale::None) => Ok(Scale::None),
(&Scale::Scale3D(_, ..), _) | (_, &Scale::Scale3D(_, ..)) => {
let from = ComputedScale::resolve(self);
let to = ComputedScale::resolve(other);
let (from, to) = (self.resolve(), other.resolve());
// FIXME(emilio, bug 1464791): why does this do something different than
// Scale3D / TransformOperation::Scale3D?
if procedure == Procedure::Add {
@ -2241,8 +2240,7 @@ impl Animate for ComputedScale {
))
},
(&Scale::Scale(_, ..), _) | (_, &Scale::Scale(_, ..)) => {
let from = ComputedScale::resolve(self);
let to = ComputedScale::resolve(other);
let (from, to) = (self.resolve(), other.resolve());
// FIXME(emilio, bug 1464791): why does this do something different than
// Scale / TransformOperation::Scale?
if procedure == Procedure::Add {

View File

@ -370,7 +370,7 @@ ${helpers.predefined_type(
"generics::transform::Translate::None",
animation_value_type="ComputedValue",
boxed=True,
flags="CREATES_STACKING_CONTEXT FIXPOS_CB GETCS_NEEDS_LAYOUT_FLUSH",
flags="CREATES_STACKING_CONTEXT FIXPOS_CB",
gecko_pref="layout.css.individual-transform.enabled",
spec="https://drafts.csswg.org/css-transforms-2/#individual-transforms",
servo_restyle_damage="reflow_out_of_flow",

View File

@ -14,6 +14,7 @@ use style_traits::{CssWriter, ToCss};
use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero};
use values::distance::{ComputeSquaredDistance, SquaredDistance};
use values::generics::length::{MaxLength as GenericMaxLength, MozLength as GenericMozLength};
use values::generics::transform::IsZeroLength;
use values::generics::NonNegative;
use values::specified::length::ViewportPercentageLength;
use values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativeLength};
@ -494,6 +495,17 @@ impl ToComputedValue for specified::LengthOrPercentage {
}
}
impl IsZeroLength for LengthOrPercentage {
#[inline]
fn is_zero_length(&self) -> bool {
match *self {
LengthOrPercentage::Length(l) => l.0 == 0.0,
LengthOrPercentage::Percentage(p) => p.0 == 0.0,
LengthOrPercentage::Calc(c) => c.unclamped_length().0 == 0.0 && c.percentage() == 0.0,
}
}
}
#[allow(missing_docs)]
#[animate(fallback = "Self::animate_fallback")]
#[css(derive_debug)]

View File

@ -334,7 +334,6 @@ impl Translate {
pub fn to_transform_operation(&self) -> Option<TransformOperation> {
match *self {
generic::Translate::None => None,
generic::Translate::TranslateX(tx) => Some(generic::TransformOperation::TranslateX(tx)),
generic::Translate::Translate(tx, ty) => {
Some(generic::TransformOperation::Translate(tx, Some(ty)))
},
@ -347,7 +346,6 @@ impl Translate {
/// Convert Translate to TransformOperation.
pub fn from_transform_operation(operation: &TransformOperation) -> Translate {
match *operation {
generic::TransformOperation::TranslateX(tx) => generic::Translate::TranslateX(tx),
generic::TransformOperation::Translate(tx, Some(ty)) => {
generic::Translate::Translate(tx, ty)
},

View File

@ -610,7 +610,6 @@ impl<Number: ToCss + PartialEq> ToCss for Scale<Number> {
SpecifiedValueInfo,
ToAnimatedZero,
ToComputedValue,
ToCss,
)]
/// A value of the `Translate` property
///
@ -618,14 +617,56 @@ impl<Number: ToCss + PartialEq> ToCss for Scale<Number> {
pub enum Translate<LengthOrPercentage, Length> {
/// 'none'
None,
/// '<length-percentage>'
TranslateX(LengthOrPercentage),
/// '<length-percentage> <length-percentage>'
/// '<length-percentage>' or '<length-percentage> <length-percentage>'
Translate(LengthOrPercentage, LengthOrPercentage),
/// '<length-percentage> <length-percentage> <length>'
Translate3D(LengthOrPercentage, LengthOrPercentage, Length),
}
/// A trait to check if this is a zero length.
/// An alternative way is use num_traits::Zero. However, in order to implement num_traits::Zero,
/// we also have to implement Add, which may be complicated for LengthOrPercentage::Calc.
/// We could do this if other types also need it. If so, we could drop this trait.
pub trait IsZeroLength {
/// Returns true if this is a zero length.
fn is_zero_length(&self) -> bool;
}
impl<LoP: ToCss + IsZeroLength, L: ToCss> ToCss for Translate<LoP, L> {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
where
W: fmt::Write,
{
// The spec says:
// 1. If a 2d translation is specified, the property must serialize with only one or two
// values (per usual, if the second value is 0px, the default, it must be omitted when
// serializing).
// 2. If a 3d translation is specified, all three values must be serialized.
// https://drafts.csswg.org/css-transforms-2/#individual-transform-serialization
//
// We don't omit the 3rd component even if it is 0px for now, and the related
// spec issue is https://github.com/w3c/csswg-drafts/issues/3305
match *self {
Translate::None => dest.write_str("none"),
Translate::Translate(ref x, ref y) => {
x.to_css(dest)?;
if !y.is_zero_length() {
dest.write_char(' ')?;
y.to_css(dest)?;
}
Ok(())
},
Translate::Translate3D(ref x, ref y, ref z) => {
x.to_css(dest)?;
dest.write_char(' ')?;
y.to_css(dest)?;
dest.write_char(' ')?;
z.to_css(dest)
},
}
}
}
#[allow(missing_docs)]
#[derive(
Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss,

View File

@ -19,6 +19,7 @@ use style_traits::{ParseError, SpecifiedValueInfo, StyleParseErrorKind};
use values::computed::{self, CSSPixelLength, Context, ExtremumLength};
use values::generics::length::{MaxLength as GenericMaxLength, MozLength as GenericMozLength};
use values::generics::NonNegative;
use values::generics::transform::IsZeroLength;
use values::specified::calc::CalcNode;
use values::{Auto, CSSFloat, Either, IsAuto, Normal};
@ -97,6 +98,16 @@ impl FontBaseSize {
}
impl FontRelativeLength {
/// Return true if this is a zero value.
fn is_zero(&self) -> bool {
match *self {
FontRelativeLength::Em(v) |
FontRelativeLength::Ex(v) |
FontRelativeLength::Ch(v) |
FontRelativeLength::Rem(v) => v == 0.0,
}
}
/// Computes the font-relative length.
pub fn to_computed_value(&self, context: &Context, base_size: FontBaseSize) -> CSSPixelLength {
use std::f32;
@ -230,6 +241,16 @@ pub enum ViewportPercentageLength {
}
impl ViewportPercentageLength {
/// Return true if this is a zero value.
fn is_zero(&self) -> bool {
match *self {
ViewportPercentageLength::Vw(v) |
ViewportPercentageLength::Vh(v) |
ViewportPercentageLength::Vmin(v) |
ViewportPercentageLength::Vmax(v) => v == 0.0,
}
}
/// Computes the given viewport-relative length for the given viewport size.
pub fn to_computed_value(&self, viewport_size: Size2D<Au>) -> CSSPixelLength {
let (factor, length) = match *self {
@ -485,6 +506,18 @@ impl NoCalcLength {
impl SpecifiedValueInfo for NoCalcLength {}
impl IsZeroLength for NoCalcLength {
#[inline]
fn is_zero_length(&self) -> bool {
match *self {
NoCalcLength::Absolute(v) => v.is_zero(),
NoCalcLength::FontRelative(v) => v.is_zero(),
NoCalcLength::ViewportPercentage(v) => v.is_zero(),
NoCalcLength::ServoCharacterWidth(v) => v.0 == 0,
}
}
}
/// An extension to `NoCalcLength` to parse `calc` expressions.
/// This is commonly used for the `<length>` values.
///
@ -839,6 +872,17 @@ impl LengthOrPercentage {
}
}
impl IsZeroLength for LengthOrPercentage {
#[inline]
fn is_zero_length(&self) -> bool {
match *self {
LengthOrPercentage::Length(l) => l.is_zero_length(),
LengthOrPercentage::Percentage(p) => p.0 == 0.0,
LengthOrPercentage::Calc(_) => false,
}
}
}
/// Either a `<length>`, a `<percentage>`, or the `auto` keyword.
#[allow(missing_docs)]
#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]

View File

@ -395,7 +395,7 @@ impl Parse for Translate {
}
// 'translate: <length-percentage> '
Ok(generic::Translate::TranslateX(tx))
Ok(generic::Translate::Translate(tx, specified::LengthOrPercentage::zero()))
}
}

View File

@ -1,22 +1,11 @@
[translate-interpolation.html]
[Animation between "480px 400px 320px" and "240% 160%" at progress 0]
expected: FAIL
bug: https://github.com/w3c/csswg-drafts/issues/3290
[Animation between "480px 400px 320px" and "240% 160%" at progress 1]
expected: FAIL
bug: https://github.com/w3c/csswg-drafts/issues/3290
[Animation between "none" and "8px 80% 800px" at progress 0]
expected: FAIL
[Animation between "none" and "none" at progress -1]
expected: FAIL
[Animation between "none" and "none" at progress 0]
expected: FAIL
[Animation between "none" and "none" at progress 0.125]
expected: FAIL
[Animation between "none" and "none" at progress 0.875]
expected: FAIL
[Animation between "none" and "none" at progress 1]
expected: FAIL
[Animation between "none" and "none" at progress 2]
expected: FAIL
bug: https://github.com/w3c/csswg-drafts/issues/3290

View File

@ -1,3 +0,0 @@
[transforms-support-calc.html]
[translate supports calc]
expected: FAIL

View File

@ -17,9 +17,12 @@ test_valid_value("translate", "none");
test_valid_value("translate", "0px");
test_valid_value("translate", "100%");
test_valid_value("translate", "100px 0px", "100px");
test_valid_value("translate", "100px 0%", "100px");
test_valid_value("translate", "100px 200%");
test_valid_value("translate", "100% 200px");
test_valid_value("translate", "100px 200px 0px");
test_valid_value("translate", "100px 200px 300px");
test_valid_value("translate", "100% 200% 300px");

View File

@ -31,7 +31,7 @@
test(function(){
target.style = 'translate: calc(30px + 20%) calc(-200px + 100%);';
assert_equals(getComputedStyle(target).translate, 'calc(30px + 20%) calc(-200px + 100%)');
assert_equals(getComputedStyle(target).translate, 'calc(20% + 30px) calc(100% - 200px)');
}, 'translate supports calc');
test(function(){

View File

@ -1885,7 +1885,7 @@ const translateListType = {
1000
);
testAnimationSamples(animation, idlName,
[{ time: 500, expected: '200px -25.5px 200px' }]);
[{ time: 500, expected: '200px calc(25% - 50.5px) 200px' }]);
}, `${property} with combination of percentages and lengths`);
},
testAddition: function(property, setup) {
@ -1924,8 +1924,8 @@ const translateListType = {
{ duration: 1000, fill: 'both',
composite: 'add' });
testAnimationSamples(animation, idlName,
[ { time: 0, expected: '-150px' },
{ time: 1000, expected: '550px' }]);
[ { time: 0, expected: 'calc(50% - 200px)' },
{ time: 1000, expected: 'calc(50% + 500px)' }]);
}, `${property} with underlying percentage value`);
},