Attributes with inherit or currentColor and without a proper pare…

…nt is an error now.

Closes #120

CHANGELOG.md

@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 ### Added
 - `--list-separator`.
 
+### Changed
+- Attributes with `inherit` or `currentColor` and without a proper parent is an error now.
+
 ### Fixed
 - Groups removing with transform and non-SVG child.
 - Transform to path applying when a path has a style defined in a parent element.

src/cleaner.rs

@@ -60,8 +60,11 @@ pub fn parse_data(data: &str, opt: &ParseOptions) -> Result<Document, svgdom::Er
  Document::from_str_with_opt(data, opt)
 }
 
-pub fn clean_doc(doc: &mut Document, options: &CleaningOptions, opt: &WriteOptions)
- -> Result<(), error::Error> {
+pub fn clean_doc(
+ doc: &mut Document,
+ options: &CleaningOptions,
+ opt: &WriteOptions
+) -> Result<(), error::Error> {
  preclean_checks(doc)?;
 
  // NOTE: Order is important.
@@ -74,7 +77,7 @@ pub fn clean_doc(doc: &mut Document, options: &CleaningOptions, opt: &WriteOptio
  resolve_radial_gradient_attributes(doc);
  resolve_stop_attributes(doc)?;
 
- resolve_inherit(doc);
+ resolve_inherit(doc)?;
  fix_invalid_attributes(doc);
  group_defs(doc);
 

src/task/resolve_inherit.rs

@@ -3,19 +3,21 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use svgdom::{
- Attribute,
  AttributeId,
  AttributeValue,
  Document,
  Node,
  ValueId,
 };
 
+use error::{
+ ErrorKind,
+ Result,
+};
+
 // TODO: split
 /// Resolve `inherit` and `currentColor` attributes.
-///
-/// The function will fallback to a default value when possible.
-pub fn resolve_inherit(doc: &Document) {
+pub fn resolve_inherit(doc: &Document) -> Result<()> {
  let mut vec_inherit = Vec::new();
  let mut vec_curr_color = Vec::new();
 
@@ -41,35 +43,32 @@ pub fn resolve_inherit(doc: &Document) {
  }
 
  for id in &vec_inherit {
- resolve_impl(&mut node, *id, *id);
+ resolve_impl(&mut node, *id, *id)?;
  }
 
  for id in &vec_curr_color {
  let av = node.attributes().get_value(AttributeId::Color).cloned();
  if let Some(av) = av {
  node.set_attribute((*id, av.clone()));
  } else {
- resolve_impl(&mut node, *id, AttributeId::Color);
+ resolve_impl(&mut node, *id, AttributeId::Color)?;
  }
  }
  }
+
+ Ok(())
 }
 
-fn resolve_impl(node: &mut Node, curr_attr: AttributeId, parent_attr: AttributeId) {
+fn resolve_impl(node: &mut Node, curr_attr: AttributeId, parent_attr: AttributeId) -> Result<()> {
  if let Some(n) = node.parents().find(|n| n.has_attribute(parent_attr)) {
  let av = n.attributes().get_value(parent_attr).cloned();
  if let Some(av) = av {
  node.set_attribute((curr_attr, av.clone()));
  }
+
+ Ok(())
  } else {
- match Attribute::default(curr_attr) {
- Some(a) => node.set_attribute((curr_attr, a.value)),
- None => {
- warn!("Failed to resolve attribute: {}. Removing it.",
- node.attributes().get(curr_attr).unwrap());
- node.remove_attribute(curr_attr);
- }
- }
+ Err(ErrorKind::UnresolvedAttribute(curr_attr.to_string()).into())
  }
 }
 
@@ -78,9 +77,26 @@ mod tests {
  use super::*;
  use svgdom::{Document, ToStringWithOptions};
 
+ #[cfg(test)]
  macro_rules! test {
  ($name:ident, $in_text:expr, $out_text:expr) => (
- base_test!($name, resolve_inherit, $in_text, $out_text);
+ #[test]
+ fn $name() {
+ let mut doc = Document::from_str($in_text).unwrap();
+ resolve_inherit(&mut doc).unwrap();
+ assert_eq_text!(doc.to_string_with_opt(&write_opt_for_tests!()), $out_text);
+ }
+ )
+ }
+
+ #[cfg(test)]
+ macro_rules! test_err {
+ ($name:ident, $in_text:expr) => (
+ #[test]
+ fn $name() {
+ let mut doc = Document::from_str($in_text).unwrap();
+ assert_eq!(resolve_inherit(&mut doc).is_err(), true);
+ }
  )
  }
 
@@ -113,15 +129,6 @@ mod tests {
 "<svg fill='#ff0000' stroke='#00ff00'>
  <rect fill='#ff0000' stroke='#00ff00'/>
 </svg>
-");
-
- test!(inherit_4,
-"<svg>
- <rect fill='inherit'/>
-</svg>",
-"<svg>
- <rect fill='#000000'/>
-</svg>
 ");
 
  test!(current_color_1,
@@ -151,19 +158,20 @@ mod tests {
 </svg>
 ");
 
- test!(default_1,
+ test_err!(unresolvable_1,
 "<svg>
- <rect fill='currentColor'/>
  <rect fill='inherit'/>
-</svg>",
+</svg>"
+);
+
+ test_err!(unresolvable_2,
 "<svg>
- <rect fill='#000000'/>
- <rect fill='#000000'/>
-</svg>
-");
+ <rect fill='currentColor'/>
+ <rect fill='inherit'/>
+</svg>"
+);
 
- test!(unresolved_1,
-"<svg font-family='inherit'/>",
-"<svg/>
-");
+ test_err!(unresolvable_3,
+"<svg font-family='inherit'/>"
+);
 }