Skip to content

Commit

Permalink
Bug 1808409 - Part 4: Accept auto for {scroll|view}-timeline-name. r=…
Browse files Browse the repository at this point in the history
…emilio

Per w3c/csswg-drafts#7431, both timeline name
should accept `auto`.

Differential Revision: https://phabricator.services.mozilla.com/D166476
  • Loading branch information
BorisChiou committed Jan 24, 2023
1 parent fc5ae50 commit ed3876e
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 91 deletions.
1 change: 1 addition & 0 deletions layout/style/ServoBindings.toml
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ cbindgen-types = [
{ gecko = "StyleScrollSnapStop", servo = "crate::values::computed::ScrollSnapStop" },
{ gecko = "StyleScrollSnapStrictness", servo = "crate::values::computed::ScrollSnapStrictness" },
{ gecko = "StyleScrollSnapType", servo = "crate::values::computed::ScrollSnapType" },
{ gecko = "StyleAnimationName", servo = "crate::values::computed::AnimationName" },
{ gecko = "StyleScrollTimelineName", servo = "crate::values::computed::ScrollTimelineName" },
{ gecko = "StyleScrollAxis", servo = "crate::values::computed::ScrollAxis" },
{ gecko = "StyleViewTimelineInset", servo = "crate::values::computed::ViewTimelineInset" },
Expand Down
14 changes: 6 additions & 8 deletions layout/style/test/property_database.js
Original file line number Diff line number Diff line change
Expand Up @@ -13652,6 +13652,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
initial_values: ["none"],
other_values: [
"all",
"auto",
"ball",
"mall",
"color",
Expand All @@ -13664,13 +13665,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
"\\2bounce",
"-\\2bounce",
],
invalid_values: [
"auto",
"bounce, abc",
"abc bounce",
"10px",
"rgb(1, 2, 3)",
],
invalid_values: ["bounce, abc", "abc bounce", "10px", "rgb(1, 2, 3)"],
};

gCSSProperties["scroll-timeline-axis"] = {
Expand All @@ -13689,6 +13684,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
subproperties: ["scroll-timeline-name", "scroll-timeline-axis"],
initial_values: ["none block", "block none", "block", "none"],
other_values: [
"auto inline",
"bounce inline",
"bounce vertical",
"horizontal bounce",
Expand All @@ -13707,6 +13703,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
initial_values: ["none"],
other_values: [
"all",
"auto",
"ball",
"mall",
"color",
Expand All @@ -13721,7 +13718,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
"bounce, abc",
"none, none",
],
invalid_values: ["auto", "abc bounce", "10px", "rgb(1, 2, 3)"],
invalid_values: ["abc bounce", "10px", "rgb(1, 2, 3)"],
};

gCSSProperties["view-timeline-axis"] = {
Expand Down Expand Up @@ -13749,6 +13746,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
subproperties: ["view-timeline-name", "view-timeline-axis"],
initial_values: ["none block", "none"],
other_values: [
"auto inline",
"bounce inline",
"bounce vertical",
"\\32bounce inline",
Expand Down
8 changes: 1 addition & 7 deletions servo/components/style/values/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,13 +600,6 @@ impl TimelineOrKeyframesName {

impl Eq for TimelineOrKeyframesName {}

/// A trait that returns whether a given type is the `auto` value or not. So far
/// only needed for background-size serialization, which special-cases `auto`.
pub trait IsAuto {
/// Returns whether the value is the `auto` value.
fn is_auto(&self) -> bool;
}

/// The typedef of <timeline-name>.
#[repr(transparent)]
#[derive(
Expand Down Expand Up @@ -659,6 +652,7 @@ impl ToCss for TimelineName {
}

/// The typedef of <keyframes-name>.
#[repr(transparent)]
#[derive(
Clone,
Debug,
Expand Down
59 changes: 6 additions & 53 deletions servo/components/style/values/specified/box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,7 @@ impl AnimationIterationCount {
ToShmem,
)]
#[value_info(other_values = "none")]
#[repr(C)]
pub struct AnimationName(pub KeyframesName);

impl AnimationName {
Expand Down Expand Up @@ -825,8 +826,9 @@ fn is_default<T: Default + PartialEq>(value: &T) -> bool {
pub enum AnimationTimeline {
/// Use default timeline. The animation’s timeline is a DocumentTimeline.
Auto,
/// The scroll-timeline name.
/// The scroll-timeline name or view-timeline-name.
/// https://drafts.csswg.org/scroll-animations-1/#scroll-timelines-named
/// https://drafts.csswg.org/scroll-animations-1/#view-timeline-name
Timeline(TimelineName),
/// The scroll() notation.
/// https://drafts.csswg.org/scroll-animations-1/#scroll-notation
Expand Down Expand Up @@ -854,15 +856,6 @@ impl Parse for AnimationTimeline {
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
// We are using the same parser for TimelineName and KeyframesName, but animation-timeline
// accepts "auto", so need to manually parse this. (We can not derive
// Parse because TimelineName excludes only the "none" keyword).
//
// FIXME: Bug 1733260: we may drop None based on the spec issue:
// https://github.com/w3c/csswg-drafts/issues/6674
//
// If `none` is removed, then we could potentially shrink this the same
// way we deal with animation-name.
if input.try_parse(|i| i.expect_ident_matching("auto")).is_ok() {
return Ok(Self::Auto);
}
Expand All @@ -871,7 +864,7 @@ impl Parse for AnimationTimeline {
return Ok(AnimationTimeline::Timeline(TimelineName::none()));
}

// https://drafts.csswg.org/scroll-animations-1/rewrite#scroll-notation
// https://drafts.csswg.org/scroll-animations-1/#scroll-notation
if input
.try_parse(|i| i.expect_function_matching("scroll"))
.is_ok()
Expand All @@ -888,48 +881,8 @@ impl Parse for AnimationTimeline {
}
}

/// A value for the scroll-timeline-name.
///
/// Note: The spec doesn't mention `auto` for scroll-timeline-name. However, `auto` is a keyword in
/// animation-timeline, so we reject `auto` for scroll-timeline-name now.
///
/// https://drafts.csswg.org/scroll-animations-1/#scroll-timeline-name
#[derive(
Clone,
Debug,
Eq,
Hash,
MallocSizeOf,
PartialEq,
SpecifiedValueInfo,
ToComputedValue,
ToCss,
ToResolvedValue,
ToShmem,
)]
#[repr(C)]
pub struct ScrollTimelineName(pub TimelineName);

impl ScrollTimelineName {
/// Returns the `none` value.
pub fn none() -> Self {
Self(TimelineName::none())
}
}

impl Parse for ScrollTimelineName {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
if let Ok(name) = input.try_parse(|input| TimelineName::parse(context, input)) {
return Ok(Self(name));
}

input.expect_ident_matching("none")?;
Ok(Self(TimelineName::none()))
}
}
/// A value for the scroll-timeline-name or view-timeline-name.
pub type ScrollTimelineName = AnimationName;

/// https://drafts.csswg.org/css-scroll-snap-1/#snap-axis
#[allow(missing_docs)]
Expand Down
4 changes: 2 additions & 2 deletions servo/ports/geckolib/cbindgen.toml
Original file line number Diff line number Diff line change
Expand Up @@ -949,9 +949,9 @@ renaming_overrides_prefixing = true
SERVO_FIXED_POINT_HELPERS(StyleFontStretch, uint16_t, StyleFONT_STRETCH_FRACTION_BITS);
"""

"ScrollTimelineName" = """
"AnimationName" = """
public:
StyleScrollTimelineName(): _0(RefPtr<nsAtom>(nsGkAtoms::_empty).forget()) {}
StyleAnimationName(): _0(RefPtr<nsAtom>(nsGkAtoms::_empty).forget()) {}
"""

"GenericViewTimelineInset" = """
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit ed3876e

Please sign in to comment.