Apply a modified patch submitted by Jan Thor in bug 717254 to fix a bug whereby Scour would keep the <defs/> element if it was there but had only whitespace or unreferenced children.

scour.py

@@ -596,8 +596,9 @@ def removeUnreferencedElements(doc):
 	"""
 	global numElemsRemoved
 	num = 0
+	
+	# Remove certain unreferenced elements outside of defs
 	removeTags = ['linearGradient', 'radialGradient', 'pattern']
-
 	identifiedElements = findElementsWithId(doc.documentElement)
 	referencedIDs = findReferencedElements(doc.documentElement)
 
@@ -609,8 +610,7 @@ def removeUnreferencedElements(doc):
 				num += 1
 				numElemsRemoved += 1
 
-	# TODO: should also go through defs and vacuum it
+	# Remove most unreferenced elements inside defs
-	num = 0
 	defs = doc.documentElement.getElementsByTagName('defs')
 	for aDef in defs:
 		elemsToRemove = removeUnusedDefs(doc, aDef)
@@ -2784,15 +2784,21 @@ def scourString(in_string, options=None):
 	if options.remove_metadata:
 		removeMetadataElements(doc)
 	
+	# remove unreferenced gradients/patterns outside of defs
+	# and most unreferenced elements inside of defs
+	while removeUnreferencedElements(doc) > 0:
+		pass
+
 	# remove empty defs, metadata, g
-	# NOTE: these elements will be removed even if they have (invalid) text nodes
+	# NOTE: these elements will be removed if they just have whitespace-only text nodes
-	elemsToRemove = []
 	for tag in ['defs', 'metadata', 'g'] :
 		for elem in doc.documentElement.getElementsByTagName(tag) :
 			removeElem = not elem.hasChildNodes()
 			if removeElem == False :
 				for child in elem.childNodes :
-					if child.nodeType in [1, 3, 4, 8] :
+					if child.nodeType in [1, 4, 8]:
+						break
+					elif child.nodeType == 3 and not child.nodeValue.isspace():
 						break
 				else:
 					removeElem = True
@@ -2800,10 +2806,6 @@ def scourString(in_string, options=None):
 				elem.parentNode.removeChild(elem)
 				numElemsRemoved += 1
 
-	# remove unreferenced gradients/patterns outside of defs
-	while removeUnreferencedElements(doc) > 0:
-		pass
-
 	if options.strip_ids:
 		bContinueLooping = True
 		while bContinueLooping:

testscour.py

@@ -1148,20 +1148,20 @@ class StyleToAttr(unittest.TestCase):
 class PathEmptyMove(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/path-empty-move.svg')
-		self.assertEquals(doc.getElementsByTagName('path')[0].getAttribute('d'), 'm100 100l200 100z');
+		self.assertEquals(doc.getElementsByTagName('path')[0].getAttribute('d'), 'm100 100l200 100z')
-		self.assertEquals(doc.getElementsByTagName('path')[1].getAttribute('d'), 'm100 100v200l100 100z');
+		self.assertEquals(doc.getElementsByTagName('path')[1].getAttribute('d'), 'm100 100v200l100 100z')
 
 class DefaultsRemovalToplevel(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[1].getAttribute('fill-rule'), '',
-			'Default attribute fill-rule:nonzero not removed');
+			'Default attribute fill-rule:nonzero not removed')
 
 class DefaultsRemovalToplevelInverse(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[0].getAttribute('fill-rule'), 'evenodd',
-			'Non-Default attribute fill-rule:evenodd removed');
+			'Non-Default attribute fill-rule:evenodd removed')
 
 class DefaultsRemovalToplevelFormat(unittest.TestCase):
 	def runTest(self):
@@ -1173,37 +1173,49 @@ class DefaultsRemovalInherited(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[3].getAttribute('fill-rule'), '',
-			'Default attribute fill-rule:nonzero not removed in child');
+			'Default attribute fill-rule:nonzero not removed in child')
 
 class DefaultsRemovalInheritedInverse(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[2].getAttribute('fill-rule'), 'evenodd',
-			'Non-Default attribute fill-rule:evenodd removed in child');
+			'Non-Default attribute fill-rule:evenodd removed in child')
 
 class DefaultsRemovalInheritedFormat(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[2].getAttribute('stroke-width'), '',
-			'Default attribute stroke-width:1.00 not removed in child');
+			'Default attribute stroke-width:1.00 not removed in child')
 
 class DefaultsRemovalOverwrite(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[5].getAttribute('fill-rule'), 'nonzero',
-			'Default attribute removed, although it overwrites parent element');
+			'Default attribute removed, although it overwrites parent element')
 
 class DefaultsRemovalOverwriteMarker(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[4].getAttribute('marker-start'), 'none',
-			'Default marker attribute removed, although it overwrites parent element');
+			'Default marker attribute removed, although it overwrites parent element')
 
 class DefaultsRemovalNonOverwrite(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg')
 		self.assertEquals(doc.getElementsByTagName('path')[10].getAttribute('fill-rule'), '',
-			'Default attribute not removed, although its parent used default');
+			'Default attribute not removed, although its parent used default')
+
+class RemoveDefsWithUnreferencedElements(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/useless-defs.svg')
+		self.assertEquals(doc.getElementsByTagName('defs').length, 0,
+			'Kept defs, although it contains only unreferenced elements')
+
+class RemoveDefsWithWhitespace(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/whitespace-defs.svg')
+		self.assertEquals(doc.getElementsByTagName('defs').length, 0,
+			'Kept defs, although it contains only whitespace or is <defs/>')
 
 # TODO: write tests for --enable-viewboxing
 # TODO; write a test for embedding rasters

unittests/useless-defs.svg

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg">
+  <defs id="defs_one">
+    <linearGradient id="testgradient">
+      <stop style="stop-color:#e4ff01" offset="0"/>
+      <stop style="stop-color:#befb00" offset=".36267"/>
+      <stop style="stop-color:#216908" offset=".78168"/>
+      <stop style="stop-color:#0a2002" offset="1"/>
+    </linearGradient>
+    <filter id="CDS" x="-0.2" y="0.2" width="1.4" height="1.4">
+      <feGaussianBlur stdDeviation="8"/>
+      <feColorMatrix values="1 0 0 0 0 0 1 0 0 0 0 0 1 0 0 0 0 0 0.6 0"/>
+    </filter>
+    <pattern id="testpattern">
+        <rect x="0" y="0" width="10" height="10" style="fill:#f00;"/>
+    </pattern>
+    <symbol id="testsymbol">
+        <rect x="0" y="0" width="10" height="10" style="fill:#0f0;"/>
+    </symbol>
+  </defs>
+</svg>

unittests/whitespace-defs.svg

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg">
+  <defs id="defs_whitespace">
+  </defs>
+  <defs id="defs_empty"/>
+</svg>