Bug 1808410 - Part 2: Add animation-timeline: view() in style system. r=emilio

Support view() notation for animation-timeline: `<view()> = view( [ <axis> || <'view-timeline-inset'> ]? )` We move AnimationTimeline and its related types into the generics folder, and define two new structs for scroll() and view(). Note: 1. The syntax of scroll() doesn't match the current version of the spec. I will update it in Bug 1814444. 2. We will handle the creation/usage of the Anonymous View Progress Timelines in the next patch. Differential Revision: https://phabricator.services.mozilla.com/D173904
2023-05-02 22:47:28 +00:00 · 2023-05-02 22:47:28 +00:00 · 97804f78e4
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@ -59,7 +59,8 @@ exports.CSS_PROPERTIES = {
      "step-end",
      "step-start",
      "steps",
-      "unset"
+      "unset",
+      "view"
    ]
  },
  "-moz-animation-delay": {
@ -1331,7 +1332,8 @@ exports.CSS_PROPERTIES = {
      "step-end",
      "step-start",
      "steps",
-      "unset"
+      "unset",
+      "view"
    ]
  },
  "-webkit-animation-delay": {
@ -3366,7 +3368,8 @@ exports.CSS_PROPERTIES = {
      "step-end",
      "step-start",
      "steps",
-      "unset"
+      "unset",
+      "view"
    ]
  },
  "animation-composition": {
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@ -268,8 +268,10 @@ static already_AddRefed<dom::AnimationTimeline> GetTimeline(
    case StyleAnimationTimeline::Tag::Scroll: {
      const auto& scroll = aStyleTimeline.AsScroll();
      return ScrollTimeline::MakeAnonymous(aPresContext->Document(), aTarget,
-                                           scroll._0, scroll._1);
+                                           scroll.axis, scroll.scroller);
    }
+    case StyleAnimationTimeline::Tag::View:
+      // TODO: Support this in this patch series. Treat it as auto for now.
    case StyleAnimationTimeline::Tag::Auto:
      return do_AddRef(aTarget.mElement->OwnerDoc()->Timeline());
  }
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@ -13756,6 +13756,14 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-driven-animations.enabled")) {
      "scroll(nearest)",
      "scroll(inline nearest)",
      "scroll(vertical root)",
+      "view()",
+      "view(inline)",
+      "view(auto)",
+      "view(auto 1px)",
+      "view(inline auto)",
+      "view(vertical auto auto)",
+      "view(horizontal 1px 1%)",
+      "view(1px 1% block)",
    ],
    invalid_values: [
      "bounce, initial",
--- a/servo/components/style/values/computed/animation.rs
+++ b/servo/components/style/values/computed/animation.rs
@ -11,7 +11,7 @@ use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};

 pub use crate::values::specified::animation::{
-    AnimationName, AnimationTimeline, ScrollAxis, ScrollTimelineName, TransitionProperty,
+    AnimationName, ScrollAxis, ScrollTimelineName, TransitionProperty,
 };

 /// A computed value for the `animation-iteration-count` property.
@ -62,5 +62,8 @@ impl ToCss for AnimationIterationCount {
    }
 }

+/// A computed value for the `animation-timeline` property.
+pub type AnimationTimeline = generics::GenericAnimationTimeline<LengthPercentage>;
+
 /// A computed value for the `view-timeline-inset` property.
 pub type ViewTimelineInset = generics::GenericViewTimelineInset<LengthPercentage>;
--- a/servo/components/style/values/generics/animation.rs
+++ b/servo/components/style/values/generics/animation.rs
@ -5,9 +5,82 @@
 //! Generic values for properties related to animations and transitions.

 use crate::values::generics::length::GenericLengthPercentageOrAuto;
+use crate::values::specified::animation::{ScrollAxis, ScrollFunction};
+use crate::values::TimelineName;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};

+/// The view() notation.
+/// https://drafts.csswg.org/scroll-animations-1/#view-notation
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
+)]
+#[css(function = "view")]
+#[repr(C)]
+pub struct GenericViewFunction<LengthPercent> {
+    /// The axis of scrolling that drives the progress of the timeline.
+    #[css(skip_if = "ScrollAxis::is_default")]
+    pub axis: ScrollAxis,
+    /// An adjustment of the view progress visibility range.
+    #[css(skip_if = "GenericViewTimelineInset::is_auto")]
+    #[css(field_bound)]
+    pub inset: GenericViewTimelineInset<LengthPercent>,
+}
+
+pub use self::GenericViewFunction as ViewFunction;
+
+/// A value for the <single-animation-timeline>.
+///
+/// https://drafts.csswg.org/css-animations-2/#typedef-single-animation-timeline
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
+)]
+#[repr(C, u8)]
+pub enum GenericAnimationTimeline<LengthPercent> {
+    /// Use default timeline. The animation’s timeline is a DocumentTimeline.
+    Auto,
+    /// 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
+    Scroll(ScrollFunction),
+    /// The view() notation.
+    /// https://drafts.csswg.org/scroll-animations-1/#view-notation
+    View(#[css(field_bound)] GenericViewFunction<LengthPercent>),
+}
+
+pub use self::GenericAnimationTimeline as AnimationTimeline;
+
+impl<LengthPercent> AnimationTimeline<LengthPercent> {
+    /// Returns the `auto` value.
+    pub fn auto() -> Self {
+        Self::Auto
+    }
+
+    /// Returns true if it is auto (i.e. the default value).
+    pub fn is_auto(&self) -> bool {
+        matches!(self, Self::Auto)
+    }
+}
+
 /// A generic value for the `[ [ auto | <length-percentage> ]{1,2} ]`.
 ///
 /// https://drafts.csswg.org/scroll-animations-1/#view-timeline-inset
@ -32,6 +105,14 @@ pub struct GenericViewTimelineInset<LengthPercent> {

 pub use self::GenericViewTimelineInset as ViewTimelineInset;

+impl<LengthPercent> ViewTimelineInset<LengthPercent> {
+    /// Returns true if it is auto.
+    #[inline]
+    fn is_auto(&self) -> bool {
+        self.start.is_auto() && self.end.is_auto()
+    }
+}
+
 impl<LengthPercent> ToCss for ViewTimelineInset<LengthPercent>
 where
    LengthPercent: PartialEq + ToCss,
--- a/servo/components/style/values/specified/animation.rs
+++ b/servo/components/style/values/specified/animation.rs
@ -13,7 +13,9 @@ use crate::values::{CustomIdent, KeyframesName, TimelineName};
 use crate::Atom;
 use cssparser::Parser;
 use std::fmt::{self, Write};
-use style_traits::{CssWriter, KeywordsCollectFn, ParseError, SpecifiedValueInfo, ToCss};
+use style_traits::{
+    CssWriter, KeywordsCollectFn, ParseError, SpecifiedValueInfo, StyleParseErrorKind, ToCss,
+};

 /// A given transition property, that is either `All`, a longhand or shorthand
 /// property, or an unsupported or custom property.
@ -203,9 +205,15 @@ pub enum Scroller {
    Nearest,
    /// The document viewport as the scroll container.
    Root,
-    // FIXME: Bug 1764450: Once we support container-name CSS property (Bug 1744224), we may add
-    // <custom-ident> here, based on the result of the spec issue:
-    // https://github.com/w3c/csswg-drafts/issues/7046
+    // FIXME: Bug 1814444. Support self keyword.
+}
+
+impl Scroller {
+    /// Returns true if it is default.
+    #[inline]
+    fn is_default(&self) -> bool {
+        matches!(*self, Self::Nearest)
+    }
 }

 impl Default for Scroller {
@ -245,25 +253,25 @@ pub enum ScrollAxis {
    Horizontal = 3,
 }

+impl ScrollAxis {
+    /// Returns true if it is default.
+    #[inline]
+    pub fn is_default(&self) -> bool {
+        matches!(*self, Self::Block)
+    }
+}
+
 impl Default for ScrollAxis {
    fn default() -> Self {
        Self::Block
    }
 }

-#[inline]
-fn is_default<T: Default + PartialEq>(value: &T) -> bool {
-    *value == Default::default()
-}
-
-/// A value for the <single-animation-timeline>.
-///
-/// https://drafts.csswg.org/css-animations-2/#typedef-single-animation-timeline
+/// The scroll() notation.
+/// https://drafts.csswg.org/scroll-animations-1/#scroll-notation
 #[derive(
    Clone,
    Debug,
-    Eq,
-    Hash,
    MallocSizeOf,
    PartialEq,
    SpecifiedValueInfo,
@ -272,40 +280,77 @@ fn is_default<T: Default + PartialEq>(value: &T) -> bool {
    ToResolvedValue,
    ToShmem,
 )]
-#[repr(C, u8)]
-pub enum AnimationTimeline {
-    /// Use default timeline. The animation’s timeline is a DocumentTimeline.
-    Auto,
-    /// 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
-    #[css(function)]
-    Scroll(
-        #[css(skip_if = "is_default")] ScrollAxis,
-        #[css(skip_if = "is_default")] Scroller,
-    ),
+#[css(function = "scroll")]
+#[repr(C)]
+pub struct ScrollFunction {
+    /// The axis of scrolling that drives the progress of the timeline.
+    #[css(skip_if = "ScrollAxis::is_default")]
+    pub axis: ScrollAxis,
+    /// The scroll container element whose scroll position drives the progress of the timeline.
+    #[css(skip_if = "Scroller::is_default")]
+    pub scroller: Scroller,
 }

-impl AnimationTimeline {
-    /// Returns the `auto` value.
-    pub fn auto() -> Self {
-        Self::Auto
-    }
-
-    /// Returns true if it is auto (i.e. the default value).
-    pub fn is_auto(&self) -> bool {
-        matches!(self, Self::Auto)
+impl ScrollFunction {
+    /// Parse the inner function arguments of `scroll()`.
+    fn parse_arguments<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
+        // <scroll()> = scroll( [ <scroller> || <axis> ]? )
+        // https://drafts.csswg.org/scroll-animations-1/#funcdef-scroll
+        //
+        // FIXME: This doesn't match the spec. I will update it in Bug 1814444.
+        Ok(Self {
+            axis: input.try_parse(ScrollAxis::parse).unwrap_or_default(),
+            scroller: input.try_parse(Scroller::parse).unwrap_or_default(),
+        })
    }
 }

+impl generics::ViewFunction<LengthPercentage> {
+    /// Parse the inner function arguments of `view()`.
+    fn parse_arguments<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        // <view()> = view( [ <axis> || <'view-timeline-inset'> ]? )
+        // https://drafts.csswg.org/scroll-animations-1/#funcdef-view
+        let mut axis = None;
+        let mut inset = None;
+        loop {
+            if axis.is_none() {
+                axis = input.try_parse(ScrollAxis::parse).ok();
+            }
+
+            if inset.is_none() {
+                inset = input
+                    .try_parse(|i| ViewTimelineInset::parse(context, i))
+                    .ok();
+                if inset.is_some() {
+                    continue;
+                }
+            }
+            break;
+        }
+
+        Ok(Self {
+            inset: inset.unwrap_or_default(),
+            axis: axis.unwrap_or_default(),
+        })
+    }
+}
+
+/// A specified value for the `animation-timeline` property.
+pub type AnimationTimeline = generics::GenericAnimationTimeline<LengthPercentage>;
+
 impl Parse for AnimationTimeline {
    fn parse<'i, 't>(
        context: &ParserContext,
        input: &mut Parser<'i, 't>,
    ) -> Result<Self, ParseError<'i>> {
+        use crate::values::generics::animation::ViewFunction;
+
+        // <single-animation-timeline> = auto | none | <custom-ident> | <scroll()> | <view()>
+        // https://drafts.csswg.org/css-animations-2/#typedef-single-animation-timeline
+
        if input.try_parse(|i| i.expect_ident_matching("auto")).is_ok() {
            return Ok(Self::Auto);
        }
@ -314,20 +359,24 @@ impl Parse for AnimationTimeline {
            return Ok(AnimationTimeline::Timeline(TimelineName::none()));
        }

-        // https://drafts.csswg.org/scroll-animations-1/#scroll-notation
-        if input
-            .try_parse(|i| i.expect_function_matching("scroll"))
-            .is_ok()
-        {
-            return input.parse_nested_block(|i| {
-                Ok(Self::Scroll(
-                    i.try_parse(ScrollAxis::parse).unwrap_or(ScrollAxis::Block),
-                    i.try_parse(Scroller::parse).unwrap_or(Scroller::Nearest),
-                ))
-            });
+        if let Ok(name) = input.try_parse(|i| TimelineName::parse(context, i)) {
+            return Ok(AnimationTimeline::Timeline(name));
        }

-        TimelineName::parse(context, input).map(AnimationTimeline::Timeline)
+        // Parse possible functions
+        let location = input.current_source_location();
+        let function = input.expect_function()?.clone();
+        input.parse_nested_block(move |i| {
+            match_ignore_ascii_case! { &function,
+                "scroll" => ScrollFunction::parse_arguments(i).map(Self::Scroll),
+                "view" => ViewFunction::parse_arguments(context, i).map(Self::View),
+                _ => {
+                    Err(location.new_custom_error(
+                        StyleParseErrorKind::UnexpectedFunction(function.clone())
+                    ))
+                },
+            }
+        })
    }
 }

--- a/testing/web-platform/meta/scroll-animations/css/animation-shorthand.html.ini
+++ b/testing/web-platform/meta/scroll-animations/css/animation-shorthand.html.ini
@ -1,9 +1,15 @@
 [animation-shorthand.html]
  [e.style['animation'\] = "1s linear 1s 2 reverse forwards paused anim scroll()" should not set the property value]
    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1824261
+
+  [e.style['animation'\] = "1s linear 1s 2 reverse forwards paused anim view()" should not set the property value]
+    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1824261

  [e.style['animation'\] = "1s linear 1s 2 reverse forwards paused anim timeline" should not set the property value]
    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1824261

  [e.style['animation'\] = "1s linear 1s 2 reverse forwards paused anim1,\\n   1s linear 1s 2 reverse forwards paused anim2,\\n   1s linear 1s 2 reverse forwards paused anim3" should set animation-range-end]
    expected: FAIL
--- a/testing/web-platform/meta/scroll-animations/css/animation-timeline-computed.html.ini
+++ b/testing/web-platform/meta/scroll-animations/css/animation-timeline-computed.html.ini
@ -1,40 +1,4 @@
 [animation-timeline-computed.html]
-  [Property animation-timeline value 'view()']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(block)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(inline)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(horizontal)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(vertical)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(vertical 1px)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(1px auto)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(auto 1px)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(vertical 1px auto)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(1px vertical)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(vertical auto)']
-    expected: FAIL
-
-  [Property animation-timeline value 'view(vertical auto auto)']
-    expected: FAIL
-
  [Property animation-timeline value 'scroll(self)']
    expected: FAIL

--- a/testing/web-platform/meta/scroll-animations/css/animation-timeline-parsing.html.ini
+++ b/testing/web-platform/meta/scroll-animations/css/animation-timeline-parsing.html.ini
@ -1,66 +1,4 @@
 [animation-timeline-parsing.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [e.style['animation-timeline'\] = "view()" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(block)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(inline)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(horizontal)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(vertical)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(vertical 1px 2px)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(vertical 1px)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(vertical auto)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(vertical auto auto)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(vertical auto 1px)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(1px 2px vertical)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(1px vertical)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(auto horizontal)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(1px 2px)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(1px)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(1px 1px)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(1px auto)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(auto calc(1% + 1px))" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(auto)" should set the property value]
-    expected: FAIL
-
-  [e.style['animation-timeline'\] = "view(auto auto)" should set the property value]
-    expected: FAIL
-
  [e.style['animation-timeline'\] = "\\"test\\"" should not set the property value]
    expected: FAIL