SystemLanguage: turn into an enum; store Valid and Invalid variants

Previously, parsing an invalid BCP47 language tag would be reported as
an Err(), and the corresponding element inside a <switch> would be set
"in error".  Then, the <switch> implementation would detect that the
child element is in error and ignore it.

We are switching to storing the error condition in the SystemLanguage itself.

Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/763>

2 files changed, 55 insertions, 36 deletions

diff --git a/src/cond.rs b/src/cond.rs
index 99fc99c8..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] = &[];
@@ -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,
+        }
     }
 }
 
@@ -160,66 +185,58 @@ mod tests {
 
     #[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 958665fd..f868dc59 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -130,7 +130,7 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
         };
 
         let mut set_attributes = || -> Result<(), ElementError> {
-            e.set_conditional_processing_attributes()?;
+            e.set_conditional_processing_attributes(session)?;
             e.set_presentation_attributes(session);
             Ok(())
         };
@@ -192,7 +192,10 @@ 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,
+    ) -> Result<(), ElementError> {
         for (attr, value) in self.attributes.iter() {
             match attr.expanded() {
                 expanded_name!("", "requiredExtensions") => {
@@ -204,8 +207,7 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
                 }
 
                 expanded_name!("", "systemLanguage") => {
-                    self.system_language =
-                        Some(SystemLanguage::from_attribute(value).attribute(attr)?);
+                    self.system_language = Some(SystemLanguage::from_attribute(value, session));
                 }
 
                 _ => {}