Skip to content

Commit

Permalink
Auto merge of #13121 - emilio:negative-calc, r=SimonSapin
Browse files Browse the repository at this point in the history
style: Properly track whether negative values of calc() are allowed

<!-- Please describe your changes on the following line: -->

r? @SimonSapin

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [x] There are tests for these changes OR

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

In order to clamp them at computed value time.

<!-- 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/13121)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Sep 1, 2016
2 parents e699d9b + 2617d15 commit fbf77e4
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 27 deletions.
1 change: 1 addition & 0 deletions components/style/values/computed/mod.rs
Expand Up @@ -167,6 +167,7 @@ impl ToComputedValue for specified::CalcLengthOrPercentage {
self.compute_from_viewport_and_font_size(context.viewport_size(),
context.style().get_font().clone_font_size(),
context.style().root_font_size())

}
}

Expand Down
88 changes: 61 additions & 27 deletions components/style/values/specified/mod.rs
Expand Up @@ -286,22 +286,24 @@ impl Length {
}

#[inline]
fn parse_internal(input: &mut Parser, context: &AllowedNumericType) -> Result<Length, ()> {
fn parse_internal(input: &mut Parser, context: AllowedNumericType) -> Result<Length, ()> {
match try!(input.next()) {
Token::Dimension(ref value, ref unit) if context.is_ok(value.value) =>
Length::parse_dimension(value.value, unit),
Token::Number(ref value) if value.value == 0. =>
Ok(Length::Absolute(Au(0))),
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") =>
input.parse_nested_block(CalcLengthOrPercentage::parse_length),
input.parse_nested_block(|input| {
CalcLengthOrPercentage::parse_length(input, context)
}),
_ => Err(())
}
}
pub fn parse(input: &mut Parser) -> Result<Length, ()> {
Length::parse_internal(input, &AllowedNumericType::All)
Length::parse_internal(input, AllowedNumericType::All)
}
pub fn parse_non_negative(input: &mut Parser) -> Result<Length, ()> {
Length::parse_internal(input, &AllowedNumericType::NonNegative)
Length::parse_internal(input, AllowedNumericType::NonNegative)
}
pub fn parse_dimension(value: CSSFloat, unit: &str) -> Result<Length, ()> {
match_ignore_ascii_case! { unit,
Expand Down Expand Up @@ -467,6 +469,8 @@ pub struct CalcLengthOrPercentage {
pub ch: Option<FontRelativeLength>,
pub rem: Option<FontRelativeLength>,
pub percentage: Option<Percentage>,
/// Whether the value returned can be negative at computed value time.
pub allowed_numeric_type: AllowedNumericType,
}

impl CalcLengthOrPercentage {
Expand Down Expand Up @@ -617,15 +621,19 @@ impl CalcLengthOrPercentage {
}
}

fn parse_length(input: &mut Parser) -> Result<Length, ()> {
CalcLengthOrPercentage::parse(input, CalcUnit::Length).map(Length::Calc)
fn parse_length(input: &mut Parser,
context: AllowedNumericType) -> Result<Length, ()> {
CalcLengthOrPercentage::parse(input, CalcUnit::Length, context).map(Length::Calc)
}

fn parse_length_or_percentage(input: &mut Parser) -> Result<CalcLengthOrPercentage, ()> {
CalcLengthOrPercentage::parse(input, CalcUnit::LengthOrPercentage)
fn parse_length_or_percentage(input: &mut Parser,
context: AllowedNumericType) -> Result<CalcLengthOrPercentage, ()> {
CalcLengthOrPercentage::parse(input, CalcUnit::LengthOrPercentage, context)
}

fn parse(input: &mut Parser, expected_unit: CalcUnit) -> Result<CalcLengthOrPercentage, ()> {
fn parse(input: &mut Parser,
expected_unit: CalcUnit,
context: AllowedNumericType) -> Result<CalcLengthOrPercentage, ()> {
let ast = try!(CalcLengthOrPercentage::parse_sum(input, expected_unit));

let mut simplified = Vec::new();
Expand Down Expand Up @@ -692,6 +700,7 @@ impl CalcLengthOrPercentage {
ch: ch.map(FontRelativeLength::Ch),
rem: rem.map(FontRelativeLength::Rem),
percentage: percentage.map(Percentage),
allowed_numeric_type: context,
})
}

Expand Down Expand Up @@ -770,16 +779,31 @@ impl CalcLengthOrPercentage {
val.to_computed_value(viewport_size));
}
}

for val in &[self.ch, self.em, self.ex, self.rem] {
if let Some(val) = *val {
length = Some(length.unwrap_or(Au(0)) + val.to_computed_value(
font_size, root_font_size));
}
}

// https://drafts.csswg.org/css-values/#calc-range
let mut percentage = self.percentage.map(|p| p.0);
if let AllowedNumericType::NonNegative = self.allowed_numeric_type {
if let Some(ref mut length) = length {
*length = cmp::max(*length, Au(0));
}

if let Some(ref mut percentage) = percentage {
if *percentage < 0. {
*percentage = 0.;
}
}
}

computed::CalcLengthOrPercentage {
length: length,
percentage: self.percentage.map(|p| p.0)
percentage: percentage,
}
}
}
Expand Down Expand Up @@ -884,7 +908,7 @@ impl LengthOrPercentage {
LengthOrPercentage::Length(Length::Absolute(Au(0)))
}

fn parse_internal(input: &mut Parser, context: &AllowedNumericType)
fn parse_internal(input: &mut Parser, context: AllowedNumericType)
-> Result<LengthOrPercentage, ()>
{
match try!(input.next()) {
Expand All @@ -895,19 +919,21 @@ impl LengthOrPercentage {
Token::Number(ref value) if value.value == 0. =>
Ok(LengthOrPercentage::Length(Length::Absolute(Au(0)))),
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
let calc = try!(input.parse_nested_block(|input| {
CalcLengthOrPercentage::parse_length_or_percentage(input, context)
}));
Ok(LengthOrPercentage::Calc(calc))
},
_ => Err(())
}
}
#[inline]
pub fn parse(input: &mut Parser) -> Result<LengthOrPercentage, ()> {
LengthOrPercentage::parse_internal(input, &AllowedNumericType::All)
LengthOrPercentage::parse_internal(input, AllowedNumericType::All)
}
#[inline]
pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrPercentage, ()> {
LengthOrPercentage::parse_internal(input, &AllowedNumericType::NonNegative)
LengthOrPercentage::parse_internal(input, AllowedNumericType::NonNegative)
}
}

Expand Down Expand Up @@ -942,7 +968,7 @@ impl ToCss for LengthOrPercentageOrAuto {
}

impl LengthOrPercentageOrAuto {
fn parse_internal(input: &mut Parser, context: &AllowedNumericType)
fn parse_internal(input: &mut Parser, context: AllowedNumericType)
-> Result<LengthOrPercentageOrAuto, ()>
{
match try!(input.next()) {
Expand All @@ -955,19 +981,21 @@ impl LengthOrPercentageOrAuto {
Token::Ident(ref value) if value.eq_ignore_ascii_case("auto") =>
Ok(LengthOrPercentageOrAuto::Auto),
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
let calc = try!(input.parse_nested_block(|input| {
CalcLengthOrPercentage::parse_length_or_percentage(input, context)
}));
Ok(LengthOrPercentageOrAuto::Calc(calc))
},
_ => Err(())
}
}
#[inline]
pub fn parse(input: &mut Parser) -> Result<LengthOrPercentageOrAuto, ()> {
LengthOrPercentageOrAuto::parse_internal(input, &AllowedNumericType::All)
LengthOrPercentageOrAuto::parse_internal(input, AllowedNumericType::All)
}
#[inline]
pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrPercentageOrAuto, ()> {
LengthOrPercentageOrAuto::parse_internal(input, &AllowedNumericType::NonNegative)
LengthOrPercentageOrAuto::parse_internal(input, AllowedNumericType::NonNegative)
}
}

Expand Down Expand Up @@ -1001,7 +1029,7 @@ impl ToCss for LengthOrPercentageOrNone {
}
}
impl LengthOrPercentageOrNone {
fn parse_internal(input: &mut Parser, context: &AllowedNumericType)
fn parse_internal(input: &mut Parser, context: AllowedNumericType)
-> Result<LengthOrPercentageOrNone, ()>
{
match try!(input.next()) {
Expand All @@ -1012,7 +1040,9 @@ impl LengthOrPercentageOrNone {
Token::Number(ref value) if value.value == 0. =>
Ok(LengthOrPercentageOrNone::Length(Length::Absolute(Au(0)))),
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
let calc = try!(input.parse_nested_block(|input| {
CalcLengthOrPercentage::parse_length_or_percentage(input, context)
}));
Ok(LengthOrPercentageOrNone::Calc(calc))
},
Token::Ident(ref value) if value.eq_ignore_ascii_case("none") =>
Expand All @@ -1022,11 +1052,11 @@ impl LengthOrPercentageOrNone {
}
#[inline]
pub fn parse(input: &mut Parser) -> Result<LengthOrPercentageOrNone, ()> {
LengthOrPercentageOrNone::parse_internal(input, &AllowedNumericType::All)
LengthOrPercentageOrNone::parse_internal(input, AllowedNumericType::All)
}
#[inline]
pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrPercentageOrNone, ()> {
LengthOrPercentageOrNone::parse_internal(input, &AllowedNumericType::NonNegative)
LengthOrPercentageOrNone::parse_internal(input, AllowedNumericType::NonNegative)
}
}

Expand Down Expand Up @@ -1055,7 +1085,7 @@ impl ToCss for LengthOrNone {
}
}
impl LengthOrNone {
fn parse_internal(input: &mut Parser, context: &AllowedNumericType)
fn parse_internal(input: &mut Parser, context: AllowedNumericType)
-> Result<LengthOrNone, ()>
{
match try!(input.next()) {
Expand All @@ -1064,19 +1094,21 @@ impl LengthOrNone {
Token::Number(ref value) if value.value == 0. =>
Ok(LengthOrNone::Length(Length::Absolute(Au(0)))),
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") =>
input.parse_nested_block(CalcLengthOrPercentage::parse_length).map(LengthOrNone::Length),
input.parse_nested_block(|input| {
CalcLengthOrPercentage::parse_length(input, context)
}).map(LengthOrNone::Length),
Token::Ident(ref value) if value.eq_ignore_ascii_case("none") =>
Ok(LengthOrNone::None),
_ => Err(())
}
}
#[inline]
pub fn parse(input: &mut Parser) -> Result<LengthOrNone, ()> {
LengthOrNone::parse_internal(input, &AllowedNumericType::All)
LengthOrNone::parse_internal(input, AllowedNumericType::All)
}
#[inline]
pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrNone, ()> {
LengthOrNone::parse_internal(input, &AllowedNumericType::NonNegative)
LengthOrNone::parse_internal(input, AllowedNumericType::NonNegative)
}
}

Expand Down Expand Up @@ -1127,7 +1159,9 @@ impl LengthOrPercentageOrAutoOrContent {
Token::Ident(ref value) if value.eq_ignore_ascii_case("content") =>
Ok(LengthOrPercentageOrAutoOrContent::Content),
Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
let calc = try!(input.parse_nested_block(|input| {
CalcLengthOrPercentage::parse_length_or_percentage(input, context)
}));
Ok(LengthOrPercentageOrAutoOrContent::Calc(calc))
},
_ => Err(())
Expand Down
2 changes: 2 additions & 0 deletions components/style_traits/values.rs
Expand Up @@ -63,6 +63,8 @@ macro_rules! __define_css_keyword_enum__actual {


pub mod specified {
#[repr(u8)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum AllowedNumericType {
All,
Expand Down

0 comments on commit fbf77e4

Please sign in to comment.