Merge pull request #111 from Ede123/property_inheritance

Fix incorrect property inheritance

scour/scour.py

@@ -1456,7 +1456,7 @@ def repairStyle(node, options):
             for uselessStyle in ['fill', 'fill-opacity', 'fill-rule', 'stroke', 'stroke-linejoin',
                'stroke-opacity', 'stroke-miterlimit', 'stroke-linecap', 'stroke-dasharray',
                'stroke-dashoffset', 'stroke-opacity'] :
-               if uselessStyle in styleMap:
+               if uselessStyle in styleMap and not styleInheritedByChild(node, uselessStyle):
                   del styleMap[uselessStyle]
                   num += 1
 
@@ -1465,17 +1465,19 @@ def repairStyle(node, options):
       if 'stroke' in styleMap and styleMap['stroke'] == 'none' :
          for strokestyle in [ 'stroke-width', 'stroke-linejoin', 'stroke-miterlimit',
                'stroke-linecap', 'stroke-dasharray', 'stroke-dashoffset', 'stroke-opacity'] :
-            if strokestyle in styleMap :
+            if strokestyle in styleMap and not styleInheritedByChild(node, strokestyle):
                del styleMap[strokestyle]
                num += 1
-         # TODO: This is actually a problem if a parent element has a specified stroke
          # we need to properly calculate computed values
+         if not styleInheritedByChild(node, 'stroke'):
+            if styleInheritedFromParent(node, 'stroke') in [None, 'none']:
                del styleMap['stroke']
+               num += 1
 
       #  if fill:none, then remove all fill-related properties (fill-rule, etc)
       if 'fill' in styleMap and styleMap['fill'] == 'none' :
          for fillstyle in [ 'fill-rule', 'fill-opacity' ] :
-            if fillstyle in styleMap :
+            if fillstyle in styleMap and not styleInheritedByChild(node, fillstyle):
                del styleMap[fillstyle]
                num += 1
 
@@ -1484,7 +1486,7 @@ def repairStyle(node, options):
          fillOpacity = float(styleMap['fill-opacity'])
          if fillOpacity == 0.0 :
             for uselessFillStyle in [ 'fill', 'fill-rule' ] :
-               if uselessFillStyle in styleMap:
+               if uselessFillStyle in styleMap and not styleInheritedByChild(node, uselessFillStyle):
                   del styleMap[uselessFillStyle]
                   num += 1
 
@@ -1494,7 +1496,7 @@ def repairStyle(node, options):
          if strokeOpacity == 0.0 :
             for uselessStrokeStyle in [ 'stroke', 'stroke-width', 'stroke-linejoin', 'stroke-linecap',
                      'stroke-dasharray', 'stroke-dashoffset' ] :
-               if uselessStrokeStyle in styleMap:
+               if uselessStrokeStyle in styleMap and not styleInheritedByChild(node, uselessStrokeStyle):
                   del styleMap[uselessStrokeStyle]
                   num += 1
 
@@ -1504,7 +1506,7 @@ def repairStyle(node, options):
          if strokeWidth.value == 0.0 :
             for uselessStrokeStyle in [ 'stroke', 'stroke-linejoin', 'stroke-linecap',
                      'stroke-dasharray', 'stroke-dashoffset', 'stroke-opacity' ] :
-               if uselessStrokeStyle in styleMap:
+               if uselessStrokeStyle in styleMap and not styleInheritedByChild(node, uselessStrokeStyle):
                   del styleMap[uselessStrokeStyle]
                   num += 1
 
@@ -1562,6 +1564,83 @@ def repairStyle(node, options):
 
    return num
 
+def styleInheritedFromParent(node, style):
+   """
+   Returns the value of 'style' that is inherited from the parents of the passed-in node
+   
+   Warning: This method only considers presentation attributes and inline styles,
+            any style sheets are ignored!
+   """
+   parentNode = node.parentNode;
+
+   # return None if we reached the Document element
+   if parentNode.nodeType == 9:
+      return None
+
+   # check styles first (they take precedence over presentation attributes)
+   styles = _getStyle(parentNode)
+   if style in styles.keys():
+      value = styles[style]
+      if not value == 'inherit':
+         return value
+      
+   # check attributes
+   value = parentNode.getAttribute(style)
+   if value not in ['', 'inherit']:
+      return parentNode.getAttribute(style)
+
+   # check the next parent recursively if we did not find a value yet
+   return styleInheritedFromParent(parentNode, style)
+
+def styleInheritedByChild(node, style, nodeIsChild=False):
+   """
+   Returns whether 'style' is inherited by any children of the passed-in node
+
+   If False is returned, it is guaranteed that 'style' can safely be removed
+   from the passed-in node without influencing visual output of it's children
+
+   If True is returned, the passed-in node should not have its text-based
+   attributes removed.
+   
+   Warning: This method only considers presentation attributes and inline styles,
+            any style sheets are ignored!
+   """
+   # Comment, text and CDATA nodes don't have attributes and aren't containers so they can't inherit attributes
+   if node.nodeType != 1:
+      return False
+
+   
+   if nodeIsChild:
+      # if the current child node sets a new value for 'style'
+      # we can stop the search in the current branch of the DOM tree
+
+      # check attributes
+      if node.getAttribute(style) not in ['', 'inherit']:
+         return False
+      # check styles
+      styles = _getStyle(node)
+      if (style in styles.keys()) and not (styles[style] == 'inherit'):
+         return False
+   else:
+      # if the passed-in node does not have any children 'style' can obviously not be inherited
+      if not node.childNodes:
+         return False
+
+   # If we have child nodes recursively check those
+   if node.childNodes:
+      for child in node.childNodes:
+         if styleInheritedByChild(child, style, True):
+            return True
+
+   # If the current element is a container element the inherited style is meaningless
+   # (since we made sure it's not inherited by any of its children)
+   if node.nodeName in ['a', 'defs', 'glyph', 'g', 'marker', 'mask', 'missing-glyph', 'pattern', 'svg', 'switch', 'symbol']:
+      return False
+
+   # in all other cases we have to assume the inherited value of 'style' is meaningfull and has to be kept
+   # (e.g nodes without children at the end of the DOM tree, text nodes, ...)
+   return True
+
 def mayContainTextNodes(node):
    """
    Returns True if the passed-in node is probably a text element, or at least

testscour.py

@@ -495,12 +495,30 @@ class RemoveStrokeWhenStrokeNone(unittest.TestCase):
 		self.assertEqual(doc.getElementsByTagNameNS(SVGNS, 'path')[0].getAttribute('stroke'), '',
 			'stroke attribute not emptied when no stroke' )
 
+class KeepStrokeWhenInheritedFromParent(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/stroke-none.svg')
+		self.assertEqual(doc.getElementById('p1').getAttribute('stroke'), 'none',
+			'stroke attribute removed despite a different value being inherited from a parent' )
+
+class KeepStrokeWhenInheritedByChild(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/stroke-none.svg')
+		self.assertEqual(doc.getElementById('g2').getAttribute('stroke'), 'none',
+			'stroke attribute removed despite it being inherited by a child' )
+
 class RemoveStrokeWidthWhenStrokeNone(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/stroke-none.svg')
 		self.assertEqual(doc.getElementsByTagNameNS(SVGNS, 'path')[0].getAttribute('stroke-width'), '',
 			'stroke-width attribute not emptied when no stroke' )
 
+class KeepStrokeWidthWhenInheritedByChild(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/stroke-none.svg')
+		self.assertEqual(doc.getElementById('g3').getAttribute('stroke-width'), '1px',
+			'stroke-width attribute removed despite it being inherited by a child' )
+
 class RemoveStrokeOpacityWhenStrokeNone(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/stroke-none.svg')

unittests/stroke-none.svg

@@ -1,4 +1,16 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <svg xmlns="http://www.w3.org/2000/svg">
  <path id="p" fill="black" style="stroke: none; stroke-width: 1px; stroke-linecap: butt; stroke-dasharray: none; stroke-dashoffset: 2; stroke-linejoin: miter; stroke-opacity: 1;" d="M 7.7592046,36.982095 C 7.8831049,40.873696 7.8339808,45.305308 7.8339808,49.436888  Z" />
+
+ <g id="g1" style="stroke:#000">
+  <path id="p0" d="M 7.7592046,36.982095 C 7.8831049,40.873696 7.8339808,45.305308 7.8339808,49.436888  Z" />
+  <path id="p1" style="stroke:none" d="M 7.7592046,36.982095 C 7.8831049,40.873696 7.8339808,45.305308 7.8339808,49.436888  Z" />
+  <g id="g2" style="stroke:none">
+   <path id="p2" d="M 7.7592046,36.982095 C 7.8831049,40.873696 7.8339808,45.305308 7.8339808,49.436888  Z" />
+  </g>
+ </g>
+
+ <g id="g3" style="stroke:none;stroke-width:1px">
+  <path id="p3" style="stroke:#000" d="M 7.7592046,36.982095 C 7.8831049,40.873696 7.8339808,45.305308 7.8339808,49.436888  Z" />
+ </g>
 </svg>