Merge branch 'attribute-parsers-737' into 'main'

#737 - Treat attribute parsing errors as per the spec

Closes #737 and #913

See merge request GNOME/librsvg!763

37 files changed, 981 insertions, 760 deletions

diff --git a/src/cond.rs b/src/cond.rs
index 0a147bec..04c01042 100644
--- a/src/cond.rs
+++ b/src/cond.rs
@@ -9,6 +9,7 @@ use language_tags::LanguageTag;
 
 use crate::accept_language::{LanguageTags, UserLanguage};
 use crate::error::*;
+use crate::session::Session;
 
 // No extensions at the moment.
 static IMPLEMENTED_EXTENSIONS: &[&str] = &[];
@@ -20,11 +21,11 @@ impl RequiredExtensions {
     /// Parse a requiredExtensions attribute.
     ///
     /// <http://www.w3.org/TR/SVG/struct.html#RequiredExtensionsAttribute>
-    pub fn from_attribute(s: &str) -> Result<RequiredExtensions, ValueErrorKind> {
-        Ok(RequiredExtensions(
+    pub fn from_attribute(s: &str) -> RequiredExtensions {
+        RequiredExtensions(
             s.split_whitespace()
                 .all(|f| IMPLEMENTED_EXTENSIONS.binary_search(&f).is_ok()),
-        ))
+        )
     }
 
     /// Evaluate a requiredExtensions value for conditional processing.
@@ -64,11 +65,11 @@ pub struct RequiredFeatures(pub bool);
 impl RequiredFeatures {
     // Parse a requiredFeatures attribute
     // http://www.w3.org/TR/SVG/struct.html#RequiredFeaturesAttribute
-    pub fn from_attribute(s: &str) -> Result<RequiredFeatures, ValueErrorKind> {
-        Ok(RequiredFeatures(
+    pub fn from_attribute(s: &str) -> RequiredFeatures {
+        RequiredFeatures(
             s.split_whitespace()
                 .all(|f| IMPLEMENTED_FEATURES.binary_search(&f).is_ok()),
-        ))
+        )
     }
 
     /// Evaluate a requiredFeatures value for conditional processing.
@@ -77,8 +78,18 @@ impl RequiredFeatures {
     }
 }
 
+/// The systemLanguage attribute inside `<cond>` element's children.
+///
+/// Parsing the value of a `systemLanguage` attribute may fail if the document supplies
+/// invalid BCP47 language tags.  In that case, we store an `Invalid` variant.
+///
+/// That variant is used later, during [`SystemLanguage::eval`], to see whether the
+/// `<cond>` should match or not.
 #[derive(Debug, PartialEq)]
-pub struct SystemLanguage(LanguageTags);
+pub enum SystemLanguage {
+    Valid(LanguageTags),
+    Invalid,
+}
 
 impl SystemLanguage {
     /// Parse a `systemLanguage` attribute and match it against a given `Locale`
@@ -96,7 +107,7 @@ impl SystemLanguage {
     ///
     /// [`systemLanguage`]: https://www.w3.org/TR/SVG/struct.html#ConditionalProcessingSystemLanguageAttribute
     /// [BCP47]: http://www.ietf.org/rfc/bcp/bcp47.txt
-    pub fn from_attribute(s: &str) -> Result<SystemLanguage, ValueErrorKind> {
+    pub fn from_attribute(s: &str, session: &Session) -> SystemLanguage {
         let attribute_tags = s
             .split(',')
             .map(str::trim)
@@ -105,14 +116,28 @@ impl SystemLanguage {
                     ValueErrorKind::parse_error(&format!("invalid language tag: \"{}\"", e))
                 })
             })
-            .collect::<Result<Vec<LanguageTag>, _>>()?;
-
-        Ok(SystemLanguage(LanguageTags::from(attribute_tags)))
+            .collect::<Result<Vec<LanguageTag>, _>>();
+
+        match attribute_tags {
+            Ok(tags) => SystemLanguage::Valid(LanguageTags::from(tags)),
+
+            Err(e) => {
+                rsvg_log!(
+                    session,
+                    "ignoring systemLanguage attribute with invalid value: {}",
+                    e
+                );
+                SystemLanguage::Invalid
+            }
+        }
     }
 
     /// Evaluate a systemLanguage value for conditional processing.
     pub fn eval(&self, user_language: &UserLanguage) -> bool {
-        user_language.any_matches(&self.0)
+        match *self {
+            SystemLanguage::Valid(ref tags) => user_language.any_matches(tags),
+            SystemLanguage::Invalid => false,
+        }
     }
 }
 
@@ -124,7 +149,7 @@ mod tests {
     #[test]
     fn required_extensions() {
         assert_eq!(
-            RequiredExtensions::from_attribute("http://test.org/NotExisting/1.0").unwrap(),
+            RequiredExtensions::from_attribute("http://test.org/NotExisting/1.0"),
             RequiredExtensions(false)
         );
     }
@@ -132,14 +157,12 @@ mod tests {
     #[test]
     fn required_features() {
         assert_eq!(
-            RequiredFeatures::from_attribute("http://www.w3.org/TR/SVG11/feature#NotExisting")
-                .unwrap(),
+            RequiredFeatures::from_attribute("http://www.w3.org/TR/SVG11/feature#NotExisting"),
             RequiredFeatures(false)
         );
 
         assert_eq!(
-            RequiredFeatures::from_attribute("http://www.w3.org/TR/SVG11/feature#BasicFilter")
-                .unwrap(),
+            RequiredFeatures::from_attribute("http://www.w3.org/TR/SVG11/feature#BasicFilter"),
             RequiredFeatures(true)
         );
 
@@ -147,8 +170,7 @@ mod tests {
             RequiredFeatures::from_attribute(
                 "http://www.w3.org/TR/SVG11/feature#BasicFilter \
                  http://www.w3.org/TR/SVG11/feature#NotExisting",
-            )
-            .unwrap(),
+            ),
             RequiredFeatures(false)
         );
 
@@ -156,74 +178,65 @@ mod tests {
             RequiredFeatures::from_attribute(
                 "http://www.w3.org/TR/SVG11/feature#BasicFilter \
                  http://www.w3.org/TR/SVG11/feature#BasicText",
-            )
-            .unwrap(),
+            ),
             RequiredFeatures(true)
         );
     }
 
     #[test]
     fn system_language() {
+        let session = Session::new_for_test_suite();
+
         let locale = Locale::new("de,en-US").unwrap();
         let user_language = UserLanguage::LanguageTags(LanguageTags::from_locale(&locale).unwrap());
 
-        assert!(SystemLanguage::from_attribute("").is_err());
+        assert!(matches!(
+            SystemLanguage::from_attribute("", &session),
+            SystemLanguage::Invalid
+        ));
 
-        assert!(SystemLanguage::from_attribute("12345").is_err());
+        assert!(matches!(
+            SystemLanguage::from_attribute("12345", &session),
+            SystemLanguage::Invalid
+        ));
 
         assert_eq!(
-            SystemLanguage::from_attribute("fr")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("fr", &session).eval(&user_language),
             false
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("en")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("en", &session).eval(&user_language),
             false
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("de")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("de", &session).eval(&user_language),
             true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("en-US")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("en-US", &session).eval(&user_language),
             true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("en-GB")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("en-GB", &session).eval(&user_language),
             false
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("DE")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("DE", &session).eval(&user_language),
             true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("de-LU")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("de-LU", &session).eval(&user_language),
             true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("fr, de")
-                .unwrap()
-                .eval(&user_language),
+            SystemLanguage::from_attribute("fr, de", &session).eval(&user_language),
             true
         );
     }
diff --git a/src/element.rs b/src/element.rs
index 3d8fb60b..bf5c1a30 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -45,34 +45,37 @@ use crate::style::Style;
 use crate::text::{TRef, TSpan, Text};
 use crate::xml::Attributes;
 
-// After creating/parsing a Element, it will be in a success or an error state.
-// We represent this with a Result, aliased as a ElementResult.  There is no
-// extra information for the Ok case; all the interesting stuff is in the
-// Err case.
-//
-// https://www.w3.org/TR/SVG/implnote.html#ErrorProcessing
-//
-// When an element has an error during parsing, the SVG spec calls the element
-// to be "in error".  We skip rendering of elements that are in error.
-//
-// When we parse an element's attributes, we stop as soon as we
-// encounter the first error:  a parse error, or an invalid value,
-// etc.  No further attributes will be processed, although note that
-// the order in which an element's attributes are processed is not
-// defined.
-//
-// Alternatively, we could try to parse/validate all the attributes
-// that come in an element and build up a Vec<ElementError>.  However, we
-// don't do this now.  Doing that may be more useful for an SVG
-// validator, not a renderer like librsvg is.
-pub type ElementResult = Result<(), ElementError>;
-
 pub trait SetAttributes {
     /// Sets per-element attributes.
     ///
     /// Each element is supposed to iterate the `attributes`, and parse any ones it needs.
-    fn set_attributes(&mut self, _attributes: &Attributes, _session: &Session) -> ElementResult {
-        Ok(())
+    /// SVG specifies that unknown attributes should be ignored, and known attributes with invalid
+    /// values should be ignored so that the attribute ends up with its "initial value".
+    ///
+    /// You can use the [`set_attribute`] function to do that.
+    fn set_attributes(&mut self, _attributes: &Attributes, _session: &Session) {}
+}
+
+/// Sets `dest` if `parse_result` is `Ok()`, otherwise just logs the error.
+///
+/// Implementations of the [`SetAttributes`] trait generally scan a list of attributes
+/// for the ones they can handle, and parse their string values.  Per the SVG spec, an attribute
+/// with an invalid value should be ignored, and it should fall back to the default value.
+///
+/// In librsvg, those default values are set in each element's implementation of the [`Default`] trait:
+/// at element creation time, each element gets initialized to its `Default`, and then each attribute
+/// gets parsed.  This function will set that attribute's value only if parsing was successful.
+///
+/// In case the `parse_result` is an error, this function will log an appropriate notice
+/// via the [`Session`].
+pub fn set_attribute<T>(dest: &mut T, parse_result: Result<T, ElementError>, session: &Session) {
+    match parse_result {
+        Ok(v) => *dest = v,
+        Err(e) => {
+            // FIXME: this does not provide a clue of what was the problematic element.
+            // We need tracking of the current parsing position to do that.
+            rsvg_log!(session, "ignoring attribute with invalid value: {}", e);
+        }
     }
 }
 
@@ -98,7 +101,6 @@ pub struct ElementInner<T: SetAttributes + Draw> {
     attributes: Attributes,
     specified_values: SpecifiedValues,
     important_styles: HashSet<QualName>,
-    is_in_error: bool,
     values: ComputedValues,
     required_extensions: Option<RequiredExtensions>,
     required_features: Option<RequiredFeatures>,
@@ -111,7 +113,6 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
         session: &Session,
         element_name: QualName,
         attributes: Attributes,
-        is_in_error: bool,
         element_impl: T,
     ) -> ElementInner<T> {
         let mut e = Self {
@@ -119,7 +120,6 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
             attributes,
             specified_values: Default::default(),
             important_styles: Default::default(),
-            is_in_error,
             values: Default::default(),
             required_extensions: Default::default(),
             required_features: Default::default(),
@@ -127,15 +127,8 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
             element_impl,
         };
 
-        let mut set_attributes = || -> Result<(), ElementError> {
-            e.set_conditional_processing_attributes()?;
-            e.set_presentation_attributes(session)?;
-            Ok(())
-        };
-
-        if let Err(error) = set_attributes() {
-            e.set_error(error, session);
-        }
+        e.set_conditional_processing_attributes(session);
+        e.set_presentation_attributes(session);
 
         e
     }
@@ -190,36 +183,30 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
                 .unwrap_or(true)
     }
 
-    fn set_conditional_processing_attributes(&mut self) -> Result<(), ElementError> {
+    fn set_conditional_processing_attributes(&mut self, session: &Session) {
         for (attr, value) in self.attributes.iter() {
             match attr.expanded() {
                 expanded_name!("", "requiredExtensions") => {
-                    self.required_extensions =
-                        Some(RequiredExtensions::from_attribute(value).attribute(attr)?);
+                    self.required_extensions = Some(RequiredExtensions::from_attribute(value));
                 }
 
                 expanded_name!("", "requiredFeatures") => {
-                    self.required_features =
-                        Some(RequiredFeatures::from_attribute(value).attribute(attr)?);
+                    self.required_features = Some(RequiredFeatures::from_attribute(value));
                 }
 
                 expanded_name!("", "systemLanguage") => {
-                    self.system_language =
-                        Some(SystemLanguage::from_attribute(value).attribute(attr)?);
+                    self.system_language = Some(SystemLanguage::from_attribute(value, session));
                 }
 
                 _ => {}
             }
         }
-
-        Ok(())
     }
 
     /// Hands the `attrs` to the node's state, to apply the presentation attributes.
-    #[allow(clippy::unnecessary_wraps)]
-    fn set_presentation_attributes(&mut self, session: &Session) -> Result<(), ElementError> {
+    fn set_presentation_attributes(&mut self, session: &Session) {
         self.specified_values
-            .parse_presentation_attributes(session, &self.attributes)
+            .parse_presentation_attributes(session, &self.attributes);
     }
 
     // Applies a style declaration to the node's specified_values
@@ -240,25 +227,14 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
             .map(|(_, value)| value);
 
         if let Some(style) = style {
-            if let Err(e) = self.specified_values.parse_style_declarations(
+            self.specified_values.parse_style_declarations(
                 style,
                 Origin::Author,
                 &mut self.important_styles,
                 session,
-            ) {
-                self.set_error(e, session);
-            }
+            );
         }
     }
-
-    fn set_error(&mut self, error: ElementError, session: &Session) {
-        rsvg_log!(session, "setting node {} in error: {}", self, error);
-        self.is_in_error = true;
-    }
-
-    fn is_in_error(&self) -> bool {
-        self.is_in_error
-    }
 }
 
 impl<T: SetAttributes + Draw> Draw for ElementInner<T> {
@@ -270,22 +246,11 @@ impl<T: SetAttributes + Draw> Draw for ElementInner<T> {
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
     ) -> Result<BoundingBox, RenderingError> {
-        if !self.is_in_error() {
-            let values = cascaded.get();
-            if values.is_displayed() {
-                self.element_impl
-                    .draw(node, acquired_nodes, cascaded, draw_ctx, clipping)
-            } else {
-                Ok(draw_ctx.empty_bbox())
-            }
+        let values = cascaded.get();
+        if values.is_displayed() {
+            self.element_impl
+                .draw(node, acquired_nodes, cascaded, draw_ctx, clipping)
         } else {
-            rsvg_log!(
-                draw_ctx.session(),
-                "(not rendering element {} because it is in error)",
-                self
-            );
-
-            // maybe we should actually return a RenderingError::ElementIsInError here?
             Ok(draw_ctx.empty_bbox())
         }
     }
@@ -520,10 +485,6 @@ impl Element {
         call_inner!(self, set_style_attribute, session);
     }
 
-    pub fn is_in_error(&self) -> bool {
-        call_inner!(self, is_in_error)
-    }
-
     pub fn as_filter_effect(&self) -> Option<&dyn FilterEffect> {
         match self {
             Element::FeBlend(ref fe) => Some(&fe.element_impl),
@@ -598,21 +559,12 @@ macro_rules! e {
             attributes: Attributes,
         ) -> Element {
             let mut element_impl = <$element_type>::default();
-
-            let is_in_error = if let Err(e) = element_impl.set_attributes(&attributes, session) {
-                // FIXME: this does not provide a clue of what was the problematic attribute, or the
-                // problematic element.  We need tracking of the current parsing position to do that.
-                rsvg_log!(session, "setting element in error: {}", e);
-                true
-            } else {
-                false
-            };
+            element_impl.set_attributes(&attributes, session);
 
             let element = Element::$element_type(Box::new(ElementInner::new(
                 session,
                 element_name.clone(),
                 attributes,
-                is_in_error,
                 element_impl,
             )));
 
diff --git a/src/filter.rs b/src/filter.rs
index 963b963c..259a5b18 100644
--- a/src/filter.rs
+++ b/src/filter.rs
@@ -7,7 +7,7 @@ use std::slice::Iter;
 use crate::coord_units::CoordUnits;
 use crate::document::{AcquiredNodes, NodeId};
 use crate::drawing_ctx::{DrawingCtx, ViewParams};
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::error::ValueErrorKind;
 use crate::filter_func::FilterFunction;
 use crate::filters::{FilterResolveError, FilterSpec, UserSpacePrimitive};
@@ -71,20 +71,26 @@ impl Filter {
 }
 
 impl SetAttributes for Filter {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "filterUnits") => self.filter_units = attr.parse(value)?,
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "width") => self.width = attr.parse(value)?,
-                expanded_name!("", "height") => self.height = attr.parse(value)?,
-                expanded_name!("", "primitiveUnits") => self.primitive_units = attr.parse(value)?,
+                expanded_name!("", "filterUnits") => {
+                    set_attribute(&mut self.filter_units, attr.parse(value), session)
+                }
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "width") => {
+                    set_attribute(&mut self.width, attr.parse(value), session)
+                }
+                expanded_name!("", "height") => {
+                    set_attribute(&mut self.height, attr.parse(value), session)
+                }
+                expanded_name!("", "primitiveUnits") => {
+                    set_attribute(&mut self.primitive_units, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -176,20 +182,6 @@ fn extract_filter_from_filter_node(
     let primitives = filter_node
         .children()
         .filter(|c| c.is_element())
-        // Skip nodes in error.
-        .filter(|c| {
-            let in_error = c.borrow_element().is_in_error();
-
-            if in_error {
-                rsvg_log!(
-                    session,
-                    "(ignoring filter primitive {} because it is in error)",
-                    c
-                );
-            }
-
-            !in_error
-        })
         // Keep only filter primitives (those that implement the Filter trait)
         .filter(|c| c.borrow_element().as_filter_effect().is_some())
         .map(|primitive_node| {
@@ -249,24 +241,12 @@ fn filter_spec_from_filter_node(
             let element = node.borrow_element();
 
             match *element {
-                Element::Filter(_) => {
-                    if element.is_in_error() {
-                        rsvg_log!(
-                            session,
-                            "element {} will not be filtered since its filter \"{}\" is in error",
-                            node_being_filtered_name,
-                            node_id,
-                        );
-                        Err(FilterResolveError::ChildNodeInError)
-                    } else {
-                        extract_filter_from_filter_node(
-                            node,
-                            acquired_nodes,
-                            &session,
-                            &filter_view_params,
-                        )
-                    }
-                }
+                Element::Filter(_) => extract_filter_from_filter_node(
+                    node,
+                    acquired_nodes,
+                    &session,
+                    &filter_view_params,
+                ),
 
                 _ => {
                     rsvg_log!(
diff --git a/src/filter_func.rs b/src/filter_func.rs
index 26dcd810..b01fe2f4 100644
--- a/src/filter_func.rs
+++ b/src/filter_func.rs
@@ -28,7 +28,7 @@ use crate::filters::{
 };
 use crate::length::*;
 use crate::paint_server::resolve_color;
-use crate::parsers::{CustomIdent, NumberOrPercentage, Parse};
+use crate::parsers::{CustomIdent, NumberOptionalNumber, NumberOrPercentage, Parse};
 use crate::unit_interval::UnitInterval;
 
 /// CSS Filter functions from the Filter Effects Module Level 1
@@ -271,7 +271,7 @@ impl Blur {
         let gaussian_blur = ResolvedPrimitive {
             primitive: Primitive::default(),
             params: PrimitiveParams::GaussianBlur(GaussianBlur {
-                std_deviation: (std_dev, std_dev),
+                std_deviation: NumberOptionalNumber(std_dev, std_dev),
                 ..GaussianBlur::default()
             }),
         }
@@ -385,7 +385,7 @@ impl DropShadow {
             primitive: Primitive::default(),
             params: PrimitiveParams::GaussianBlur(GaussianBlur {
                 in1: Input::SourceAlpha,
-                std_deviation: (std_dev, std_dev),
+                std_deviation: NumberOptionalNumber(std_dev, std_dev),
                 ..GaussianBlur::default()
             }),
         }
diff --git a/src/filters/blend.rs b/src/filters/blend.rs
index 2928fdfa..4baf4476 100644
--- a/src/filters/blend.rs
+++ b/src/filters/blend.rs
@@ -3,7 +3,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node};
 use crate::parsers::{Parse, ParseValue};
@@ -60,18 +60,16 @@ pub struct Blend {
 }
 
 impl SetAttributes for FeBlend {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        let (in1, in2) = self.base.parse_two_inputs(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        let (in1, in2) = self.base.parse_two_inputs(attrs, session);
         self.params.in1 = in1;
         self.params.in2 = in2;
 
         for (attr, value) in attrs.iter() {
             if let expanded_name!("", "mode") = attr.expanded() {
-                self.params.mode = attr.parse(value)?;
+                set_attribute(&mut self.params.mode, attr.parse(value), session);
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/filters/color_matrix.rs b/src/filters/color_matrix.rs
index c3c2b538..7bb3689f 100644
--- a/src/filters/color_matrix.rs
+++ b/src/filters/color_matrix.rs
@@ -1,10 +1,10 @@
 use cssparser::Parser;
-use markup5ever::{expanded_name, local_name, namespace_url, ns};
+use markup5ever::{expanded_name, local_name, namespace_url, ns, QualName};
 use nalgebra::{Matrix3, Matrix4x5, Matrix5, Vector5};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node};
 use crate::parsers::{NumberList, Parse, ParseValue};
@@ -62,10 +62,9 @@ impl Default for ColorMatrix {
     }
 }
 
-#[rustfmt::skip]
 impl SetAttributes for FeColorMatrix {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         // First, determine the operation type.
         let mut operation_type = Default::default();
@@ -73,60 +72,110 @@ impl SetAttributes for FeColorMatrix {
             .iter()
             .filter(|(attr, _)| attr.expanded() == expanded_name!("", "type"))
         {
-            operation_type = attr.parse(value)?;
+            set_attribute(&mut operation_type, attr.parse(value), session);
         }
 
         // Now read the matrix correspondingly.
-        // LuminanceToAlpha doesn't accept any matrix.
-        if operation_type == OperationType::LuminanceToAlpha {
-            self.params.matrix = {
-                Matrix5::new(
-                    0.0,    0.0,    0.0,    0.0, 0.0,
-                    0.0,    0.0,    0.0,    0.0, 0.0,
-                    0.0,    0.0,    0.0,    0.0, 0.0,
-                    0.2125, 0.7154, 0.0721, 0.0, 0.0,
-                    0.0,    0.0,    0.0,    0.0, 1.0,
-                )
-            };
-        } else {
-            for (attr, value) in attrs
-                .iter()
-                .filter(|(attr, _)| attr.expanded() == expanded_name!("", "values"))
-            {
-                let new_matrix = match operation_type {
-                    OperationType::LuminanceToAlpha => unreachable!(),
-                    OperationType::Matrix => {
-                        let NumberList::<20, 20>(v) = attr.parse(value)?;
-                        let matrix = Matrix4x5::from_row_slice(&v);
-                        let mut matrix = matrix.fixed_resize(0.0);
-                        matrix[(4, 4)] = 1.0;
-                        matrix
-                    }
-                    OperationType::Saturate => {
-                        let s: f64 = attr.parse(value)?;
-
-                        Matrix5::new(
-                            0.213 + 0.787 * s, 0.715 - 0.715 * s, 0.072 - 0.072 * s, 0.0, 0.0,
-                            0.213 - 0.213 * s, 0.715 + 0.285 * s, 0.072 - 0.072 * s, 0.0, 0.0,
-                            0.213 - 0.213 * s, 0.715 - 0.715 * s, 0.072 + 0.928 * s, 0.0, 0.0,
-                            0.0,               0.0,               0.0,               1.0, 0.0,
-                            0.0,               0.0,               0.0,               0.0, 1.0,
-                        )
-                    }
-                    OperationType::HueRotate => {
-                        let degrees: f64 = attr.parse(value)?;
-                        ColorMatrix::hue_rotate_matrix(degrees.to_radians())
-                    }
-                };
+        //
+        // Here we cannot assume that ColorMatrix::default() has provided the correct
+        // initial value for the matrix itself, since the initial value for the matrix
+        // (i.e. the value to which it should fall back if the `values` attribute is in
+        // error) depends on the operation_type.
+        //
+        // So, for each operation_type, first initialize the proper default matrix, then
+        // try to parse the value.
+
+        use OperationType::*;
+
+        self.params.matrix = match operation_type {
+            Matrix => ColorMatrix::default_matrix(),
+            Saturate => ColorMatrix::saturate_matrix(1.0),
+            HueRotate => ColorMatrix::hue_rotate_matrix(0.0),
+            LuminanceToAlpha => ColorMatrix::luminance_to_alpha_matrix(),
+        };
 
-                self.params.matrix = new_matrix;
+        for (attr, value) in attrs
+            .iter()
+            .filter(|(attr, _)| attr.expanded() == expanded_name!("", "values"))
+        {
+            match operation_type {
+                Matrix => parse_matrix(&mut self.params.matrix, attr, value, session),
+                Saturate => parse_saturate_matrix(&mut self.params.matrix, attr, value, session),
+                HueRotate => parse_hue_rotate_matrix(&mut self.params.matrix, attr, value, session),
+                LuminanceToAlpha => {
+                    parse_luminance_to_alpha_matrix(&mut self.params.matrix, attr, value, session)
+                }
             }
         }
+    }
+}
+
+fn parse_matrix(dest: &mut Matrix5<f64>, attr: QualName, value: &str, session: &Session) {
+    let parsed: Result<NumberList<20, 20>, _> = attr.parse(value);
+
+    match parsed {
+        Ok(NumberList(v)) => {
+            let matrix = Matrix4x5::from_row_slice(&v);
+            let mut matrix = matrix.fixed_resize(0.0);
+            matrix[(4, 4)] = 1.0;
+            *dest = matrix;
+        }
 
-        Ok(())
+        Err(e) => {
+            rsvg_log!(session, "element feColorMatrix with type=\"matrix\", expected a values attribute with 20 numbers: {}", e);
+        }
     }
 }
 
+fn parse_saturate_matrix(dest: &mut Matrix5<f64>, attr: QualName, value: &str, session: &Session) {
+    let parsed: Result<f64, _> = attr.parse(value);
+
+    match parsed {
+        Ok(s) => {
+            *dest = ColorMatrix::saturate_matrix(s);
+        }
+
+        Err(e) => {
+            rsvg_log!(session, "element feColorMatrix with type=\"saturate\", expected a values attribute with 1 number: {}", e);
+        }
+    }
+}
+
+fn parse_hue_rotate_matrix(
+    dest: &mut Matrix5<f64>,
+    attr: QualName,
+    value: &str,
+    session: &Session,
+) {
+    let parsed: Result<f64, _> = attr.parse(value);
+
+    match parsed {
+        Ok(degrees) => {
+            *dest = ColorMatrix::hue_rotate_matrix(degrees.to_radians());
+        }
+
+        Err(e) => {
+            rsvg_log!(session, "element feColorMatrix with type=\"hueRotate\", expected a values attribute with 1 number: {}", e);
+        }
+    }
+}
+
+fn parse_luminance_to_alpha_matrix(
+    _dest: &mut Matrix5<f64>,
+    _attr: QualName,
+    _value: &str,
+    session: &Session,
+) {
+    // There's nothing to parse, since our caller already supplied the default value,
+    // and type="luminanceToAlpha" does not takes a `values` attribute.  So, just warn
+    // that the value is being ignored.
+
+    rsvg_log!(
+        session,
+        "ignoring \"values\" attribute for feColorMatrix with type=\"luminanceToAlpha\""
+    );
+}
+
 impl ColorMatrix {
     pub fn render(
         &self,
@@ -192,19 +241,29 @@ impl ColorMatrix {
         })
     }
 
+    /// Compute a `type="hueRotate"` matrix.
+    ///
+    /// https://drafts.fxtf.org/filter-effects/#element-attrdef-fecolormatrix-values
+    #[rustfmt::skip]
     pub fn hue_rotate_matrix(radians: f64) -> Matrix5<f64> {
         let (sin, cos) = radians.sin_cos();
 
         let a = Matrix3::new(
-            0.213, 0.715, 0.072, 0.213, 0.715, 0.072, 0.213, 0.715, 0.072,
+            0.213, 0.715, 0.072,
+            0.213, 0.715, 0.072,
+            0.213, 0.715, 0.072,
         );
 
         let b = Matrix3::new(
-            0.787, -0.715, -0.072, -0.213, 0.285, -0.072, -0.213, -0.715, 0.928,
+             0.787, -0.715, -0.072,
+            -0.213,  0.285, -0.072,
+            -0.213, -0.715,  0.928,
         );
 
         let c = Matrix3::new(
-            -0.213, -0.715, 0.928, 0.143, 0.140, -0.283, -0.787, 0.715, 0.072,
+            -0.213, -0.715,  0.928,
+             0.143,  0.140, -0.283,
+            -0.787,  0.715,  0.072,
         );
 
         let top_left = a + b * cos + c * sin;
@@ -214,6 +273,41 @@ impl ColorMatrix {
         matrix[(4, 4)] = 1.0;
         matrix
     }
+
+    /// Compute a `type="luminanceToAlpha"` matrix.
+    ///
+    /// https://drafts.fxtf.org/filter-effects/#element-attrdef-fecolormatrix-values
+    #[rustfmt::skip]
+    fn luminance_to_alpha_matrix() -> Matrix5<f64> {
+        Matrix5::new(
+            0.0,    0.0,    0.0,    0.0, 0.0,
+            0.0,    0.0,    0.0,    0.0, 0.0,
+            0.0,    0.0,    0.0,    0.0, 0.0,
+            0.2126, 0.7152, 0.0722, 0.0, 0.0,
+            0.0,    0.0,    0.0,    0.0, 1.0,
+        )
+    }
+
+    /// Compute a `type="saturate"` matrix.
+    ///
+    /// https://drafts.fxtf.org/filter-effects/#element-attrdef-fecolormatrix-values
+    #[rustfmt::skip]
+    fn saturate_matrix(s: f64) -> Matrix5<f64> {
+        Matrix5::new(
+            0.213 + 0.787 * s, 0.715 - 0.715 * s, 0.072 - 0.072 * s, 0.0, 0.0,
+            0.213 - 0.213 * s, 0.715 + 0.285 * s, 0.072 - 0.072 * s, 0.0, 0.0,
+            0.213 - 0.213 * s, 0.715 - 0.715 * s, 0.072 + 0.928 * s, 0.0, 0.0,
+            0.0,               0.0,               0.0,               1.0, 0.0,
+            0.0,               0.0,               0.0,               0.0, 1.0,
+        )
+    }
+
+    /// Default for `type="matrix"`.
+    ///
+    /// https://drafts.fxtf.org/filter-effects/#element-attrdef-fecolormatrix-values
+    fn default_matrix() -> Matrix5<f64> {
+        Matrix5::identity()
+    }
 }
 
 impl FilterEffect for FeColorMatrix {
diff --git a/src/filters/component_transfer.rs b/src/filters/component_transfer.rs
index 31027e6c..02efa6ca 100644
--- a/src/filters/component_transfer.rs
+++ b/src/filters/component_transfer.rs
@@ -5,7 +5,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node, NodeBorrow};
 use crate::parsers::{NumberList, Parse, ParseValue};
@@ -41,9 +41,8 @@ pub struct ComponentTransfer {
 }
 
 impl SetAttributes for FeComponentTransfer {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
-        Ok(())
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
     }
 }
 
@@ -215,20 +214,33 @@ macro_rules! func_x {
 
         impl SetAttributes for $func_name {
             #[inline]
-            fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+            fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
                 for (attr, value) in attrs.iter() {
                     match attr.expanded() {
-                        expanded_name!("", "type") => self.function_type = attr.parse(value)?,
+                        expanded_name!("", "type") => {
+                            set_attribute(&mut self.function_type, attr.parse(value), session)
+                        }
                         expanded_name!("", "tableValues") => {
                             // #691: Limit list to 256 to mitigate malicious SVGs
-                            let NumberList::<0, 256>(v) = attr.parse(value)?;
-                            self.table_values = v;
+                            let mut number_list = NumberList::<0, 256>(Vec::new());
+                            set_attribute(&mut number_list, attr.parse(value), session);
+                            self.table_values = number_list.0;
+                        }
+                        expanded_name!("", "slope") => {
+                            set_attribute(&mut self.slope, attr.parse(value), session)
+                        }
+                        expanded_name!("", "intercept") => {
+                            set_attribute(&mut self.intercept, attr.parse(value), session)
+                        }
+                        expanded_name!("", "amplitude") => {
+                            set_attribute(&mut self.amplitude, attr.parse(value), session)
+                        }
+                        expanded_name!("", "exponent") => {
+                            set_attribute(&mut self.exponent, attr.parse(value), session)
+                        }
+                        expanded_name!("", "offset") => {
+                            set_attribute(&mut self.offset, attr.parse(value), session)
                         }
-                        expanded_name!("", "slope") => self.slope = attr.parse(value)?,
-                        expanded_name!("", "intercept") => self.intercept = attr.parse(value)?,
-                        expanded_name!("", "amplitude") => self.amplitude = attr.parse(value)?,
-                        expanded_name!("", "exponent") => self.exponent = attr.parse(value)?,
-                        expanded_name!("", "offset") => self.offset = attr.parse(value)?,
 
                         _ => (),
                     }
@@ -244,8 +256,6 @@ macro_rules! func_x {
                     }
                     _ => (),
                 }
-
-                Ok(())
             }
         }
 
@@ -397,15 +407,6 @@ fn get_functions(node: &Node) -> Result<Functions, FilterResolveError> {
     let func_b_node = get_func_x_node!(node, FeFuncB, Channel::B);
     let func_a_node = get_func_x_node!(node, FeFuncA, Channel::A);
 
-    for node in [&func_r_node, &func_g_node, &func_b_node, &func_a_node]
-        .iter()
-        .filter_map(|x| x.as_ref())
-    {
-        if node.borrow_element().is_in_error() {
-            return Err(FilterResolveError::ChildNodeInError);
-        }
-    }
-
     let r = func_or_default!(func_r_node, FeFuncR);
     let g = func_or_default!(func_g_node, FeFuncG);
     let b = func_or_default!(func_b_node, FeFuncB);
diff --git a/src/filters/composite.rs b/src/filters/composite.rs
index 1b690420..c05468e8 100644
--- a/src/filters/composite.rs
+++ b/src/filters/composite.rs
@@ -3,7 +3,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node};
 use crate::parsers::{Parse, ParseValue};
@@ -54,23 +54,31 @@ pub struct Composite {
 }
 
 impl SetAttributes for FeComposite {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        let (in1, in2) = self.base.parse_two_inputs(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        let (in1, in2) = self.base.parse_two_inputs(attrs, session);
         self.params.in1 = in1;
         self.params.in2 = in2;
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "operator") => self.params.operator = attr.parse(value)?,
-                expanded_name!("", "k1") => self.params.k1 = attr.parse(value)?,
-                expanded_name!("", "k2") => self.params.k2 = attr.parse(value)?,
-                expanded_name!("", "k3") => self.params.k3 = attr.parse(value)?,
-                expanded_name!("", "k4") => self.params.k4 = attr.parse(value)?,
+                expanded_name!("", "operator") => {
+                    set_attribute(&mut self.params.operator, attr.parse(value), session)
+                }
+                expanded_name!("", "k1") => {
+                    set_attribute(&mut self.params.k1, attr.parse(value), session)
+                }
+                expanded_name!("", "k2") => {
+                    set_attribute(&mut self.params.k2, attr.parse(value), session)
+                }
+                expanded_name!("", "k3") => {
+                    set_attribute(&mut self.params.k3, attr.parse(value), session)
+                }
+                expanded_name!("", "k4") => {
+                    set_attribute(&mut self.params.k4, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/filters/context.rs b/src/filters/context.rs
index 47ae32af..96babf78 100644
--- a/src/filters/context.rs
+++ b/src/filters/context.rs
@@ -280,6 +280,20 @@ impl FilterContext {
         self.effects_region
     }
 
+    /// Get a filter primitive's default input as if its `in=\"...\"` were not specified.
+    ///
+    /// Per https://drafts.fxtf.org/filter-effects/#element-attrdef-filter-primitive-in,
+    /// "References to non-existent results will be treated as if no result was
+    /// specified".  That is, fall back to the last result in the filter chain, or if this
+    /// is the first in the chain, just use SourceGraphic.
+    fn get_unspecified_input(&self) -> FilterInput {
+        if let Some(output) = self.last_result.as_ref() {
+            FilterInput::PrimitiveOutput(output.clone())
+        } else {
+            FilterInput::StandardInput(self.source_graphic().clone())
+        }
+    }
+
     /// Retrieves the filter input surface according to the SVG rules.
     fn get_input_raw(
         &self,
@@ -288,16 +302,7 @@ impl FilterContext {
         in_: &Input,
     ) -> Result<FilterInput, FilterError> {
         match *in_ {
-            Input::Unspecified => {
-                // No value => use the last result.
-                // As per the SVG spec, if the filter primitive is the first in the chain, return the
-                // source graphic.
-                if let Some(output) = self.last_result.as_ref() {
-                    Ok(FilterInput::PrimitiveOutput(output.clone()))
-                } else {
-                    Ok(FilterInput::StandardInput(self.source_graphic().clone()))
-                }
-            }
+            Input::Unspecified => Ok(self.get_unspecified_input()),
 
             Input::SourceGraphic => Ok(FilterInput::StandardInput(self.source_graphic().clone())),
 
@@ -328,12 +333,25 @@ impl FilterContext {
                 .stroke_paint_image(acquired_nodes, draw_ctx)
                 .map(FilterInput::StandardInput),
 
-            Input::FilterOutput(ref name) => self
-                .previous_results
-                .get(name)
-                .cloned()
-                .map(FilterInput::PrimitiveOutput)
-                .ok_or(FilterError::InvalidInput),
+            Input::FilterOutput(ref name) => {
+                let input = match self.previous_results.get(name).cloned() {
+                    Some(filter_output) => {
+                        // Happy path: we found a previous primitive's named output, so pass it on.
+                        FilterInput::PrimitiveOutput(filter_output)
+                    }
+
+                    None => {
+                        // Fallback path: we didn't find a primitive's output by the
+                        // specified name, so fall back to using an unspecified output.
+                        // Per the spec, "References to non-existent results will be
+                        // treated as if no result was specified." -
+                        // https://drafts.fxtf.org/filter-effects/#element-attrdef-filter-primitive-in
+                        self.get_unspecified_input()
+                    }
+                };
+
+                Ok(input)
+            }
         }
     }
 
diff --git a/src/filters/convolve_matrix.rs b/src/filters/convolve_matrix.rs
index 54a390c8..db9dc047 100644
--- a/src/filters/convolve_matrix.rs
+++ b/src/filters/convolve_matrix.rs
@@ -1,13 +1,13 @@
 use cssparser::Parser;
-use markup5ever::{expanded_name, local_name, namespace_url, ns, QualName};
+use markup5ever::{expanded_name, local_name, namespace_url, ns};
 use nalgebra::{DMatrix, Dynamic, VecStorage};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node};
-use crate::parsers::{NonNegative, NumberList, NumberOptionalNumber, Parse, ParseValue};
+use crate::parsers::{NumberList, NumberOptionalNumber, Parse, ParseValue};
 use crate::properties::ColorInterpolationFilters;
 use crate::rect::IRect;
 use crate::session::Session;
@@ -37,8 +37,8 @@ pub struct FeConvolveMatrix {
 #[derive(Clone)]
 pub struct ConvolveMatrix {
     in1: Input,
-    order: (u32, u32),
-    kernel_matrix: Option<DMatrix<f64>>,
+    order: NumberOptionalNumber<u32>,
+    kernel_matrix: NumberList<0, 400>, // #691: Limit list to 400 (20x20) to mitigate malicious SVGs
     divisor: f64,
     bias: f64,
     target_x: Option<u32>,
@@ -55,8 +55,8 @@ impl Default for ConvolveMatrix {
     fn default() -> ConvolveMatrix {
         ConvolveMatrix {
             in1: Default::default(),
-            order: (3, 3),
-            kernel_matrix: None,
+            order: NumberOptionalNumber(3, 3),
+            kernel_matrix: NumberList(Vec::new()),
             divisor: 0.0,
             bias: 0.0,
             target_x: None,
@@ -70,70 +70,51 @@ impl Default for ConvolveMatrix {
 }
 
 impl SetAttributes for FeConvolveMatrix {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "order") => {
-                    let NumberOptionalNumber(x, y) = attr.parse(value)?;
-                    self.params.order = (x, y);
+                    set_attribute(&mut self.params.order, attr.parse(value), session)
+                }
+                expanded_name!("", "kernelMatrix") => {
+                    set_attribute(&mut self.params.kernel_matrix, attr.parse(value), session)
+                }
+                expanded_name!("", "divisor") => {
+                    set_attribute(&mut self.params.divisor, attr.parse(value), session)
+                }
+                expanded_name!("", "bias") => {
+                    set_attribute(&mut self.params.bias, attr.parse(value), session)
+                }
+                expanded_name!("", "targetX") => {
+                    set_attribute(&mut self.params.target_x, attr.parse(value), session)
+                }
+                expanded_name!("", "targetY") => {
+                    set_attribute(&mut self.params.target_y, attr.parse(value), session)
+                }
+                expanded_name!("", "edgeMode") => {
+                    set_attribute(&mut self.params.edge_mode, attr.parse(value), session)
                 }
-                expanded_name!("", "divisor") => self.params.divisor = attr.parse(value)?,
-                expanded_name!("", "bias") => self.params.bias = attr.parse(value)?,
-                expanded_name!("", "targetX") => self.params.target_x = attr.parse(value)?,
-                expanded_name!("", "targetY") => self.params.target_y = attr.parse(value)?,
-                expanded_name!("", "edgeMode") => self.params.edge_mode = attr.parse(value)?,
                 expanded_name!("", "kernelUnitLength") => {
-                    let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
-                    self.params.kernel_unit_length = Some((x, y))
+                    let v: Result<NumberOptionalNumber<f64>, _> = attr.parse(value);
+                    match v {
+                        Ok(NumberOptionalNumber(x, y)) => {
+                            self.params.kernel_unit_length = Some((x, y));
+                        }
+
+                        Err(e) => {
+                            rsvg_log!(session, "ignoring attribute with invalid value: {}", e);
+                        }
+                    }
                 }
                 expanded_name!("", "preserveAlpha") => {
-                    self.params.preserve_alpha = attr.parse(value)?
+                    set_attribute(&mut self.params.preserve_alpha, attr.parse(value), session);
                 }
 
                 _ => (),
             }
         }
-
-        // Finally, parse the kernel matrix.
-        for (attr, value) in attrs
-            .iter()
-            .filter(|(attr, _)| attr.expanded() == expanded_name!("", "kernelMatrix"))
-        {
-            self.params.kernel_matrix = Some({
-                let number_of_elements =
-                    self.params.order.0 as usize * self.params.order.1 as usize;
-
-                // #352: Parse as an unbounded list rather than exact length to prevent aborts due
-                //       to huge allocation attempts by underlying Vec::with_capacity().
-                // #691: Limit list to 400 (20x20) to mitigate malicious SVGs
-                let NumberList::<0, 400>(v) = attr.parse(value)?;
-                // #691: Update check as v.len can be different than number of elements because
-                //       of the above limit (and will = 400 if that happens)
-                if v.len() != number_of_elements && v.len() != 400 {
-                    return Err(ValueErrorKind::value_error(&format!(
-                        "incorrect number of elements: expected {}",
-                        number_of_elements
-                    )))
-                    .attribute(attr);
-                }
-
-                DMatrix::from_data(VecStorage::new(
-                    Dynamic::new(self.params.order.1 as usize),
-                    Dynamic::new(self.params.order.0 as usize),
-                    v,
-                ))
-            });
-        }
-
-        // kernel_matrix must have been specified.
-        if self.params.kernel_matrix.is_none() {
-            return Err(ValueErrorKind::value_error("the value must be set"))
-                .attribute(QualName::new(None, ns!(svg), local_name!("kernelMatrix")));
-        }
-
-        Ok(())
     }
 }
 
@@ -189,6 +170,13 @@ impl ConvolveMatrix {
 
         let scale = self
             .kernel_unit_length
+            .and_then(|(x, y)| {
+                if x <= 0.0 || y <= 0.0 {
+                    None
+                } else {
+                    Some((x, y))
+                }
+            })
             .map(|(dx, dy)| ctx.paffine().transform_distance(dx, dy));
 
         if let Some((ox, oy)) = scale {
@@ -199,7 +187,33 @@ impl ConvolveMatrix {
             bounds = new_bounds;
         }
 
-        let matrix = self.kernel_matrix.as_ref().unwrap();
+        let cols = self.order.0 as usize;
+        let rows = self.order.1 as usize;
+        let number_of_elements = cols * rows;
+        let numbers = self.kernel_matrix.0.clone();
+
+        if numbers.len() != number_of_elements && numbers.len() != 400 {
+            // "If the result of orderX * orderY is not equal to the the number of entries
+            // in the value list, the filter primitive acts as a pass through filter."
+            //
+            // https://drafts.fxtf.org/filter-effects/#element-attrdef-feconvolvematrix-kernelmatrix
+            rsvg_log!(
+                draw_ctx.session(),
+                "feConvolveMatrix got {} elements when it expected {}; ignoring it",
+                numbers.len(),
+                number_of_elements
+            );
+            return Ok(FilterOutput {
+                surface: input_1.surface().clone(),
+                bounds: original_bounds,
+            });
+        }
+
+        let matrix = DMatrix::from_data(VecStorage::new(
+            Dynamic::new(rows),
+            Dynamic::new(cols),
+            numbers,
+        ));
 
         let divisor = if self.divisor != 0.0 {
             self.divisor
diff --git a/src/filters/displacement_map.rs b/src/filters/displacement_map.rs
index c88b4eec..d8781d80 100644
--- a/src/filters/displacement_map.rs
+++ b/src/filters/displacement_map.rs
@@ -3,7 +3,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node};
 use crate::parsers::{Parse, ParseValue};
@@ -50,25 +50,33 @@ pub struct DisplacementMap {
 }
 
 impl SetAttributes for FeDisplacementMap {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        let (in1, in2) = self.base.parse_two_inputs(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        let (in1, in2) = self.base.parse_two_inputs(attrs, session);
         self.params.in1 = in1;
         self.params.in2 = in2;
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "scale") => self.params.scale = attr.parse(value)?,
+                expanded_name!("", "scale") => {
+                    set_attribute(&mut self.params.scale, attr.parse(value), session)
+                }
                 expanded_name!("", "xChannelSelector") => {
-                    self.params.x_channel_selector = attr.parse(value)?
+                    set_attribute(
+                        &mut self.params.x_channel_selector,
+                        attr.parse(value),
+                        session,
+                    );
                 }
                 expanded_name!("", "yChannelSelector") => {
-                    self.params.y_channel_selector = attr.parse(value)?
+                    set_attribute(
+                        &mut self.params.y_channel_selector,
+                        attr.parse(value),
+                        session,
+                    );
                 }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/filters/flood.rs b/src/filters/flood.rs
index 4a596e92..a3f06422 100644
--- a/src/filters/flood.rs
+++ b/src/filters/flood.rs
@@ -1,6 +1,6 @@
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::SetAttributes;
 use crate::node::{CascadedValues, Node};
 use crate::paint_server::resolve_color;
 use crate::rect::IRect;
@@ -25,8 +25,8 @@ pub struct Flood {
 }
 
 impl SetAttributes for FeFlood {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.base.parse_no_inputs(attrs)
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.base.parse_no_inputs(attrs, session);
     }
 }
 
diff --git a/src/filters/gaussian_blur.rs b/src/filters/gaussian_blur.rs
index 3d4fe9ae..d44e5ae0 100644
--- a/src/filters/gaussian_blur.rs
+++ b/src/filters/gaussian_blur.rs
@@ -6,9 +6,9 @@ use nalgebra::{DMatrix, Dynamic, VecStorage};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::node::{CascadedValues, Node};
-use crate::parsers::{NonNegative, NumberOptionalNumber, ParseValue};
+use crate::parsers::{NumberOptionalNumber, ParseValue};
 use crate::properties::ColorInterpolationFilters;
 use crate::rect::IRect;
 use crate::session::Session;
@@ -38,25 +38,33 @@ pub struct FeGaussianBlur {
 }
 
 /// Resolved `feGaussianBlur` primitive for rendering.
-#[derive(Default, Clone)]
+#[derive(Clone)]
 pub struct GaussianBlur {
     pub in1: Input,
-    pub std_deviation: (f64, f64),
+    pub std_deviation: NumberOptionalNumber<f64>,
     pub color_interpolation_filters: ColorInterpolationFilters,
 }
 
+// We need this because NumberOptionalNumber doesn't impl Default
+impl Default for GaussianBlur {
+    fn default() -> GaussianBlur {
+        GaussianBlur {
+            in1: Default::default(),
+            std_deviation: NumberOptionalNumber(0.0, 0.0),
+            color_interpolation_filters: Default::default(),
+        }
+    }
+}
+
 impl SetAttributes for FeGaussianBlur {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         for (attr, value) in attrs.iter() {
             if let expanded_name!("", "stdDeviation") = attr.expanded() {
-                let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
-                self.params.std_deviation = (x, y);
+                set_attribute(&mut self.params.std_deviation, attr.parse(value), session);
             }
         }
-
-        Ok(())
     }
 }
 
@@ -209,7 +217,18 @@ impl GaussianBlur {
             .clipped
             .into();
 
-        let (std_x, std_y) = self.std_deviation;
+        let NumberOptionalNumber(std_x, std_y) = self.std_deviation;
+
+        // "A negative value or a value of zero disables the effect of
+        // the given filter primitive (i.e., the result is the filter
+        // input image)."
+        if std_x <= 0.0 && std_y <= 0.0 {
+            return Ok(FilterOutput {
+                surface: input_1.surface().clone(),
+                bounds,
+            });
+        }
+
         let (std_x, std_y) = ctx.paffine().transform_distance(std_x, std_y);
 
         // The deviation can become negative here due to the transform.
diff --git a/src/filters/image.rs b/src/filters/image.rs
index 65ddf759..a926c952 100644
--- a/src/filters/image.rs
+++ b/src/filters/image.rs
@@ -3,7 +3,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 use crate::aspect_ratio::AspectRatio;
 use crate::document::{AcquiredNodes, NodeId};
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::href::{is_href, set_href};
 use crate::node::{CascadedValues, Node};
 use crate::parsers::ParseValue;
@@ -116,25 +116,23 @@ impl Image {
 }
 
 impl SetAttributes for FeImage {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.base.parse_no_inputs(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.base.parse_no_inputs(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "preserveAspectRatio") => {
-                    self.params.aspect = attr.parse(value)?
+                    set_attribute(&mut self.params.aspect, attr.parse(value), session);
                 }
 
                 // "path" is used by some older Adobe Illustrator versions
                 ref a if is_href(a) || *a == expanded_name!("", "path") => {
-                    set_href(a, &mut self.params.href, value.to_string());
+                    set_href(a, &mut self.params.href, Some(value.to_string()));
                 }
 
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/filters/lighting.rs b/src/filters/lighting.rs
index 3d4c73a5..e222de22 100644
--- a/src/filters/lighting.rs
+++ b/src/filters/lighting.rs
@@ -9,7 +9,7 @@ use std::cmp::max;
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::filters::{
     bounds::BoundsBuilder,
     context::{FilterContext, FilterOutput},
@@ -43,7 +43,7 @@ pub struct DiffuseLightingParams {
     in1: Input,
     surface_scale: f64,
     kernel_unit_length: Option<(f64, f64)>,
-    diffuse_constant: f64,
+    diffuse_constant: NonNegative,
 }
 
 impl Default for DiffuseLightingParams {
@@ -52,7 +52,7 @@ impl Default for DiffuseLightingParams {
             in1: Default::default(),
             surface_scale: 1.0,
             kernel_unit_length: None,
-            diffuse_constant: 1.0,
+            diffuse_constant: NonNegative(1.0),
         }
     }
 }
@@ -69,7 +69,7 @@ pub struct SpecularLightingParams {
     in1: Input,
     surface_scale: f64,
     kernel_unit_length: Option<(f64, f64)>,
-    specular_constant: f64,
+    specular_constant: NonNegative,
     specular_exponent: f64,
 }
 
@@ -79,7 +79,7 @@ impl Default for SpecularLightingParams {
             in1: Default::default(),
             surface_scale: 1.0,
             kernel_unit_length: None,
-            specular_constant: 1.0,
+            specular_constant: NonNegative(1.0),
             specular_exponent: 1.0,
         }
     }
@@ -214,16 +214,18 @@ impl FeDistantLight {
 }
 
 impl SetAttributes for FeDistantLight {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "azimuth") => self.azimuth = attr.parse(value)?,
-                expanded_name!("", "elevation") => self.elevation = attr.parse(value)?,
+                expanded_name!("", "azimuth") => {
+                    set_attribute(&mut self.azimuth, attr.parse(value), session)
+                }
+                expanded_name!("", "elevation") => {
+                    set_attribute(&mut self.elevation, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -248,23 +250,21 @@ impl FePointLight {
 }
 
 impl SetAttributes for FePointLight {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "z") => self.z = attr.parse(value)?,
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "z") => set_attribute(&mut self.z, attr.parse(value), session),
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
 impl Draw for FePointLight {}
 
-#[derive(Clone, Debug, Default, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct FeSpotLight {
     x: f64,
     y: f64,
@@ -276,6 +276,23 @@ pub struct FeSpotLight {
     limiting_cone_angle: Option<f64>,
 }
 
+// We need this because, per the spec, the initial values for all fields are 0.0
+// except for specular_exponent, which is 1.
+impl Default for FeSpotLight {
+    fn default() -> FeSpotLight {
+        FeSpotLight {
+            x: 0.0,
+            y: 0.0,
+            z: 0.0,
+            points_at_x: 0.0,
+            points_at_y: 0.0,
+            points_at_z: 0.0,
+            specular_exponent: 1.0,
+            limiting_cone_angle: None,
+        }
+    }
+}
+
 impl FeSpotLight {
     fn transform(&self, paffine: Transform) -> LightSource {
         let (x, y) = paffine.transform_point(self.x, self.y);
@@ -298,29 +315,33 @@ impl FeSpotLight {
 }
 
 impl SetAttributes for FeSpotLight {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "z") => self.z = attr.parse(value)?,
-                expanded_name!("", "pointsAtX") => self.points_at_x = attr.parse(value)?,
-                expanded_name!("", "pointsAtY") => self.points_at_y = attr.parse(value)?,
-                expanded_name!("", "pointsAtZ") => self.points_at_z = attr.parse(value)?,
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "z") => set_attribute(&mut self.z, attr.parse(value), session),
+                expanded_name!("", "pointsAtX") => {
+                    set_attribute(&mut self.points_at_x, attr.parse(value), session)
+                }
+                expanded_name!("", "pointsAtY") => {
+                    set_attribute(&mut self.points_at_y, attr.parse(value), session)
+                }
+                expanded_name!("", "pointsAtZ") => {
+                    set_attribute(&mut self.points_at_z, attr.parse(value), session)
+                }
 
                 expanded_name!("", "specularExponent") => {
-                    self.specular_exponent = attr.parse(value)?
+                    set_attribute(&mut self.specular_exponent, attr.parse(value), session);
                 }
 
                 expanded_name!("", "limitingConeAngle") => {
-                    self.limiting_cone_angle = attr.parse(value)?
+                    set_attribute(&mut self.limiting_cone_angle, attr.parse(value), session);
                 }
 
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -333,27 +354,36 @@ fn transform_dist(t: Transform, d: f64) -> f64 {
 }
 
 impl SetAttributes for FeDiffuseLighting {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "surfaceScale") => {
-                    self.params.surface_scale = attr.parse(value)?
+                    set_attribute(&mut self.params.surface_scale, attr.parse(value), session);
                 }
                 expanded_name!("", "kernelUnitLength") => {
-                    let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
-                    self.params.kernel_unit_length = Some((x, y));
+                    let v: Result<NumberOptionalNumber<f64>, _> = attr.parse(value);
+                    match v {
+                        Ok(NumberOptionalNumber(x, y)) => {
+                            self.params.kernel_unit_length = Some((x, y));
+                        }
+
+                        Err(e) => {
+                            rsvg_log!(session, "ignoring attribute with invalid value: {}", e);
+                        }
+                    }
                 }
                 expanded_name!("", "diffuseConstant") => {
-                    let NonNegative(c) = attr.parse(value)?;
-                    self.params.diffuse_constant = c;
+                    set_attribute(
+                        &mut self.params.diffuse_constant,
+                        attr.parse(value),
+                        session,
+                    );
                 }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -373,35 +403,48 @@ impl DiffuseLighting {
             normal.dot(&light_vector) / normal.norm()
         };
 
-        self.params.diffuse_constant * k
+        self.params.diffuse_constant.0 * k
     }
 }
 
 impl SetAttributes for FeSpecularLighting {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "surfaceScale") => {
-                    self.params.surface_scale = attr.parse(value)?
+                    set_attribute(&mut self.params.surface_scale, attr.parse(value), session);
                 }
                 expanded_name!("", "kernelUnitLength") => {
-                    let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
-                    self.params.kernel_unit_length = Some((x, y));
+                    let v: Result<NumberOptionalNumber<f64>, _> = attr.parse(value);
+                    match v {
+                        Ok(NumberOptionalNumber(x, y)) => {
+                            self.params.kernel_unit_length = Some((x, y));
+                        }
+
+                        Err(e) => {
+                            rsvg_log!(session, "ignoring attribute with invalid value: {}", e);
+                        }
+                    }
                 }
                 expanded_name!("", "specularConstant") => {
-                    let NonNegative(c) = attr.parse(value)?;
-                    self.params.specular_constant = c;
+                    set_attribute(
+                        &mut self.params.specular_constant,
+                        attr.parse(value),
+                        session,
+                    );
                 }
                 expanded_name!("", "specularExponent") => {
-                    self.params.specular_exponent = attr.parse(value)?;
+                    set_attribute(
+                        &mut self.params.specular_exponent,
+                        attr.parse(value),
+                        session,
+                    );
                 }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -428,9 +471,9 @@ impl SpecularLighting {
         };
 
         if approx_eq!(f64, self.params.specular_exponent, 1.0) {
-            self.params.specular_constant * n_dot_h
+            self.params.specular_constant.0 * n_dot_h
         } else {
-            self.params.specular_constant * n_dot_h.powf(self.params.specular_exponent)
+            self.params.specular_constant.0 * n_dot_h.powf(self.params.specular_exponent)
         }
     }
 }
@@ -461,6 +504,13 @@ macro_rules! impl_lighting_filter {
                 let scale = self
                     .params
                     .kernel_unit_length
+                    .and_then(|(x, y)| {
+                        if x <= 0.0 || y <= 0.0 {
+                            None
+                        } else {
+                            Some((x, y))
+                        }
+                    })
                     .map(|(dx, dy)| ctx.paffine().transform_distance(dx, dy));
 
                 let mut input_surface = input_1.surface().clone();
@@ -674,10 +724,6 @@ macro_rules! impl_lighting_filter {
                 let source_node = source_node.unwrap();
                 let elt = source_node.borrow_element();
 
-                if elt.is_in_error() {
-                    return Err(FilterResolveError::ChildNodeInError);
-                }
-
                 let source = match *elt {
                     Element::FeDistantLight(ref l) => {
                         UntransformedLightSource::Distant(l.element_impl.clone())
diff --git a/src/filters/merge.rs b/src/filters/merge.rs
index e2e9be65..56292406 100644
--- a/src/filters/merge.rs
+++ b/src/filters/merge.rs
@@ -2,7 +2,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::node::{CascadedValues, Node, NodeBorrow};
 use crate::parsers::ParseValue;
 use crate::properties::ColorInterpolationFilters;
@@ -52,21 +52,19 @@ impl Default for FeMerge {
 }
 
 impl SetAttributes for FeMerge {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.base.parse_no_inputs(attrs)
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.base.parse_no_inputs(attrs, session);
     }
 }
 
 impl SetAttributes for FeMergeNode {
     #[inline]
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             if let expanded_name!("", "in") = attr.expanded() {
-                self.in1 = attr.parse(value)?;
+                set_attribute(&mut self.in1, attr.parse(value), session);
             }
         }
-
-        Ok(())
     }
 }
 
@@ -164,10 +162,6 @@ fn resolve_merge_nodes(node: &Node) -> Result<Vec<MergeNode>, FilterResolveError
     for child in node.children().filter(|c| c.is_element()) {
         let elt = child.borrow_element();
 
-        if elt.is_in_error() {
-            return Err(FilterResolveError::ChildNodeInError);
-        }
-
         let cascaded = CascadedValues::new_from_node(&child);
         let values = cascaded.get();
 
diff --git a/src/filters/mod.rs b/src/filters/mod.rs
index b7b545b7..c2ec8168 100644
--- a/src/filters/mod.rs
+++ b/src/filters/mod.rs
@@ -8,14 +8,15 @@ use std::time::Instant;
 use crate::bbox::BoundingBox;
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, ElementResult, SetAttributes};
-use crate::error::{ElementError, ParseError, RenderingError};
+use crate::element::{set_attribute, Draw, SetAttributes};
+use crate::error::{ParseError, RenderingError};
 use crate::filter::UserSpaceFilter;
 use crate::length::*;
 use crate::node::Node;
 use crate::paint_server::UserSpacePaintSource;
 use crate::parsers::{CustomIdent, Parse, ParseValue};
 use crate::properties::ColorInterpolationFilters;
+use crate::session::Session;
 use crate::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
 use crate::transform::Transform;
 use crate::xml::Attributes;
@@ -206,38 +207,46 @@ impl Primitive {
     fn parse_standard_attributes(
         &mut self,
         attrs: &Attributes,
-    ) -> Result<(Input, Input), ElementError> {
+        session: &Session,
+    ) -> (Input, Input) {
         let mut input_1 = Input::Unspecified;
         let mut input_2 = Input::Unspecified;
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "width") => self.width = attr.parse(value)?,
-                expanded_name!("", "height") => self.height = attr.parse(value)?,
-                expanded_name!("", "result") => self.result = attr.parse(value)?,
-                expanded_name!("", "in") => input_1 = attr.parse(value)?,
-                expanded_name!("", "in2") => input_2 = attr.parse(value)?,
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "width") => {
+                    set_attribute(&mut self.width, attr.parse(value), session)
+                }
+                expanded_name!("", "height") => {
+                    set_attribute(&mut self.height, attr.parse(value), session)
+                }
+                expanded_name!("", "result") => {
+                    set_attribute(&mut self.result, attr.parse(value), session)
+                }
+                expanded_name!("", "in") => set_attribute(&mut input_1, attr.parse(value), session),
+                expanded_name!("", "in2") => {
+                    set_attribute(&mut input_2, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
 
-        Ok((input_1, input_2))
+        (input_1, input_2)
     }
 
-    pub fn parse_no_inputs(&mut self, attrs: &Attributes) -> ElementResult {
-        let (_, _) = self.parse_standard_attributes(attrs)?;
-        Ok(())
+    pub fn parse_no_inputs(&mut self, attrs: &Attributes, session: &Session) {
+        let (_, _) = self.parse_standard_attributes(attrs, session);
     }
 
-    pub fn parse_one_input(&mut self, attrs: &Attributes) -> Result<Input, ElementError> {
-        let (input_1, _) = self.parse_standard_attributes(attrs)?;
-        Ok(input_1)
+    pub fn parse_one_input(&mut self, attrs: &Attributes, session: &Session) -> Input {
+        let (input_1, _) = self.parse_standard_attributes(attrs, session);
+        input_1
     }
 
-    pub fn parse_two_inputs(&mut self, attrs: &Attributes) -> Result<(Input, Input), ElementError> {
-        self.parse_standard_attributes(attrs)
+    pub fn parse_two_inputs(&mut self, attrs: &Attributes, session: &Session) -> (Input, Input) {
+        self.parse_standard_attributes(attrs, session)
     }
 }
 
diff --git a/src/filters/morphology.rs b/src/filters/morphology.rs
index 8a6c0725..519efca7 100644
--- a/src/filters/morphology.rs
+++ b/src/filters/morphology.rs
@@ -5,10 +5,10 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::Node;
-use crate::parsers::{NonNegative, NumberOptionalNumber, Parse, ParseValue};
+use crate::parsers::{NumberOptionalNumber, Parse, ParseValue};
 use crate::properties::ColorInterpolationFilters;
 use crate::rect::IRect;
 use crate::session::Session;
@@ -43,29 +43,39 @@ pub struct FeMorphology {
 }
 
 /// Resolved `feMorphology` primitive for rendering.
-#[derive(Clone, Default)]
+#[derive(Clone)]
 pub struct Morphology {
     in1: Input,
     operator: Operator,
-    radius: (f64, f64),
+    radius: NumberOptionalNumber<f64>,
+}
+
+// We need this because NumberOptionalNumber doesn't impl Default
+impl Default for Morphology {
+    fn default() -> Morphology {
+        Morphology {
+            in1: Default::default(),
+            operator: Default::default(),
+            radius: NumberOptionalNumber(0.0, 0.0),
+        }
+    }
 }
 
 impl SetAttributes for FeMorphology {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "operator") => self.params.operator = attr.parse(value)?,
+                expanded_name!("", "operator") => {
+                    set_attribute(&mut self.params.operator, attr.parse(value), session);
+                }
                 expanded_name!("", "radius") => {
-                    let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
-                    self.params.radius = (x, y);
+                    set_attribute(&mut self.params.radius, attr.parse(value), session);
                 }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -97,7 +107,15 @@ impl Morphology {
             .clipped
             .into();
 
-        let (rx, ry) = self.radius;
+        let NumberOptionalNumber(rx, ry) = self.radius;
+
+        if rx <= 0.0 && ry <= 0.0 {
+            return Ok(FilterOutput {
+                surface: input_1.surface().clone(),
+                bounds,
+            });
+        }
+
         let (rx, ry) = ctx.paffine().transform_distance(rx, ry);
 
         // The radii can become negative here due to the transform.
diff --git a/src/filters/offset.rs b/src/filters/offset.rs
index d60542a5..acdbab1a 100644
--- a/src/filters/offset.rs
+++ b/src/filters/offset.rs
@@ -2,7 +2,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::node::Node;
 use crate::parsers::ParseValue;
 use crate::properties::ColorInterpolationFilters;
@@ -33,18 +33,20 @@ pub struct Offset {
 }
 
 impl SetAttributes for FeOffset {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "dx") => self.params.dx = attr.parse(value)?,
-                expanded_name!("", "dy") => self.params.dy = attr.parse(value)?,
+                expanded_name!("", "dx") => {
+                    set_attribute(&mut self.params.dx, attr.parse(value), session)
+                }
+                expanded_name!("", "dy") => {
+                    set_attribute(&mut self.params.dy, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/filters/tile.rs b/src/filters/tile.rs
index 45549a71..0bd4417c 100644
--- a/src/filters/tile.rs
+++ b/src/filters/tile.rs
@@ -1,6 +1,6 @@
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::SetAttributes;
 use crate::node::Node;
 use crate::properties::ColorInterpolationFilters;
 use crate::rect::IRect;
@@ -28,9 +28,8 @@ pub struct Tile {
 }
 
 impl SetAttributes for FeTile {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.params.in1 = self.base.parse_one_input(attrs)?;
-        Ok(())
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.params.in1 = self.base.parse_one_input(attrs, session);
     }
 }
 
diff --git a/src/filters/turbulence.rs b/src/filters/turbulence.rs
index cb962c45..32035f5d 100644
--- a/src/filters/turbulence.rs
+++ b/src/filters/turbulence.rs
@@ -3,10 +3,10 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{ElementResult, SetAttributes};
+use crate::element::{set_attribute, SetAttributes};
 use crate::error::*;
 use crate::node::{CascadedValues, Node};
-use crate::parsers::{NonNegative, NumberOptionalNumber, Parse, ParseValue};
+use crate::parsers::{NumberOptionalNumber, Parse, ParseValue};
 use crate::properties::ColorInterpolationFilters;
 use crate::rect::IRect;
 use crate::session::Session;
@@ -51,9 +51,9 @@ pub struct FeTurbulence {
 /// Resolved `feTurbulence` primitive for rendering.
 #[derive(Clone)]
 pub struct Turbulence {
-    base_frequency: (f64, f64),
+    base_frequency: NumberOptionalNumber<f64>,
     num_octaves: i32,
-    seed: i32,
+    seed: f64,
     stitch_tiles: StitchTiles,
     type_: NoiseType,
     color_interpolation_filters: ColorInterpolationFilters,
@@ -64,9 +64,9 @@ impl Default for Turbulence {
     #[inline]
     fn default() -> Turbulence {
         Turbulence {
-            base_frequency: (0.0, 0.0),
+            base_frequency: NumberOptionalNumber(0.0, 0.0),
             num_octaves: 1,
-            seed: 0,
+            seed: 0.0,
             stitch_tiles: Default::default(),
             type_: Default::default(),
             color_interpolation_filters: Default::default(),
@@ -75,36 +75,30 @@ impl Default for Turbulence {
 }
 
 impl SetAttributes for FeTurbulence {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        self.base.parse_no_inputs(attrs)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.base.parse_no_inputs(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "baseFrequency") => {
-                    let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
-                    self.params.base_frequency = (x, y);
+                    set_attribute(&mut self.params.base_frequency, attr.parse(value), session);
                 }
                 expanded_name!("", "numOctaves") => {
-                    self.params.num_octaves = attr.parse(value)?;
+                    set_attribute(&mut self.params.num_octaves, attr.parse(value), session);
                 }
                 // Yes, seed needs to be parsed as a number and then truncated.
                 expanded_name!("", "seed") => {
-                    let v: f64 = attr.parse(value)?;
-                    self.params.seed = clamp(
-                        v.trunc(),
-                        f64::from(i32::min_value()),
-                        f64::from(i32::max_value()),
-                    ) as i32;
+                    set_attribute(&mut self.params.seed, attr.parse(value), session);
                 }
                 expanded_name!("", "stitchTiles") => {
-                    self.params.stitch_tiles = attr.parse(value)?
+                    set_attribute(&mut self.params.stitch_tiles, attr.parse(value), session);
+                }
+                expanded_name!("", "type") => {
+                    set_attribute(&mut self.params.type_, attr.parse(value), session)
                 }
-                expanded_name!("", "type") => self.params.type_ = attr.parse(value)?,
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -355,9 +349,25 @@ impl Turbulence {
 
         let affine = ctx.paffine().invert().unwrap();
 
+        let seed = clamp(
+            self.seed.trunc(), // per the spec, round towards zero
+            f64::from(i32::min_value()),
+            f64::from(i32::max_value()),
+        ) as i32;
+
+        // "Negative values are unsupported" -> set to the initial value which is 0.0
+        //
+        // https://drafts.fxtf.org/filter-effects/#element-attrdef-feturbulence-basefrequency
+        let base_frequency = {
+            let NumberOptionalNumber(base_freq_x, base_freq_y) = self.base_frequency;
+            let x = base_freq_x.max(0.0);
+            let y = base_freq_y.max(0.0);
+            (x, y)
+        };
+
         let noise_generator = NoiseGenerator::new(
-            self.seed,
-            self.base_frequency,
+            seed,
+            base_frequency,
             self.num_octaves,
             self.type_,
             self.stitch_tiles,
diff --git a/src/gradient.rs b/src/gradient.rs
index 13e10559..345eb7cc 100644
--- a/src/gradient.rs
+++ b/src/gradient.rs
@@ -8,7 +8,7 @@ use markup5ever::{
 use crate::coord_units::CoordUnits;
 use crate::document::{AcquiredNodes, NodeId, NodeStack};
 use crate::drawing_ctx::ViewParams;
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::error::*;
 use crate::href::{is_href, set_href};
 use crate::length::*;
@@ -66,14 +66,12 @@ pub struct Stop {
 }
 
 impl SetAttributes for Stop {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
-            if let expanded_name!("", "offset") = attr.expanded() {
-                self.offset = attr.parse(value)?;
+            if attr.expanded() == expanded_name!("", "offset") {
+                set_attribute(&mut self.offset, attr.parse(value), session);
             }
         }
-
-        Ok(())
     }
 }
 
@@ -287,7 +285,7 @@ impl UnresolvedVariant {
 #[derive(Default)]
 struct Common {
     units: Option<GradientUnits>,
-    transform: Option<Transform>,
+    transform: Option<TransformAttribute>,
     spread: Option<SpreadMethod>,
 
     fallback: Option<NodeId>,
@@ -323,7 +321,7 @@ pub struct RadialGradient {
 /// field was specified.
 struct UnresolvedGradient {
     units: Option<GradientUnits>,
-    transform: Option<Transform>,
+    transform: Option<TransformAttribute>,
     spread: Option<SpreadMethod>,
     stops: Option<Vec<ColorStop>>,
 
@@ -334,7 +332,7 @@ struct UnresolvedGradient {
 #[derive(Clone)]
 pub struct ResolvedGradient {
     units: GradientUnits,
-    transform: Transform,
+    transform: TransformAttribute,
     spread: SpreadMethod,
     stops: Vec<ColorStop>,
 
@@ -410,7 +408,7 @@ impl UnresolvedGradient {
 
     /// Looks for <stop> children inside a linearGradient or radialGradient node,
     /// and adds their info to the UnresolvedGradient &self.
-    fn add_color_stops_from_node(&mut self, node: &Node, opacity: UnitInterval, session: &Session) {
+    fn add_color_stops_from_node(&mut self, node: &Node, opacity: UnitInterval) {
         assert!(matches!(
             *node.borrow_element(),
             Element::LinearGradient(_) | Element::RadialGradient(_)
@@ -420,26 +418,18 @@ impl UnresolvedGradient {
             let elt = child.borrow_element();
 
             if let Element::Stop(ref stop) = *elt {
-                if elt.is_in_error() {
-                    rsvg_log!(
-                        session,
-                        "(not using gradient stop {} because it is in error)",
-                        child
-                    );
-                } else {
-                    let cascaded = CascadedValues::new_from_node(&child);
-                    let values = cascaded.get();
+                let cascaded = CascadedValues::new_from_node(&child);
+                let values = cascaded.get();
 
-                    let UnitInterval(stop_opacity) = values.stop_opacity().0;
-                    let UnitInterval(o) = opacity;
+                let UnitInterval(stop_opacity) = values.stop_opacity().0;
+                let UnitInterval(o) = opacity;
 
-                    let composed_opacity = UnitInterval(stop_opacity * o);
+                let composed_opacity = UnitInterval(stop_opacity * o);
 
-                    let rgba =
-                        resolve_color(&values.stop_color().0, composed_opacity, values.color().0);
+                let rgba =
+                    resolve_color(&values.stop_color().0, composed_opacity, values.color().0);
 
-                    self.add_color_stop(stop.offset, rgba);
-                }
+                self.add_color_stop(stop.offset, rgba);
             }
         }
     }
@@ -470,7 +460,9 @@ impl UnresolvedGradient {
 
     fn resolve_from_defaults(&self) -> UnresolvedGradient {
         let units = self.units.or_else(|| Some(GradientUnits::default()));
-        let transform = self.transform.or_else(|| Some(Transform::default()));
+        let transform = self
+            .transform
+            .or_else(|| Some(TransformAttribute::default()));
         let spread = self.spread.or_else(|| Some(SpreadMethod::default()));
         let stops = self.stops.clone().or_else(|| Some(Vec::<ColorStop>::new()));
         let variant = self.variant.resolve_from_defaults();
@@ -520,46 +512,47 @@ impl RadialGradient {
 }
 
 impl SetAttributes for Common {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "gradientUnits") => self.units = attr.parse(value)?,
+                expanded_name!("", "gradientUnits") => {
+                    set_attribute(&mut self.units, attr.parse(value), session)
+                }
                 expanded_name!("", "gradientTransform") => {
-                    let transform_attr: TransformAttribute = attr.parse(value)?;
-                    self.transform = Some(transform_attr.to_transform());
+                    set_attribute(&mut self.transform, attr.parse(value), session);
+                }
+                expanded_name!("", "spreadMethod") => {
+                    set_attribute(&mut self.spread, attr.parse(value), session)
                 }
-                expanded_name!("", "spreadMethod") => self.spread = attr.parse(value)?,
                 ref a if is_href(a) => {
-                    set_href(
-                        a,
-                        &mut self.fallback,
-                        NodeId::parse(value).attribute(attr.clone())?,
+                    let mut href = None;
+                    set_attribute(
+                        &mut href,
+                        NodeId::parse(value).map(Some).attribute(attr.clone()),
+                        session,
                     );
+                    set_href(a, &mut self.fallback, href);
                 }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
 impl SetAttributes for LinearGradient {
-    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) -> ElementResult {
-        self.common.set_attributes(attrs, session)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.common.set_attributes(attrs, session);
 
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x1") => self.x1 = attr.parse(value)?,
-                expanded_name!("", "y1") => self.y1 = attr.parse(value)?,
-                expanded_name!("", "x2") => self.x2 = attr.parse(value)?,
-                expanded_name!("", "y2") => self.y2 = attr.parse(value)?,
+                expanded_name!("", "x1") => set_attribute(&mut self.x1, attr.parse(value), session),
+                expanded_name!("", "y1") => set_attribute(&mut self.y1, attr.parse(value), session),
+                expanded_name!("", "x2") => set_attribute(&mut self.x2, attr.parse(value), session),
+                expanded_name!("", "y2") => set_attribute(&mut self.y2, attr.parse(value), session),
 
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -568,12 +561,7 @@ impl Draw for LinearGradient {}
 macro_rules! impl_gradient {
     ($gradient_type:ident, $other_type:ident) => {
         impl $gradient_type {
-            fn get_unresolved(
-                &self,
-                node: &Node,
-                opacity: UnitInterval,
-                session: &Session,
-            ) -> Unresolved {
+            fn get_unresolved(&self, node: &Node, opacity: UnitInterval) -> Unresolved {
                 let mut gradient = UnresolvedGradient {
                     units: self.common.units,
                     transform: self.common.transform,
@@ -582,7 +570,7 @@ macro_rules! impl_gradient {
                     variant: self.get_unresolved_variant(),
                 };
 
-                gradient.add_color_stops_from_node(node, opacity, session);
+                gradient.add_color_stops_from_node(node, opacity);
 
                 Unresolved {
                     gradient,
@@ -595,12 +583,11 @@ macro_rules! impl_gradient {
                 node: &Node,
                 acquired_nodes: &mut AcquiredNodes<'_>,
                 opacity: UnitInterval,
-                session: &Session,
             ) -> Result<ResolvedGradient, AcquireError> {
                 let Unresolved {
                     mut gradient,
                     mut fallback,
-                } = self.get_unresolved(node, opacity, session);
+                } = self.get_unresolved(node, opacity);
 
                 let mut stack = NodeStack::new();
 
@@ -615,10 +602,10 @@ macro_rules! impl_gradient {
 
                         let unresolved = match *acquired_node.borrow_element() {
                             Element::$gradient_type(ref g) => {
-                                g.get_unresolved(&acquired_node, opacity, session)
+                                g.get_unresolved(&acquired_node, opacity)
                             }
                             Element::$other_type(ref g) => {
-                                g.get_unresolved(&acquired_node, opacity, session)
+                                g.get_unresolved(&acquired_node, opacity)
                             }
                             _ => return Err(AcquireError::InvalidLinkType(node_id.clone())),
                         };
@@ -643,8 +630,9 @@ impl_gradient!(LinearGradient, RadialGradient);
 impl_gradient!(RadialGradient, LinearGradient);
 
 impl SetAttributes for RadialGradient {
-    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) -> ElementResult {
-        self.common.set_attributes(attrs, session)?;
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        self.common.set_attributes(attrs, session);
+
         // Create a local expanded name for "fr" because markup5ever doesn't have built-in
         let expanded_name_fr = ExpandedName {
             ns: &Namespace::from(""),
@@ -653,23 +641,19 @@ impl SetAttributes for RadialGradient {
 
         for (attr, value) in attrs.iter() {
             let attr_expanded = attr.expanded();
-
-            if attr_expanded == expanded_name_fr {
-                self.fr = attr.parse(value)?;
-            } else {
-                match attr_expanded {
-                    expanded_name!("", "cx") => self.cx = attr.parse(value)?,
-                    expanded_name!("", "cy") => self.cy = attr.parse(value)?,
-                    expanded_name!("", "r") => self.r = attr.parse(value)?,
-                    expanded_name!("", "fx") => self.fx = attr.parse(value)?,
-                    expanded_name!("", "fy") => self.fy = attr.parse(value)?,
-
-                    _ => (),
+            match attr_expanded {
+                expanded_name!("", "cx") => set_attribute(&mut self.cx, attr.parse(value), session),
+                expanded_name!("", "cy") => set_attribute(&mut self.cy, attr.parse(value), session),
+                expanded_name!("", "r") => set_attribute(&mut self.r, attr.parse(value), session),
+                expanded_name!("", "fx") => set_attribute(&mut self.fx, attr.parse(value), session),
+                expanded_name!("", "fy") => set_attribute(&mut self.fy, attr.parse(value), session),
+                a if a == expanded_name_fr => {
+                    set_attribute(&mut self.fr, attr.parse(value), session)
                 }
+
+                _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -687,7 +671,8 @@ impl ResolvedGradient {
         let view_params = current_params.with_units(units);
         let params = NormalizeParams::new(values, &view_params);
 
-        let transform = transform.pre_transform(&self.transform).invert()?;
+        let gradient_transform = self.transform.to_transform();
+        let transform = transform.pre_transform(&gradient_transform).invert()?;
 
         let variant = match self.variant {
             ResolvedGradientVariant::Linear { x1, y1, x2, y2 } => GradientVariant::Linear {
@@ -753,11 +738,8 @@ mod tests {
             Attributes::new(),
         ));
 
-        let unresolved = borrow_element_as!(node, LinearGradient).get_unresolved(
-            &node,
-            UnitInterval::clamp(1.0),
-            &session,
-        );
+        let unresolved = borrow_element_as!(node, LinearGradient)
+            .get_unresolved(&node, UnitInterval::clamp(1.0));
         let gradient = unresolved.gradient.resolve_from_defaults();
         assert!(gradient.is_resolved());
 
@@ -767,11 +749,8 @@ mod tests {
             Attributes::new(),
         ));
 
-        let unresolved = borrow_element_as!(node, RadialGradient).get_unresolved(
-            &node,
-            UnitInterval::clamp(1.0),
-            &session,
-        );
+        let unresolved = borrow_element_as!(node, RadialGradient)
+            .get_unresolved(&node, UnitInterval::clamp(1.0));
         let gradient = unresolved.gradient.resolve_from_defaults();
         assert!(gradient.is_resolved());
     }
diff --git a/src/href.rs b/src/href.rs
index cb6892ec..94b05913 100644
--- a/src/href.rs
+++ b/src/href.rs
@@ -25,11 +25,11 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns, ExpandedName};
 ///     LocalName::from("href"),
 /// );
 ///
-/// let value = expanded_name!("", "path");
-/// let mut foo = Some(value);
+/// let value = "some_url";
+/// let mut my_field: Option<String> = None;
 ///
 /// match qual_name.expanded() {
-///     ref name if is_href(name) => set_href(name, &mut foo, value),
+///     ref name if is_href(name) => set_href(name, &mut my_field, Some(value.to_string())),
 ///     _ => unreachable!(),
 /// }
 /// ```
@@ -43,8 +43,8 @@ pub fn is_href(name: &ExpandedName<'_>) -> bool {
 /// Sets an `href` attribute in preference over an `xlink:href` one.
 ///
 /// See [`is_href`] for example usage.
-pub fn set_href<T>(name: &ExpandedName<'_>, dest: &mut Option<T>, href: T) {
+pub fn set_href<T>(name: &ExpandedName<'_>, dest: &mut Option<T>, href: Option<T>) {
     if dest.is_none() || *name != expanded_name!(xlink "href") {
-        *dest = Some(href);
+        *dest = href;
     }
 }
diff --git a/src/image.rs b/src/image.rs
index 530bf09e..97567d16 100644
--- a/src/image.rs
+++ b/src/image.rs
@@ -6,7 +6,7 @@ use crate::aspect_ratio::AspectRatio;
 use crate::bbox::BoundingBox;
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, SetAttributes};
 use crate::error::*;
 use crate::href::{is_href, set_href};
 use crate::layout::{self, StackingContext};
@@ -28,21 +28,21 @@ pub struct Image {
 }
 
 impl SetAttributes for Image {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "preserveAspectRatio") => self.aspect = attr.parse(value)?,
+                expanded_name!("", "preserveAspectRatio") => {
+                    set_attribute(&mut self.aspect, attr.parse(value), session)
+                }
 
                 // "path" is used by some older Adobe Illustrator versions
                 ref a if is_href(a) || *a == expanded_name!("", "path") => {
-                    set_href(a, &mut self.href, value.to_string())
+                    set_href(a, &mut self.href, Some(value.to_string()))
                 }
 
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/marker.rs b/src/marker.rs
index e416122a..f95be8a2 100644
--- a/src/marker.rs
+++ b/src/marker.rs
@@ -11,7 +11,7 @@ use crate::aspect_ratio::*;
 use crate::bbox::BoundingBox;
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, SetAttributes};
 use crate::error::*;
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::layout::{self, Shape, StackingContext};
@@ -200,22 +200,36 @@ impl Marker {
 }
 
 impl SetAttributes for Marker {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "markerUnits") => self.units = attr.parse(value)?,
-                expanded_name!("", "refX") => self.ref_x = attr.parse(value)?,
-                expanded_name!("", "refY") => self.ref_y = attr.parse(value)?,
-                expanded_name!("", "markerWidth") => self.width = attr.parse(value)?,
-                expanded_name!("", "markerHeight") => self.height = attr.parse(value)?,
-                expanded_name!("", "orient") => self.orient = attr.parse(value)?,
-                expanded_name!("", "preserveAspectRatio") => self.aspect = attr.parse(value)?,
-                expanded_name!("", "viewBox") => self.vbox = attr.parse(value)?,
+                expanded_name!("", "markerUnits") => {
+                    set_attribute(&mut self.units, attr.parse(value), session)
+                }
+                expanded_name!("", "refX") => {
+                    set_attribute(&mut self.ref_x, attr.parse(value), session)
+                }
+                expanded_name!("", "refY") => {
+                    set_attribute(&mut self.ref_y, attr.parse(value), session)
+                }
+                expanded_name!("", "markerWidth") => {
+                    set_attribute(&mut self.width, attr.parse(value), session)
+                }
+                expanded_name!("", "markerHeight") => {
+                    set_attribute(&mut self.height, attr.parse(value), session)
+                }
+                expanded_name!("", "orient") => {
+                    set_attribute(&mut self.orient, attr.parse(value), session)
+                }
+                expanded_name!("", "preserveAspectRatio") => {
+                    set_attribute(&mut self.aspect, attr.parse(value), session)
+                }
+                expanded_name!("", "viewBox") => {
+                    set_attribute(&mut self.vbox, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/paint_server.rs b/src/paint_server.rs
index 07d8dafc..137435f7 100644
--- a/src/paint_server.rs
+++ b/src/paint_server.rs
@@ -147,7 +147,7 @@ impl PaintServer {
 
                     match *node.borrow_element() {
                         Element::LinearGradient(ref g) => {
-                            g.resolve(node, acquired_nodes, opacity, session).map(|g| {
+                            g.resolve(node, acquired_nodes, opacity).map(|g| {
                                 Arc::new(PaintSource::Gradient(
                                     g,
                                     alternate.map(|c| resolve_color(&c, opacity, current_color)),
@@ -163,7 +163,7 @@ impl PaintServer {
                             })
                         }
                         Element::RadialGradient(ref g) => {
-                            g.resolve(node, acquired_nodes, opacity, session).map(|g| {
+                            g.resolve(node, acquired_nodes, opacity).map(|g| {
                                 Arc::new(PaintSource::Gradient(
                                     g,
                                     alternate.map(|c| resolve_color(&c, opacity, current_color)),
diff --git a/src/parsers.rs b/src/parsers.rs
index d15b4d1a..d6b82637 100644
--- a/src/parsers.rs
+++ b/src/parsers.rs
@@ -157,7 +157,7 @@ impl Parse for u32 {
 }
 
 /// Number lists with bounds for the required and maximum number of items.
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct NumberList<const REQUIRED: usize, const MAX: usize>(pub Vec<f64>);
 
 impl<const REQUIRED: usize, const MAX: usize> Parse for NumberList<REQUIRED, MAX> {
diff --git a/src/pattern.rs b/src/pattern.rs
index 6841586e..69b82c6c 100644
--- a/src/pattern.rs
+++ b/src/pattern.rs
@@ -6,7 +6,7 @@ use crate::aspect_ratio::*;
 use crate::coord_units::CoordUnits;
 use crate::document::{AcquiredNode, AcquiredNodes, NodeId, NodeStack};
 use crate::drawing_ctx::ViewParams;
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::error::*;
 use crate::href::{is_href, set_href};
 use crate::length::*;
@@ -33,7 +33,7 @@ struct Common {
     // In that case, the fully resolved pattern will have a .vbox=Some(None) value.
     vbox: Option<Option<ViewBox>>,
     preserve_aspect_ratio: Option<AspectRatio>,
-    transform: Option<Transform>,
+    transform: Option<TransformAttribute>,
     x: Option<Length<Horizontal>>,
     y: Option<Length<Vertical>>,
     width: Option<ULength<Horizontal>>,
@@ -93,7 +93,7 @@ pub struct ResolvedPattern {
     content_units: PatternContentUnits,
     vbox: Option<ViewBox>,
     preserve_aspect_ratio: AspectRatio,
-    transform: Transform,
+    transform: TransformAttribute,
     x: Length<Horizontal>,
     y: Length<Vertical>,
     width: ULength<Horizontal>,
@@ -124,37 +124,52 @@ pub struct Pattern {
 }
 
 impl SetAttributes for Pattern {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "patternUnits") => self.common.units = attr.parse(value)?,
+                expanded_name!("", "patternUnits") => {
+                    set_attribute(&mut self.common.units, attr.parse(value), session)
+                }
                 expanded_name!("", "patternContentUnits") => {
-                    self.common.content_units = attr.parse(value)?
+                    set_attribute(&mut self.common.content_units, attr.parse(value), session);
+                }
+                expanded_name!("", "viewBox") => {
+                    set_attribute(&mut self.common.vbox, attr.parse(value), session)
                 }
-                expanded_name!("", "viewBox") => self.common.vbox = attr.parse(value)?,
                 expanded_name!("", "preserveAspectRatio") => {
-                    self.common.preserve_aspect_ratio = attr.parse(value)?
+                    set_attribute(
+                        &mut self.common.preserve_aspect_ratio,
+                        attr.parse(value),
+                        session,
+                    );
                 }
                 expanded_name!("", "patternTransform") => {
-                    let transform_attr: TransformAttribute = attr.parse(value)?;
-                    self.common.transform = Some(transform_attr.to_transform());
+                    set_attribute(&mut self.common.transform, attr.parse(value), session);
                 }
                 ref a if is_href(a) => {
-                    set_href(
-                        a,
-                        &mut self.fallback,
-                        NodeId::parse(value).attribute(attr.clone())?,
+                    let mut href = None;
+                    set_attribute(
+                        &mut href,
+                        NodeId::parse(value).map(Some).attribute(attr.clone()),
+                        session,
                     );
+                    set_href(a, &mut self.fallback, href);
+                }
+                expanded_name!("", "x") => {
+                    set_attribute(&mut self.common.x, attr.parse(value), session)
+                }
+                expanded_name!("", "y") => {
+                    set_attribute(&mut self.common.y, attr.parse(value), session)
+                }
+                expanded_name!("", "width") => {
+                    set_attribute(&mut self.common.width, attr.parse(value), session)
+                }
+                expanded_name!("", "height") => {
+                    set_attribute(&mut self.common.height, attr.parse(value), session)
                 }
-                expanded_name!("", "x") => self.common.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.common.y = attr.parse(value)?,
-                expanded_name!("", "width") => self.common.width = attr.parse(value)?,
-                expanded_name!("", "height") => self.common.height = attr.parse(value)?,
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -235,7 +250,10 @@ impl UnresolvedPattern {
             .common
             .preserve_aspect_ratio
             .or_else(|| Some(AspectRatio::default()));
-        let transform = self.common.transform.or_else(|| Some(Transform::default()));
+        let transform = self
+            .common
+            .transform
+            .or_else(|| Some(TransformAttribute::default()));
         let x = self.common.x.or_else(|| Some(Default::default()));
         let y = self.common.y.or_else(|| Some(Default::default()));
         let width = self.common.width.or_else(|| Some(Default::default()));
@@ -368,7 +386,9 @@ impl ResolvedPattern {
             ),
         };
 
-        let coord_transform = coord_transform.post_transform(&self.transform);
+        let pattern_transform = self.transform.to_transform();
+
+        let coord_transform = coord_transform.post_transform(&pattern_transform);
 
         // Create the pattern contents coordinate system
         let content_transform = if let Some(vbox) = self.vbox {
@@ -396,7 +416,7 @@ impl ResolvedPattern {
         Some(UserSpacePattern {
             width,
             height,
-            transform: self.transform,
+            transform: pattern_transform,
             coord_transform,
             content_transform,
             opacity: self.opacity,
diff --git a/src/properties.rs b/src/properties.rs
index 810b1ec7..dd48b76a 100644
--- a/src/properties.rs
+++ b/src/properties.rs
@@ -892,11 +892,7 @@ impl SpecifiedValues {
         }
     }
 
-    pub fn parse_presentation_attributes(
-        &mut self,
-        session: &Session,
-        attrs: &Attributes,
-    ) -> Result<(), ElementError> {
+    pub fn parse_presentation_attributes(&mut self, session: &Session, attrs: &Attributes) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "transform") => {
@@ -912,25 +908,41 @@ impl SpecifiedValues {
                     // xml:lang is a non-presentation attribute and as such cannot have the
                     // "inherit" value.  So, we don't call parse_one_presentation_attribute()
                     // for it, but rather call its parser directly.
-                    self.set_parsed_property(&ParsedProperty::XmlLang(SpecifiedValue::Specified(
-                        attr.parse(value)?,
-                    )));
+                    let parse_result: Result<XmlLang, _> = attr.parse(value);
+                    match parse_result {
+                        Ok(lang) => {
+                            self.set_parsed_property(&ParsedProperty::XmlLang(
+                                SpecifiedValue::Specified(lang),
+                            ));
+                        }
+
+                        Err(e) => {
+                            rsvg_log!(session, "ignoring attribute with invalid value: {}", e);
+                        }
+                    }
                 }
 
                 expanded_name!(xml "space") => {
                     // xml:space is a non-presentation attribute and as such cannot have the
                     // "inherit" value.  So, we don't call parse_one_presentation_attribute()
                     // for it, but rather call its parser directly.
-                    self.set_parsed_property(&ParsedProperty::XmlSpace(SpecifiedValue::Specified(
-                        attr.parse(value)?,
-                    )));
+                    let parse_result: Result<XmlSpace, _> = attr.parse(value);
+                    match parse_result {
+                        Ok(space) => {
+                            self.set_parsed_property(&ParsedProperty::XmlSpace(
+                                SpecifiedValue::Specified(space),
+                            ));
+                        }
+
+                        Err(e) => {
+                            rsvg_log!(session, "ignoring attribute with invalid value: {}", e);
+                        }
+                    }
                 }
 
                 _ => self.parse_one_presentation_attribute(session, attr, value),
             }
         }
-
-        Ok(())
     }
 
     pub fn set_property_from_declaration(
@@ -960,7 +972,7 @@ impl SpecifiedValues {
         origin: Origin,
         important_styles: &mut HashSet<QualName>,
         session: &Session,
-    ) -> Result<(), ElementError> {
+    ) {
         let mut input = ParserInput::new(declarations);
         let mut parser = Parser::new(&mut input);
 
@@ -973,8 +985,6 @@ impl SpecifiedValues {
                 }
             })
             .for_each(|decl| self.set_property_from_declaration(&decl, origin, important_styles));
-
-        Ok(())
     }
 }
 
@@ -1118,13 +1128,4 @@ mod tests {
 
         assert_eq!(computed.opacity(), half_opacity.clone());
     }
-
-    #[test]
-    fn empty_style_attribute_parses_ok() {
-        let mut specified = SpecifiedValues::default();
-
-        assert!(specified
-            .parse_style_declarations("", Origin::Author, &mut HashSet::new(), &Session::default())
-            .is_ok())
-    }
 }
diff --git a/src/shapes.rs b/src/shapes.rs
index ad8e3259..c1e7287f 100644
--- a/src/shapes.rs
+++ b/src/shapes.rs
@@ -9,7 +9,7 @@ use std::rc::Rc;
 use crate::bbox::BoundingBox;
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, SetAttributes};
 use crate::error::*;
 use crate::iri::Iri;
 use crate::layout::{Marker, Shape, StackingContext, Stroke};
@@ -255,21 +255,19 @@ pub struct Path {
 impl_draw!(Path);
 
 impl SetAttributes for Path {
-    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             if attr.expanded() == expanded_name!("", "d") {
                 let mut builder = PathBuilder::default();
                 if let Err(e) = builder.parse(value) {
-                    // FIXME: we don't propagate errors upstream, but creating a partial
-                    // path is OK per the spec
+                    // Creating a partial path is OK per the spec; we don't throw away the partial
+                    // result in case of an error.
 
                     rsvg_log!(session, "could not parse path: {}", e);
                 }
                 self.path = Rc::new(builder.into_path());
             }
         }
-
-        Ok(())
     }
 }
 
@@ -346,14 +344,12 @@ pub struct Polygon {
 impl_draw!(Polygon);
 
 impl SetAttributes for Polygon {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             if attr.expanded() == expanded_name!("", "points") {
-                self.points = attr.parse(value)?;
+                set_attribute(&mut self.points, attr.parse(value), session);
             }
         }
-
-        Ok(())
     }
 }
 
@@ -371,14 +367,12 @@ pub struct Polyline {
 impl_draw!(Polyline);
 
 impl SetAttributes for Polyline {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             if attr.expanded() == expanded_name!("", "points") {
-                self.points = attr.parse(value)?;
+                set_attribute(&mut self.points, attr.parse(value), session);
             }
         }
-
-        Ok(())
     }
 }
 
@@ -399,18 +393,16 @@ pub struct Line {
 impl_draw!(Line);
 
 impl SetAttributes for Line {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x1") => self.x1 = attr.parse(value)?,
-                expanded_name!("", "y1") => self.y1 = attr.parse(value)?,
-                expanded_name!("", "x2") => self.x2 = attr.parse(value)?,
-                expanded_name!("", "y2") => self.y2 = attr.parse(value)?,
+                expanded_name!("", "x1") => set_attribute(&mut self.x1, attr.parse(value), session),
+                expanded_name!("", "y1") => set_attribute(&mut self.y1, attr.parse(value), session),
+                expanded_name!("", "x2") => set_attribute(&mut self.x2, attr.parse(value), session),
+                expanded_name!("", "y2") => set_attribute(&mut self.y2, attr.parse(value), session),
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/structure.rs b/src/structure.rs
index 425cdb59..c7a3c729 100644
--- a/src/structure.rs
+++ b/src/structure.rs
@@ -7,7 +7,7 @@ use crate::bbox::BoundingBox;
 use crate::coord_units::CoordUnits;
 use crate::document::{AcquiredNodes, NodeId};
 use crate::drawing_ctx::{ClipMode, DrawingCtx, ViewParams};
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::error::*;
 use crate::href::{is_href, set_href};
 use crate::layout::StackingContext;
@@ -101,7 +101,7 @@ impl Draw for Switch {
             &mut |an, dc, _transform| {
                 if let Some(child) = node.children().filter(|c| c.is_element()).find(|c| {
                     let elt = c.borrow_element();
-                    elt.get_cond(dc.user_language()) && !elt.is_in_error()
+                    elt.get_cond(dc.user_language())
                 }) {
                     child.draw(
                         an,
@@ -274,18 +274,18 @@ impl Svg {
 }
 
 impl SetAttributes for Svg {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "preserveAspectRatio") => {
-                    self.preserve_aspect_ratio = attr.parse(value)?
+                    set_attribute(&mut self.preserve_aspect_ratio, attr.parse(value), session)
+                }
+                expanded_name!("", "viewBox") => {
+                    set_attribute(&mut self.vbox, attr.parse(value), session)
                 }
-                expanded_name!("", "viewBox") => self.vbox = attr.parse(value)?,
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -355,23 +355,29 @@ impl Default for Use {
 }
 
 impl SetAttributes for Use {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                ref a if is_href(a) => set_href(
-                    a,
-                    &mut self.link,
-                    NodeId::parse(value).attribute(attr.clone())?,
-                ),
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "width") => self.width = attr.parse(value)?,
-                expanded_name!("", "height") => self.height = attr.parse(value)?,
+                ref a if is_href(a) => {
+                    let mut href = None;
+                    set_attribute(
+                        &mut href,
+                        NodeId::parse(value).map(Some).attribute(attr.clone()),
+                        session,
+                    );
+                    set_href(a, &mut self.link, href);
+                }
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "width") => {
+                    set_attribute(&mut self.width, attr.parse(value), session)
+                }
+                expanded_name!("", "height") => {
+                    set_attribute(&mut self.height, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -441,18 +447,18 @@ impl Symbol {
 }
 
 impl SetAttributes for Symbol {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
                 expanded_name!("", "preserveAspectRatio") => {
-                    self.preserve_aspect_ratio = attr.parse(value)?
+                    set_attribute(&mut self.preserve_aspect_ratio, attr.parse(value), session)
+                }
+                expanded_name!("", "viewBox") => {
+                    set_attribute(&mut self.vbox, attr.parse(value), session)
                 }
-                expanded_name!("", "viewBox") => self.vbox = attr.parse(value)?,
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -472,16 +478,12 @@ impl ClipPath {
 }
 
 impl SetAttributes for ClipPath {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
-        let result = attrs
-            .iter()
-            .find(|(attr, _)| attr.expanded() == expanded_name!("", "clipPathUnits"))
-            .and_then(|(attr, value)| attr.parse(value).ok());
-        if let Some(units) = result {
-            self.units = units
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
+        for (attr, value) in attrs.iter() {
+            if attr.expanded() == expanded_name!("", "clipPathUnits") {
+                set_attribute(&mut self.units, attr.parse(value), session);
+            }
         }
-
-        Ok(())
     }
 }
 
@@ -535,20 +537,26 @@ impl Mask {
 }
 
 impl SetAttributes for Mask {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "width") => self.width = attr.parse(value)?,
-                expanded_name!("", "height") => self.height = attr.parse(value)?,
-                expanded_name!("", "maskUnits") => self.units = attr.parse(value)?,
-                expanded_name!("", "maskContentUnits") => self.content_units = attr.parse(value)?,
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "width") => {
+                    set_attribute(&mut self.width, attr.parse(value), session)
+                }
+                expanded_name!("", "height") => {
+                    set_attribute(&mut self.height, attr.parse(value), session)
+                }
+                expanded_name!("", "maskUnits") => {
+                    set_attribute(&mut self.units, attr.parse(value), session)
+                }
+                expanded_name!("", "maskContentUnits") => {
+                    set_attribute(&mut self.content_units, attr.parse(value), session)
+                }
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -560,15 +568,13 @@ pub struct Link {
 }
 
 impl SetAttributes for Link {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) {
         for (attr, value) in attrs.iter() {
-            match attr.expanded() {
-                ref a if is_href(a) => set_href(a, &mut self.link, value.to_owned()),
-                _ => (),
+            let expanded = attr.expanded();
+            if is_href(&expanded) {
+                set_href(&expanded, &mut self.link, Some(value.to_owned()));
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/style.rs b/src/style.rs
index 8c3028e9..5c1ffb39 100644
--- a/src/style.rs
+++ b/src/style.rs
@@ -2,7 +2,7 @@
 
 use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
-use crate::element::{Draw, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, SetAttributes};
 use crate::error::*;
 use crate::session::Session;
 use crate::xml::Attributes;
@@ -54,14 +54,16 @@ impl Style {
 }
 
 impl SetAttributes for Style {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             if attr.expanded() == expanded_name!("", "type") {
-                self.type_ = StyleType::parse(value).attribute(attr)?;
+                set_attribute(
+                    &mut self.type_,
+                    StyleType::parse(value).attribute(attr),
+                    session,
+                );
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/text.rs b/src/text.rs
index 7b642975..dceae602 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -10,7 +10,7 @@ use std::sync::Arc;
 use crate::bbox::BoundingBox;
 use crate::document::{AcquiredNodes, NodeId};
 use crate::drawing_ctx::{create_pango_context, DrawingCtx, FontOptions, ViewParams};
-use crate::element::{Draw, Element, ElementResult, SetAttributes};
+use crate::element::{set_attribute, Draw, Element, SetAttributes};
 use crate::error::*;
 use crate::layout::{self, FontProperties, StackingContext, Stroke, TextSpan};
 use crate::length::*;
@@ -733,18 +733,16 @@ impl Text {
 }
 
 impl SetAttributes for Text {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "dx") => self.dx = attr.parse(value)?,
-                expanded_name!("", "dy") => self.dy = attr.parse(value)?,
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "dx") => set_attribute(&mut self.dx, attr.parse(value), session),
+                expanded_name!("", "dy") => set_attribute(&mut self.dy, attr.parse(value), session),
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
@@ -926,7 +924,7 @@ fn extract_chars_children_to_chunks_recursively(
 }
 
 impl SetAttributes for TRef {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) {
         self.link = attrs
             .iter()
             .find(|(attr, _)| attr.expanded() == expanded_name!(xlink "href"))
@@ -934,8 +932,6 @@ impl SetAttributes for TRef {
             // the <tref> element got removed in SVG2.  So, here we still use a match
             // against the full namespaced version of the attribute.
             .and_then(|(attr, value)| NodeId::parse(value).attribute(attr).ok());
-
-        Ok(())
     }
 }
 
@@ -994,18 +990,16 @@ impl TSpan {
 }
 
 impl SetAttributes for TSpan {
-    fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult {
+    fn set_attributes(&mut self, attrs: &Attributes, session: &Session) {
         for (attr, value) in attrs.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => self.x = attr.parse(value)?,
-                expanded_name!("", "y") => self.y = attr.parse(value)?,
-                expanded_name!("", "dx") => self.dx = attr.parse(value)?,
-                expanded_name!("", "dy") => self.dy = attr.parse(value)?,
+                expanded_name!("", "x") => set_attribute(&mut self.x, attr.parse(value), session),
+                expanded_name!("", "y") => set_attribute(&mut self.y, attr.parse(value), session),
+                expanded_name!("", "dx") => set_attribute(&mut self.dx, attr.parse(value), session),
+                expanded_name!("", "dy") => set_attribute(&mut self.dy, attr.parse(value), session),
                 _ => (),
             }
         }
-
-        Ok(())
     }
 }
 
diff --git a/src/transform.rs b/src/transform.rs
index 6826d905..f99f8a05 100644
--- a/src/transform.rs
+++ b/src/transform.rs
@@ -45,7 +45,7 @@ pub enum TransformProperty {
 /// The `transform` attribute from SVG1.1
 ///
 /// SVG1.1: <https://www.w3.org/TR/SVG11/coords.html#TransformAttribute>
-#[derive(Default, Debug, Clone, PartialEq)]
+#[derive(Copy, Clone, Default, Debug, PartialEq)]
 pub struct TransformAttribute(Transform);
 
 impl Default for TransformProperty {
@@ -535,7 +535,7 @@ impl Default for Transform {
 }
 
 impl TransformAttribute {
-    pub fn to_transform(&self) -> Transform {
+    pub fn to_transform(self) -> Transform {
         self.0
     }
 }
diff --git a/tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive-ref.svg b/tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive-ref.svg
new file mode 100644
index 00000000..859a6973
--- /dev/null
+++ b/tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive-ref.svg
@@ -0,0 +1,7 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="400" height="400">
+  <g>
+    <rect x="100" y="100" width="50" height="50" fill="lime"/>
+    <rect x="200" y="100" width="50" height="50" fill="lime"/>
+  </g>
+</svg>
diff --git a/tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive.svg b/tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive.svg
new file mode 100644
index 00000000..e1a276e3
--- /dev/null
+++ b/tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive.svg
@@ -0,0 +1,19 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="400" height="400">
+  <filter id="filter" filterUnits="objectBoundingBox" x="-10%" y="-10%" width="120%" height="120%">
+    <!-- First, swap the red and green channels (turn the red rectangle into green) -->
+    <feColorMatrix type="matrix"
+                   values="0 1 0 0 0
+                           1 0 0 0 0
+                           0 0 1 0 0
+                           0 0 0 1 0"/>
+
+    <!-- Second, no-op blur because stdDeviation is negative-->
+    <feGaussianBlur stdDeviation="-1 -1"/>
+  </filter>
+
+  <g filter="url(#filter)">
+    <rect x="100" y="100" width="50" height="50" fill="red"/>
+    <rect x="200" y="100" width="50" height="50" fill="red"/>
+  </g>
+</svg>
diff --git a/tests/src/reference.rs b/tests/src/reference.rs
index 46fb9dc2..2a79efe3 100644
--- a/tests/src/reference.rs
+++ b/tests/src/reference.rs
@@ -397,6 +397,12 @@ test_svg_reference!(
 );
 
 test_svg_reference!(
+    gaussian_blur_nonpositive_913,
+    "tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive.svg",
+    "tests/fixtures/reftests/svg2-reftests/913-gaussian-blur-nonpositive-ref.svg"
+);
+
+test_svg_reference!(
     bug_880_horizontal_vertical_stroked_lines,
     "tests/fixtures/reftests/bugs-reftests/880-stroke-wide-line.svg",
     "tests/fixtures/reftests/bugs-reftests/880-stroke-wide-line-ref.svg"