style: Don't allow negative radii in radial gradients.

Mostly renaming for clarity, as the gradient parsing code is a bit hairy.

This also changes -webkit- gradients, which is, I think, the right thing to do
(otherwise I need to give up on the type system and sprinkle parse_non_negatives
around, which would be unfortunate).

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1008112 on
Chromium still accepting negative radii for those, so will wait to submit the
patch for review until they reply there with their intentions.

Differential Revision: https://phabricator.services.mozilla.com/D47141
This commit is contained in:
Emilio Cobos Álvarez 2019-10-01 21:44:41 +00:00
parent 19ddfd57d5
commit d43632c9f8
5 changed files with 61 additions and 46 deletions

View file

@ -10,7 +10,7 @@
use crate::values::computed::position::Position; use crate::values::computed::position::Position;
use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::url::ComputedImageUrl;
use crate::values::computed::{Angle, Color, Context}; use crate::values::computed::{Angle, Color, Context};
use crate::values::computed::{Length, LengthPercentage, NumberOrPercentage, ToComputedValue}; use crate::values::computed::{LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, NumberOrPercentage, ToComputedValue};
use crate::values::generics::image::{self as generic, GradientCompatMode}; use crate::values::generics::image::{self as generic, GradientCompatMode};
use crate::values::specified::image::LineDirection as SpecifiedLineDirection; use crate::values::specified::image::LineDirection as SpecifiedLineDirection;
use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword}; use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword};
@ -27,12 +27,14 @@ pub type Image = generic::GenericImage<Gradient, MozImageRect, ComputedImageUrl>
/// Computed values for a CSS gradient. /// Computed values for a CSS gradient.
/// <https://drafts.csswg.org/css-images/#gradients> /// <https://drafts.csswg.org/css-images/#gradients>
pub type Gradient = pub type Gradient = generic::GenericGradient<
generic::GenericGradient<LineDirection, Length, LengthPercentage, Position, Color>; LineDirection,
LengthPercentage,
/// A computed gradient kind. NonNegativeLength,
pub type GradientKind = NonNegativeLengthPercentage,
generic::GenericGradientKind<LineDirection, Length, LengthPercentage, Position>; Position,
Color,
>;
/// A computed gradient line direction. /// A computed gradient line direction.
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToResolvedValue)] #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToResolvedValue)]
@ -48,9 +50,6 @@ pub enum LineDirection {
Corner(HorizontalPositionKeyword, VerticalPositionKeyword), Corner(HorizontalPositionKeyword, VerticalPositionKeyword),
} }
/// A computed radial gradient ending shape.
pub type EndingShape = generic::EndingShape<Length, LengthPercentage>;
/// A computed gradient item. /// A computed gradient item.
pub type GradientItem = generic::GenericGradientItem<Color, LengthPercentage>; pub type GradientItem = generic::GenericGradientItem<Color, LengthPercentage>;

View file

@ -73,9 +73,16 @@ pub use self::GenericImage as Image;
/// <https://drafts.csswg.org/css-images/#gradients> /// <https://drafts.csswg.org/css-images/#gradients>
#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)] #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
#[repr(C)] #[repr(C)]
pub struct GenericGradient<LineDirection, Length, LengthPercentage, Position, Color> { pub struct GenericGradient<
LineDirection,
LengthPercentage,
NonNegativeLength,
NonNegativeLengthPercentage,
Position,
Color,
> {
/// Gradients can be linear or radial. /// Gradients can be linear or radial.
pub kind: GenericGradientKind<LineDirection, Length, LengthPercentage, Position>, pub kind: GenericGradientKind<LineDirection, NonNegativeLength, NonNegativeLengthPercentage, Position>,
/// The color stops and interpolation hints. /// The color stops and interpolation hints.
pub items: crate::OwnedSlice<GenericGradientItem<Color, LengthPercentage>>, pub items: crate::OwnedSlice<GenericGradientItem<Color, LengthPercentage>>,
/// True if this is a repeating gradient. /// True if this is a repeating gradient.
@ -101,11 +108,11 @@ pub enum GradientCompatMode {
/// A gradient kind. /// A gradient kind.
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)] #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
#[repr(C, u8)] #[repr(C, u8)]
pub enum GenericGradientKind<LineDirection, Length, LengthPercentage, Position> { pub enum GenericGradientKind<LineDirection, NonNegativeLength, NonNegativeLengthPercentage, Position> {
/// A linear gradient. /// A linear gradient.
Linear(LineDirection), Linear(LineDirection),
/// A radial gradient. /// A radial gradient.
Radial(GenericEndingShape<Length, LengthPercentage>, Position), Radial(GenericEndingShape<NonNegativeLength, NonNegativeLengthPercentage>, Position),
} }
pub use self::GenericGradientKind as GradientKind; pub use self::GenericGradientKind as GradientKind;
@ -115,11 +122,11 @@ pub use self::GenericGradientKind as GradientKind;
Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem, Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem,
)] )]
#[repr(C, u8)] #[repr(C, u8)]
pub enum GenericEndingShape<Length, LengthPercentage> { pub enum GenericEndingShape<NonNegativeLength, NonNegativeLengthPercentage> {
/// A circular gradient. /// A circular gradient.
Circle(GenericCircle<Length>), Circle(GenericCircle<NonNegativeLength>),
/// An elliptic gradient. /// An elliptic gradient.
Ellipse(GenericEllipse<LengthPercentage>), Ellipse(GenericEllipse<NonNegativeLengthPercentage>),
} }
pub use self::GenericEndingShape as EndingShape; pub use self::GenericEndingShape as EndingShape;
@ -127,9 +134,9 @@ pub use self::GenericEndingShape as EndingShape;
/// A circle shape. /// A circle shape.
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)] #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
#[repr(C, u8)] #[repr(C, u8)]
pub enum GenericCircle<Length> { pub enum GenericCircle<NonNegativeLength> {
/// A circle radius. /// A circle radius.
Radius(Length), Radius(NonNegativeLength),
/// A circle extent. /// A circle extent.
Extent(ShapeExtent), Extent(ShapeExtent),
} }
@ -141,9 +148,9 @@ pub use self::GenericCircle as Circle;
Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem, Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem,
)] )]
#[repr(C, u8)] #[repr(C, u8)]
pub enum GenericEllipse<LengthPercentage> { pub enum GenericEllipse<NonNegativeLengthPercentage> {
/// An ellipse pair of radii. /// An ellipse pair of radii.
Radii(LengthPercentage, LengthPercentage), Radii(NonNegativeLengthPercentage, NonNegativeLengthPercentage),
/// An ellipse extent. /// An ellipse extent.
Extent(ShapeExtent), Extent(ShapeExtent),
} }
@ -314,11 +321,12 @@ where
} }
} }
impl<D, L, LoP, P, C> ToCss for Gradient<D, L, LoP, P, C> impl<D, LP, NL, NLP, P, C> ToCss for Gradient<D, LP, NL, NLP, P, C>
where where
D: LineDirection, D: LineDirection,
L: ToCss, LP: ToCss,
LoP: ToCss, NL: ToCss,
NLP: ToCss,
P: ToCss, P: ToCss,
C: ToCss, C: ToCss,
{ {

View file

@ -10,6 +10,7 @@
use crate::custom_properties::SpecifiedValue; use crate::custom_properties::SpecifiedValue;
use crate::parser::{Parse, ParserContext}; use crate::parser::{Parse, ParserContext};
use crate::stylesheets::CorsMode; use crate::stylesheets::CorsMode;
use crate::values::generics::NonNegative;
use crate::values::generics::image::PaintWorklet; use crate::values::generics::image::PaintWorklet;
use crate::values::generics::image::{ use crate::values::generics::image::{
self as generic, Circle, Ellipse, GradientCompatMode, ShapeExtent, self as generic, Circle, Ellipse, GradientCompatMode, ShapeExtent,
@ -18,7 +19,7 @@ use crate::values::generics::position::Position as GenericPosition;
use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword}; use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword};
use crate::values::specified::position::{Position, PositionComponent, Side}; use crate::values::specified::position::{Position, PositionComponent, Side};
use crate::values::specified::url::SpecifiedImageUrl; use crate::values::specified::url::SpecifiedImageUrl;
use crate::values::specified::{Angle, Color, Length, LengthPercentage}; use crate::values::specified::{Angle, Color, Length, LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage};
use crate::values::specified::{Number, NumberOrPercentage, Percentage}; use crate::values::specified::{Number, NumberOrPercentage, Percentage};
use crate::Atom; use crate::Atom;
use cssparser::{Delimiter, Parser, Token}; use cssparser::{Delimiter, Parser, Token};
@ -54,7 +55,14 @@ pub type Image = generic::Image<Gradient, MozImageRect, SpecifiedImageUrl>;
/// Specified values for a CSS gradient. /// Specified values for a CSS gradient.
/// <https://drafts.csswg.org/css-images/#gradients> /// <https://drafts.csswg.org/css-images/#gradients>
pub type Gradient = generic::Gradient<LineDirection, Length, LengthPercentage, Position, Color>; pub type Gradient = generic::Gradient<
LineDirection,
LengthPercentage,
NonNegativeLength,
NonNegativeLengthPercentage,
Position,
Color,
>;
impl SpecifiedValueInfo for Gradient { impl SpecifiedValueInfo for Gradient {
const SUPPORTED_TYPES: u8 = CssType::GRADIENT; const SUPPORTED_TYPES: u8 = CssType::GRADIENT;
@ -80,7 +88,7 @@ impl SpecifiedValueInfo for Gradient {
} }
/// A specified gradient kind. /// A specified gradient kind.
pub type GradientKind = generic::GradientKind<LineDirection, Length, LengthPercentage, Position>; pub type GradientKind = generic::GradientKind<LineDirection, NonNegativeLength, NonNegativeLengthPercentage, Position>;
/// A specified gradient line direction. /// A specified gradient line direction.
/// ///
@ -98,7 +106,7 @@ pub enum LineDirection {
} }
/// A specified ending shape. /// A specified ending shape.
pub type EndingShape = generic::EndingShape<Length, LengthPercentage>; pub type EndingShape = generic::EndingShape<NonNegativeLength, NonNegativeLengthPercentage>;
/// A specified gradient item. /// A specified gradient item.
pub type GradientItem = generic::GradientItem<Color, LengthPercentage>; pub type GradientItem = generic::GradientItem<Color, LengthPercentage>;
@ -391,11 +399,11 @@ impl Gradient {
"radial" => { "radial" => {
let first_point = Point::parse(context, input)?; let first_point = Point::parse(context, input)?;
input.expect_comma()?; input.expect_comma()?;
let first_radius = Number::parse(context, input)?; let first_radius = Number::parse_non_negative(context, input)?;
input.expect_comma()?; input.expect_comma()?;
let second_point = Point::parse(context, input)?; let second_point = Point::parse(context, input)?;
input.expect_comma()?; input.expect_comma()?;
let second_radius = Number::parse(context, input)?; let second_radius = Number::parse_non_negative(context, input)?;
let (reverse_stops, point, radius) = if second_radius.value >= first_radius.value { let (reverse_stops, point, radius) = if second_radius.value >= first_radius.value {
(false, second_point, second_radius) (false, second_point, second_radius)
@ -403,7 +411,7 @@ impl Gradient {
(true, first_point, first_radius) (true, first_point, first_radius)
}; };
let rad = Circle::Radius(Length::from_px(radius.value)); let rad = Circle::Radius(NonNegative(Length::from_px(radius.value)));
let shape = generic::EndingShape::Circle(rad); let shape = generic::EndingShape::Circle(rad);
let position: Position = point.into(); let position: Position = point.into();
@ -678,7 +686,7 @@ impl EndingShape {
return Ok(generic::EndingShape::Circle(Circle::Extent(extent))); return Ok(generic::EndingShape::Circle(Circle::Extent(extent)));
} }
if compat_mode == GradientCompatMode::Modern { if compat_mode == GradientCompatMode::Modern {
if let Ok(length) = input.try(|i| Length::parse(context, i)) { if let Ok(length) = input.try(|i| NonNegativeLength::parse(context, i)) {
return Ok(generic::EndingShape::Circle(Circle::Radius(length))); return Ok(generic::EndingShape::Circle(Circle::Radius(length)));
} }
} }
@ -692,8 +700,8 @@ impl EndingShape {
} }
if compat_mode == GradientCompatMode::Modern { if compat_mode == GradientCompatMode::Modern {
let pair: Result<_, ParseError> = input.try(|i| { let pair: Result<_, ParseError> = input.try(|i| {
let x = LengthPercentage::parse(context, i)?; let x = NonNegativeLengthPercentage::parse(context, i)?;
let y = LengthPercentage::parse(context, i)?; let y = NonNegativeLengthPercentage::parse(context, i)?;
Ok((x, y)) Ok((x, y))
}); });
if let Ok((x, y)) = pair { if let Ok((x, y)) = pair {
@ -704,24 +712,24 @@ impl EndingShape {
ShapeExtent::FarthestCorner, ShapeExtent::FarthestCorner,
))); )));
} }
if let Ok(length) = input.try(|i| Length::parse(context, i)) { if let Ok(length) = input.try(|i| NonNegativeLength::parse(context, i)) {
if let Ok(y) = input.try(|i| LengthPercentage::parse(context, i)) { if let Ok(y) = input.try(|i| NonNegativeLengthPercentage::parse(context, i)) {
if compat_mode == GradientCompatMode::Modern { if compat_mode == GradientCompatMode::Modern {
let _ = input.try(|i| i.expect_ident_matching("ellipse")); let _ = input.try(|i| i.expect_ident_matching("ellipse"));
} }
return Ok(generic::EndingShape::Ellipse(Ellipse::Radii( return Ok(generic::EndingShape::Ellipse(Ellipse::Radii(
length.into(), NonNegative(LengthPercentage::from(length.0)),
y, y,
))); )));
} }
if compat_mode == GradientCompatMode::Modern { if compat_mode == GradientCompatMode::Modern {
let y = input.try(|i| { let y = input.try(|i| {
i.expect_ident_matching("ellipse")?; i.expect_ident_matching("ellipse")?;
LengthPercentage::parse(context, i) NonNegativeLengthPercentage::parse(context, i)
}); });
if let Ok(y) = y { if let Ok(y) = y {
return Ok(generic::EndingShape::Ellipse(Ellipse::Radii( return Ok(generic::EndingShape::Ellipse(Ellipse::Radii(
length.into(), NonNegative(LengthPercentage::from(length.0)),
y, y,
))); )));
} }
@ -731,8 +739,8 @@ impl EndingShape {
return Ok(generic::EndingShape::Circle(Circle::Radius(length))); return Ok(generic::EndingShape::Circle(Circle::Radius(length)));
} }
input.try(|i| { input.try(|i| {
let x = Percentage::parse(context, i)?; let x = Percentage::parse_non_negative(context, i)?;
let y = if let Ok(y) = i.try(|i| LengthPercentage::parse(context, i)) { let y = if let Ok(y) = i.try(|i| NonNegativeLengthPercentage::parse(context, i)) {
if compat_mode == GradientCompatMode::Modern { if compat_mode == GradientCompatMode::Modern {
let _ = i.try(|i| i.expect_ident_matching("ellipse")); let _ = i.try(|i| i.expect_ident_matching("ellipse"));
} }
@ -741,9 +749,9 @@ impl EndingShape {
if compat_mode == GradientCompatMode::Modern { if compat_mode == GradientCompatMode::Modern {
i.expect_ident_matching("ellipse")?; i.expect_ident_matching("ellipse")?;
} }
LengthPercentage::parse(context, i)? NonNegativeLengthPercentage::parse(context, i)?
}; };
Ok(generic::EndingShape::Ellipse(Ellipse::Radii(x.into(), y))) Ok(generic::EndingShape::Ellipse(Ellipse::Radii(NonNegative(LengthPercentage::from(x)), y)))
}) })
} }
} }

View file

@ -704,14 +704,14 @@ impl Parse for NonNegativeLength {
impl From<NoCalcLength> for NonNegativeLength { impl From<NoCalcLength> for NonNegativeLength {
#[inline] #[inline]
fn from(len: NoCalcLength) -> Self { fn from(len: NoCalcLength) -> Self {
NonNegative::<Length>(Length::NoCalc(len)) NonNegative(Length::NoCalc(len))
} }
} }
impl From<Length> for NonNegativeLength { impl From<Length> for NonNegativeLength {
#[inline] #[inline]
fn from(len: Length) -> Self { fn from(len: Length) -> Self {
NonNegative::<Length>(len) NonNegative(len)
} }
} }

View file

@ -62,7 +62,7 @@ pub use self::length::{FontRelativeLength, Length, LengthOrNumber, NonNegativeLe
pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto}; pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto};
pub use self::length::{MaxSize, Size}; pub use self::length::{MaxSize, Size};
pub use self::length::{NoCalcLength, ViewportPercentageLength}; pub use self::length::{NoCalcLength, ViewportPercentageLength};
pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto}; pub use self::length::{NonNegativeLength, NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto};
#[cfg(feature = "gecko")] #[cfg(feature = "gecko")]
pub use self::list::ListStyleType; pub use self::list::ListStyleType;
pub use self::list::MozListReversed; pub use self::list::MozListReversed;