Skip to content
Permalink
Browse files

style: Remove LengthPercentage::was_calc.

There should not be any behavior change between specifying a percentage using %
or calc(%) per the resolution of w3c/csswg-drafts#3482.

Differential Revision: https://phabricator.services.mozilla.com/D43747
  • Loading branch information...
emilio committed Sep 12, 2019
1 parent ec408e9 commit c78f1b62def53884ecaee2e5df4eb19e4a6c25e9
@@ -26,13 +26,10 @@ impl Animate for LengthPercentage {
.animate(&other.unclamped_length(), procedure)?;
let percentage =
animate_percentage_half(self.specified_percentage(), other.specified_percentage())?;
let is_calc =
self.was_calc || other.was_calc || self.has_percentage != other.has_percentage;
Ok(Self::with_clamping_mode(
length,
percentage,
self.clamping_mode,
is_calc,
))
}
}
@@ -85,25 +85,20 @@ pub struct LengthPercentage {
/// Whether we specified a percentage or not.
#[animation(constant)]
pub has_percentage: bool,
/// Whether this was from a calc() expression. This is needed because right
/// now we don't treat calc() the same way as non-calc everywhere, but
/// that's a bug in most cases.
///
/// Please don't add new uses of this that aren't for converting to Gecko's
/// representation, or to interpolate values.
///
/// See https://github.com/w3c/csswg-drafts/issues/3482.
#[animation(constant)]
pub was_calc: bool,
}

// FIXME(emilio): This is a bit of a hack that can disappear as soon as we share
// representation of LengthPercentage with Gecko. The issue here is that Gecko
// uses CalcValue to represent position components, so they always come back as
// was_calc == true, and we mess up in the transitions code.
// NOTE(emilio): We don't compare `clamping_mode` since we want to preserve the
// invariant that `from_computed_value(length).to_computed_value(..) == length`.
//
// Right now for e.g. a non-negative length, we set clamping_mode to `All`
// unconditionally for non-calc values, and to `NonNegative` for calc.
//
// This was a pre-existing bug, though arguably so only in pretty obscure cases
// like calc(0px + 5%) and such.
// If we determine that it's sound, from_computed_value() can generate an
// absolute length, which then would get `All` as the clamping mode.
//
// We may want to just eagerly-detect whether we can clamp in
// `LengthPercentage::new` and switch to `AllowedNumericType::NonNegative` then,
// maybe.
impl PartialEq for LengthPercentage {
fn eq(&self, other: &Self) -> bool {
self.length == other.length &&
@@ -133,7 +128,6 @@ impl LengthPercentage {
length,
percentage,
AllowedNumericType::All,
/* was_calc = */ false,
)
}

@@ -148,14 +142,12 @@ impl LengthPercentage {
length: Length,
percentage: Option<Percentage>,
clamping_mode: AllowedNumericType,
was_calc: bool,
) -> Self {
Self {
clamping_mode,
length,
percentage: percentage.unwrap_or_default(),
has_percentage: percentage.is_some(),
was_calc,
}
}

@@ -285,7 +277,6 @@ impl specified::CalcLengthPercentage {
Length::new(length.min(f32::MAX).max(f32::MIN)),
self.percentage,
self.clamping_mode,
/* was_calc = */ true,
)
}

@@ -381,35 +372,29 @@ impl LengthPercentage {
}

/// Returns the clamped non-negative values.
///
/// TODO(emilio): It's a bit unfortunate that this depends on whether the
/// value was a `calc()` value or not. Should it?
#[inline]
pub fn clamp_to_non_negative(self) -> Self {
if self.was_calc {
return Self::with_clamping_mode(
self.length,
self.specified_percentage(),
AllowedNumericType::NonNegative,
self.was_calc,
);
}

debug_assert!(!self.has_percentage || self.unclamped_length() == Length::zero());
if let Some(p) = self.specified_percentage() {
// If we can eagerly clamp the percentage then just do that.
if self.length.is_zero() {
return Self::with_clamping_mode(
Length::zero(),
Some(p.clamp_to_non_negative()),
AllowedNumericType::NonNegative,
);
}

return Self::with_clamping_mode(
Length::zero(),
Some(p.clamp_to_non_negative()),
self.length,
Some(p),
AllowedNumericType::NonNegative,
self.was_calc,
);
)
}

Self::with_clamping_mode(
self.length.clamp_to_non_negative(),
None,
AllowedNumericType::NonNegative,
self.was_calc,
)
}
}
@@ -301,7 +301,6 @@ impl<S: Side> ToComputedValue for PositionComponent<S> {
l,
Some(p),
length.clamping_mode,
/* was_calc = */ true,
)
},
PositionComponent::Side(_, Some(ref length)) |

0 comments on commit c78f1b6

Please sign in to comment.
You can’t perform that action at this time.