[librsvg: 1/3] (#261): Parse the complete enable-background property




commit ea0c5b1912882ef882f0614c04b0311438591516
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Mar 25 16:59:21 2021 -0600

    (#261): Parse the complete enable-background property
    
    We just parse it so that elements will not be put in an error state if
    they have enable-background="new 1 2 3 4", even though we ignore the
    property altogether.
    
    This finally removes the hack in set_presentation_attributes() to
    ignore errors; instead we let its caller call set_error() as intended.
    
    https://gitlab.gnome.org/GNOME/librsvg/-/issues/261
    
    Also fixes https://gitlab.gnome.org/GNOME/librsvg/-/issues/321

 src/element.rs       | 21 +-----------------
 src/property_defs.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 24 deletions(-)
---
diff --git a/src/element.rs b/src/element.rs
index ad3c36a3..00c9f1ed 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -227,27 +227,8 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
     /// Hands the `attrs` to the node's state, to apply the presentation attributes.
     #[allow(clippy::unnecessary_wraps)]
     fn set_presentation_attributes(&mut self) -> Result<(), ElementError> {
-        match self
-            .specified_values
+        self.specified_values
             .parse_presentation_attributes(&self.attributes)
-        {
-            Ok(_) => Ok(()),
-            Err(e) => {
-                // FIXME: we'll ignore errors here for now.
-                //
-                // If we set the node to be in error, we expose buggy handling of the
-                // enable-background property; we are not parsing it correctly. This
-                // causes tests/fixtures/reftests/bugs/587721-text-transform.svg to fail
-                // because it has enable-background="new 0 0 1179.75118 687.74173" in the
-                // toplevel svg element.
-                //
-                //   self.set_error(e);
-                //   return;
-
-                rsvg_log!("(attribute error: {})", e);
-                Ok(())
-            }
-        }
     }
 
     // Applies a style declaration to the node's specified_values
diff --git a/src/property_defs.rs b/src/property_defs.rs
index 7a273d3a..a914e11a 100644
--- a/src/property_defs.rs
+++ b/src/property_defs.rs
@@ -55,6 +55,7 @@ use crate::paint_server::PaintServer;
 use crate::parsers::Parse;
 use crate::properties::ComputedValues;
 use crate::property_macros::Property;
+use crate::rect::Rect;
 use crate::unit_interval::UnitInterval;
 
 // https://www.w3.org/TR/SVG/text.html#BaselineShiftProperty
@@ -205,18 +206,72 @@ make_property!(
     "none" => None,
 );
 
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum EnableBackground {
+    Accumulate,
+    New(Option<Rect>),
+}
+
 // https://www.w3.org/TR/SVG/filters.html#EnableBackgroundProperty
 make_property!(
     ComputedValues,
     EnableBackground,
-    default: Accumulate,
+    default: EnableBackground::Accumulate,
     inherits_automatically: false,
 
-    identifiers:
-    "accumulate" => Accumulate,
-    "new" => New,
+    parse_impl: {
+        impl Parse for EnableBackground {
+            fn parse<'i>(parser: &mut Parser<'i, '_>) -> Result<Self, crate::error::ParseError<'i>> {
+                let loc = parser.current_source_location();
+
+                if parser
+                    .try_parse(|p| p.expect_ident_matching("accumulate"))
+                    .is_ok()
+                {
+                    return Ok(EnableBackground::Accumulate);
+                }
+
+                if parser.try_parse(|p| p.expect_ident_matching("new")).is_ok() {
+                    parser.try_parse(|p| -> Result<_, ParseError<'_>> {
+                        let x = f64::parse(p)?;
+                        let y = f64::parse(p)?;
+                        let w = f64::parse(p)?;
+                        let h = f64::parse(p)?;
+
+                        Ok(EnableBackground::New(Some(Rect::new(x, y, x + w, y + h))))
+                    }).or(Ok(EnableBackground::New(None)))
+                } else {
+                    Err(loc.new_custom_error(ValueErrorKind::parse_error("invalid syntax for 
'enable-background' property")))
+                }
+            }
+        }
+
+    }
 );
 
+#[cfg(test)]
+#[test]
+fn parses_enable_background() {
+    assert_eq!(
+        EnableBackground::parse_str("accumulate").unwrap(),
+        EnableBackground::Accumulate
+    );
+
+    assert_eq!(
+        EnableBackground::parse_str("new").unwrap(),
+        EnableBackground::New(None)
+    );
+
+    assert_eq!(
+        EnableBackground::parse_str("new 1 2 3 4").unwrap(),
+        EnableBackground::New(Some(Rect::new(1.0, 2.0, 4.0, 6.0)))
+    );
+
+    assert!(EnableBackground::parse_str("new foo").is_err());
+
+    assert!(EnableBackground::parse_str("plonk").is_err());
+}
+
 // https://www.w3.org/TR/SVG/painting.html#FillProperty
 make_property!(
     ComputedValues,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]