Auto merge of #19462 - emilio:angle-parsing, r=nox

style: Cleanup Angle parsing.

This PR also contains a functional change, allowing to parse unitless zero angles in hue-rotate().

See the links and the comments for why.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19462)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-12-03 09:14:59 -06:00 committed by GitHub
commit e4dba3abef
5 changed files with 78 additions and 56 deletions

View file

@ -102,23 +102,25 @@ impl AsRef<ComputedAngle> for Angle {
}
}
/// Whether to allow parsing an unitless zero as a valid angle.
///
/// This should always be `No`, except for exceptions like:
///
/// https://github.com/w3c/fxtf-drafts/issues/228
///
/// See also: https://github.com/w3c/csswg-drafts/issues/1162.
enum AllowUnitlessZeroAngle {
Yes,
No,
}
impl Parse for Angle {
/// Parses an angle according to CSS-VALUES § 6.1.
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
// FIXME: remove clone() when lifetimes are non-lexical
let token = input.next()?.clone();
match token {
Token::Dimension { value, ref unit, .. } => {
Angle::parse_dimension(value, unit, /* from_calc = */ false)
}
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
return input.parse_nested_block(|i| CalcNode::parse_angle(context, i))
}
_ => Err(())
}.map_err(|()| input.new_unexpected_token_error(token.clone()))
Self::parse_internal(context, input, AllowUnitlessZeroAngle::No)
}
}
@ -127,9 +129,8 @@ impl Angle {
pub fn parse_dimension(
value: CSSFloat,
unit: &str,
from_calc: bool)
-> Result<Angle, ()>
{
from_calc: bool,
) -> Result<Angle, ()> {
let angle = match_ignore_ascii_case! { unit,
"deg" => Angle::from_degrees(value, from_calc),
"grad" => Angle::from_gradians(value, from_calc),
@ -140,23 +141,33 @@ impl Angle {
Ok(angle)
}
/// Parse an angle, including unitless 0 degree.
/// Parse an `<angle>` allowing unitless zero to represent a zero angle.
///
/// Note that numbers without any AngleUnit, including unitless 0 angle,
/// should be invalid. However, some properties still accept unitless 0
/// angle and stores it as '0deg'.
///
/// We can remove this and get back to the unified version Angle::parse once
/// https://github.com/w3c/csswg-drafts/issues/1162 is resolved.
pub fn parse_with_unitless<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
-> Result<Self, ParseError<'i>> {
/// See the comment in `AllowUnitlessZeroAngle` for why.
pub fn parse_with_unitless<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
Self::parse_internal(context, input, AllowUnitlessZeroAngle::Yes)
}
fn parse_internal<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
allow_unitless_zero: AllowUnitlessZeroAngle,
) -> Result<Self, ParseError<'i>> {
// FIXME: remove clone() when lifetimes are non-lexical
let token = input.next()?.clone();
match token {
Token::Dimension { value, ref unit, .. } => {
Angle::parse_dimension(value, unit, /* from_calc = */ false)
}
Token::Number { value, .. } if value == 0. => Ok(Angle::zero()),
Token::Number { value, .. } if value == 0. => {
match allow_unitless_zero {
AllowUnitlessZeroAngle::Yes => Ok(Angle::zero()),
AllowUnitlessZeroAngle::No => Err(()),
}
},
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
return input.parse_nested_block(|i| CalcNode::parse_angle(context, i))
}

View file

@ -205,7 +205,11 @@ impl Parse for Filter {
// https://drafts.fxtf.org/filter-effects/#funcdef-filter-grayscale
Ok(GenericFilter::Grayscale(Factor::parse_with_clamping_to_one(context, i)?))
},
"hue-rotate" => Ok(GenericFilter::HueRotate(Angle::parse(context, i)?)),
"hue-rotate" => {
// We allow unitless zero here, see:
// https://github.com/w3c/fxtf-drafts/issues/228
Ok(GenericFilter::HueRotate(Angle::parse_with_unitless(context, i)?))
},
"invert" => {
// Values of amount over 100% are allowed but UAs must clamp the values to 1.
// https://drafts.fxtf.org/filter-effects/#funcdef-filter-invert

View file

@ -261,8 +261,10 @@ impl Parse for Gradient {
}
impl Gradient {
fn parse_webkit_gradient_argument<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
-> Result<Self, ParseError<'i>> {
fn parse_webkit_gradient_argument<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
type Point = GenericPosition<Component<X>, Component<Y>>;
#[derive(Clone, Copy)]
@ -518,10 +520,11 @@ impl Gradient {
impl GradientKind {
/// Parses a linear gradient.
/// CompatMode can change during `-moz-` prefixed gradient parsing if it come across a `to` keyword.
fn parse_linear<'i, 't>(context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: &mut CompatMode)
-> Result<Self, ParseError<'i>> {
fn parse_linear<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: &mut CompatMode,
) -> Result<Self, ParseError<'i>> {
let direction = if let Ok(d) = input.try(|i| LineDirection::parse(context, i, compat_mode)) {
input.expect_comma()?;
d
@ -534,10 +537,11 @@ impl GradientKind {
Ok(GenericGradientKind::Linear(direction))
}
fn parse_radial<'i, 't>(context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: &mut CompatMode)
-> Result<Self, ParseError<'i>> {
fn parse_radial<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: &mut CompatMode,
) -> Result<Self, ParseError<'i>> {
let (shape, position, angle, moz_position) = match *compat_mode {
CompatMode::Modern => {
let shape = input.try(|i| EndingShape::parse(context, i, *compat_mode));
@ -702,13 +706,16 @@ impl GenericsLineDirection for LineDirection {
}
impl LineDirection {
fn parse<'i, 't>(context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: &mut CompatMode)
-> Result<Self, ParseError<'i>> {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: &mut CompatMode,
) -> Result<Self, ParseError<'i>> {
let mut _angle = if *compat_mode == CompatMode::Moz {
input.try(|i| Angle::parse(context, i)).ok()
} else {
// Gradients allow unitless zero angles as an exception, see:
// https://github.com/w3c/csswg-drafts/issues/1162
if let Ok(angle) = input.try(|i| Angle::parse_with_unitless(context, i)) {
return Ok(LineDirection::Angle(angle));
}
@ -780,10 +787,11 @@ impl ToComputedValue for GradientPosition {
}
impl EndingShape {
fn parse<'i, 't>(context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: CompatMode)
-> Result<Self, ParseError<'i>> {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
compat_mode: CompatMode,
) -> Result<Self, ParseError<'i>> {
if let Ok(extent) = input.try(|i| ShapeExtent::parse_with_compat_mode(i, compat_mode)) {
if input.try(|i| i.expect_ident_matching("circle")).is_ok() {
return Ok(GenericEndingShape::Circle(Circle::Extent(extent)));
@ -861,9 +869,10 @@ impl EndingShape {
}
impl ShapeExtent {
fn parse_with_compat_mode<'i, 't>(input: &mut Parser<'i, 't>,
compat_mode: CompatMode)
-> Result<Self, ParseError<'i>> {
fn parse_with_compat_mode<'i, 't>(
input: &mut Parser<'i, 't>,
compat_mode: CompatMode,
) -> Result<Self, ParseError<'i>> {
match Self::parse(input)? {
ShapeExtent::Contain | ShapeExtent::Cover if compat_mode == CompatMode::Modern => {
Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
@ -876,8 +885,10 @@ impl ShapeExtent {
}
impl GradientItem {
fn parse_comma_separated<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
-> Result<Vec<Self>, ParseError<'i>> {
fn parse_comma_separated<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Vec<Self>, ParseError<'i>> {
let mut seen_stop = false;
let items = input.parse_comma_separated(|input| {
if seen_stop {

View file

@ -29,16 +29,18 @@ pub type TransformOperation = GenericTransformOperation<
LengthOrPercentage,
LengthOrPercentageOrNumber,
>;
/// A specified CSS `transform`
pub type Transform = GenericTransform<TransformOperation>;
/// The specified value of a CSS `<transform-origin>`
pub type TransformOrigin = GenericTransformOrigin<OriginComponent<X>, OriginComponent<Y>, Length>;
impl Transform {
/// Internal parse function for deciding if we wish to accept prefixed values or not
// Allow unitless zero angle for rotate() and skew() to align with gecko
///
/// `transform` allows unitless zero angles as an exception, see:
/// https://github.com/w3c/csswg-drafts/issues/1162
fn parse_internal<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,

View file

@ -29,12 +29,6 @@
[Serialization should round-trip after setting e.style['filter'\] = "drop-shadow(1px 2px 3px rgba(4, 5, 6, 0.75))"]
expected: FAIL
[e.style['filter'\] = "hue-rotate(0)" should set the property value]
expected: FAIL
[Serialization should round-trip after setting e.style['filter'\] = "hue-rotate(0)"]
expected: FAIL
[e.style['filter'\] = "url(picture.svg#f)" should set the property value]
expected: FAIL