[librsvg: 1/3] document: accumulate all the style text before parsing it

[Federico Mena Quintero <federico src gnome org>]

commit 325b9de9ffacceeaf19e07e1e22175b9d251c166
Author: Paolo Borelli <pborelli gnome org>
Date:   Sun Nov 3 09:55:18 2019 +0100

    document: accumulate all the style text before parsing it
    
    Fixes a regression introduced in the recent refactoring.
    While at it, improve the style type attribute parsing so that
    invalid values are detected during the parsing of attributes.
    A corresponding test case is included.

 rsvg_internals/src/document.rs                     |  17 +++++-----
 rsvg_internals/src/style.rs                        |  35 ++++++++++++++++++---
 .../reftests/style-with-xml-comments-ref.png       | Bin 0 -> 93 bytes
 .../fixtures/reftests/style-with-xml-comments.svg  |  10 ++++++
 4 files changed, 48 insertions(+), 14 deletions(-)
---
diff --git a/rsvg_internals/src/document.rs b/rsvg_internals/src/document.rs
index f65e2abc..9d2fa2cd 100644
--- a/rsvg_internals/src/document.rs
+++ b/rsvg_internals/src/document.rs
@@ -16,7 +16,7 @@ use crate::node::{NodeCascade, NodeData, NodeType, RsvgNode};
 use crate::properties::ComputedValues;
 use crate::property_bag::PropertyBag;
 use crate::structure::{IntrinsicDimensions, Svg};
-use crate::style::Style;
+use crate::style::{Style, StyleType};
 use crate::surface_utils::shared_surface::SharedImageSurface;
 use crate::text::NodeChars;
 use crate::xml::xml_load_from_possibly_compressed_stream;
@@ -218,6 +218,7 @@ pub struct DocumentBuilder {
     load_options: LoadOptions,
     tree: Option<RsvgNode>,
     ids: HashMap<String, RsvgNode>,
+    inline_css: String,
     stylesheets: Vec<Stylesheet>,
     css_rules: CssRules,
 }
@@ -228,6 +229,7 @@ impl DocumentBuilder {
             load_options: load_options.clone(),
             tree: None,
             ids: HashMap::new(),
+            inline_css: String::new(),
             stylesheets: Vec::new(),
             css_rules: CssRules::default(),
         }
@@ -281,8 +283,8 @@ impl DocumentBuilder {
         }
 
         if parent.borrow().get_type() == NodeType::Style {
-            if parent.borrow().get_impl::<Style>().is_text_css() {
-                self.parse_css(text);
+            if parent.borrow().get_impl::<Style>().style_type() == StyleType::TextCss {
+                self.inline_css.push_str(text);
             }
         } else {
             self.append_chars_to_parent(text, parent);
@@ -323,11 +325,6 @@ impl DocumentBuilder {
             .map_err(|_| LoadingError::BadUrl)
     }
 
-    fn parse_css(&mut self, css_data: &str) {
-        self.css_rules
-            .parse(self.load_options.base_url.as_ref(), css_data);
-    }
-
     pub fn build(mut self) -> Result<Document, LoadingError> {
         for s in self.stylesheets.iter() {
             if s.type_.as_ref().map(String::as_str) != Some("text/css")
@@ -341,6 +338,8 @@ impl DocumentBuilder {
             let _ = self.css_rules.load_css(&self.resolve_href(s.href.as_ref().unwrap())?);
         }
 
+        self.css_rules.parse(self.load_options.base_url.as_ref(), &self.inline_css);
+
         let DocumentBuilder { load_options, tree, ids, css_rules, .. } = self;
 
         match tree {
@@ -356,7 +355,7 @@ impl DocumentBuilder {
 
                     Ok(Document {
                         tree: root.clone(),
-                        ids: ids,
+                        ids,
                         externs: RefCell::new(Resources::new()),
                         images: RefCell::new(Images::new()),
                         load_options: load_options.clone(),
diff --git a/rsvg_internals/src/style.rs b/rsvg_internals/src/style.rs
index 59247a83..d67836bd 100644
--- a/rsvg_internals/src/style.rs
+++ b/rsvg_internals/src/style.rs
@@ -1,19 +1,45 @@
+use cssparser::Parser;
 use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
+use crate::error::*;
 use crate::node::{NodeResult, NodeTrait, RsvgNode};
+use crate::parsers::{Parse, ParseError, ParseValue};
 use crate::property_bag::PropertyBag;
 
+/// Represents the syntax used in the <style> node.
+///
+/// Currently only "text/css" is supported.
+#[derive(Copy, Clone, PartialEq)]
+pub enum StyleType {
+    TextCss,
+}
+
+impl Parse for StyleType {
+    type Err = ValueErrorKind;
+
+    fn parse(parser: &mut Parser<'_, '_>) -> Result<StyleType, ValueErrorKind> {
+        parser
+            .expect_ident_matching("text/css")
+            .and_then(|_| Ok(StyleType::TextCss))
+            .map_err(|_| {
+                ValueErrorKind::Parse(ParseError::new(
+                    "only the \"text/css\" style type is supported",
+                ))
+            })
+    }
+}
+
 /// Represents a <style> node.
 ///
 /// It does not render itself, and just holds CSS stylesheet information for the rest of
 /// the code to use.
 #[derive(Default)]
 pub struct Style {
-    type_: Option<String>,
+    type_: Option<StyleType>,
 }
 
 impl Style {
-    pub fn is_text_css(&self) -> bool {
+    pub fn style_type(&self) -> StyleType {
         // FIXME: See these:
         //
         // https://www.w3.org/TR/SVG11/styling.html#StyleElementTypeAttribute
@@ -22,8 +48,7 @@ impl Style {
         // If the "type" attribute is not present, we should fallback to the
         // "contentStyleType" attribute of the svg element, which in turn
         // defaults to "text/css".
-
-        self.type_.as_ref().map(|t| t == "text/css").unwrap_or(true)
+        self.type_.unwrap_or(StyleType::TextCss)
     }
 }
 
@@ -31,7 +56,7 @@ impl NodeTrait for Style {
     fn set_atts(&mut self, _: Option<&RsvgNode>, pbag: &PropertyBag<'_>) -> NodeResult {
         for (attr, value) in pbag.iter() {
             if attr.expanded() == expanded_name!(svg "type") {
-                self.type_ = Some(value.to_string());
+                self.type_ = Some(attr.parse(value)?);
             }
         }
 
diff --git a/tests/fixtures/reftests/style-with-xml-comments-ref.png 
b/tests/fixtures/reftests/style-with-xml-comments-ref.png
new file mode 100644
index 00000000..8aa2a9e1
Binary files /dev/null and b/tests/fixtures/reftests/style-with-xml-comments-ref.png differ
diff --git a/tests/fixtures/reftests/style-with-xml-comments.svg 
b/tests/fixtures/reftests/style-with-xml-comments.svg
new file mode 100644
index 00000000..6812f880
--- /dev/null
+++ b/tests/fixtures/reftests/style-with-xml-comments.svg
@@ -0,0 +1,10 @@
+<svg xmlns="http://www.w3.org/2000/svg"; width="10px" height="10px">
+  <defs>
+    <style type="text/css">
+      rect {
+        fill: <!-- yellow --> red;
+      }
+    </style>
+  </defs>
+  <rect width="10" height="10"/>
+</svg>