style: Remove LengthPercentage::was_calc.

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

Differential Revision: https://phabricator.services.mozilla.com/D43747
This commit is contained in:
Emilio Cobos Álvarez 2019-09-12 08:14:44 +00:00
parent ec408e9a57
commit c78f1b62de
3 changed files with 23 additions and 42 deletions

View file

@ -26,13 +26,10 @@ impl Animate for LengthPercentage {
.animate(&other.unclamped_length(), procedure)?; .animate(&other.unclamped_length(), procedure)?;
let percentage = let percentage =
animate_percentage_half(self.specified_percentage(), other.specified_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( Ok(Self::with_clamping_mode(
length, length,
percentage, percentage,
self.clamping_mode, self.clamping_mode,
is_calc,
)) ))
} }
} }

View file

@ -85,25 +85,20 @@ pub struct LengthPercentage {
/// Whether we specified a percentage or not. /// Whether we specified a percentage or not.
#[animation(constant)] #[animation(constant)]
pub has_percentage: bool, 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 // NOTE(emilio): We don't compare `clamping_mode` since we want to preserve the
// representation of LengthPercentage with Gecko. The issue here is that Gecko // invariant that `from_computed_value(length).to_computed_value(..) == length`.
// uses CalcValue to represent position components, so they always come back as
// was_calc == true, and we mess up in the transitions code.
// //
// This was a pre-existing bug, though arguably so only in pretty obscure cases // Right now for e.g. a non-negative length, we set clamping_mode to `All`
// like calc(0px + 5%) and such. // unconditionally for non-calc values, and to `NonNegative` for calc.
//
// 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 { impl PartialEq for LengthPercentage {
fn eq(&self, other: &Self) -> bool { fn eq(&self, other: &Self) -> bool {
self.length == other.length && self.length == other.length &&
@ -133,7 +128,6 @@ impl LengthPercentage {
length, length,
percentage, percentage,
AllowedNumericType::All, AllowedNumericType::All,
/* was_calc = */ false,
) )
} }
@ -148,14 +142,12 @@ impl LengthPercentage {
length: Length, length: Length,
percentage: Option<Percentage>, percentage: Option<Percentage>,
clamping_mode: AllowedNumericType, clamping_mode: AllowedNumericType,
was_calc: bool,
) -> Self { ) -> Self {
Self { Self {
clamping_mode, clamping_mode,
length, length,
percentage: percentage.unwrap_or_default(), percentage: percentage.unwrap_or_default(),
has_percentage: percentage.is_some(), has_percentage: percentage.is_some(),
was_calc,
} }
} }
@ -285,7 +277,6 @@ impl specified::CalcLengthPercentage {
Length::new(length.min(f32::MAX).max(f32::MIN)), Length::new(length.min(f32::MAX).max(f32::MIN)),
self.percentage, self.percentage,
self.clamping_mode, self.clamping_mode,
/* was_calc = */ true,
) )
} }
@ -381,35 +372,29 @@ impl LengthPercentage {
} }
/// Returns the clamped non-negative values. /// 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] #[inline]
pub fn clamp_to_non_negative(self) -> Self { pub fn clamp_to_non_negative(self) -> Self {
if self.was_calc { 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( return Self::with_clamping_mode(
self.length, self.length,
self.specified_percentage(), Some(p),
AllowedNumericType::NonNegative, AllowedNumericType::NonNegative,
self.was_calc, )
);
}
debug_assert!(!self.has_percentage || self.unclamped_length() == Length::zero());
if let Some(p) = self.specified_percentage() {
return Self::with_clamping_mode(
Length::zero(),
Some(p.clamp_to_non_negative()),
AllowedNumericType::NonNegative,
self.was_calc,
);
} }
Self::with_clamping_mode( Self::with_clamping_mode(
self.length.clamp_to_non_negative(), self.length.clamp_to_non_negative(),
None, None,
AllowedNumericType::NonNegative, AllowedNumericType::NonNegative,
self.was_calc,
) )
} }
} }

View file

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