Fix Bug 503034 by only removing children of a group in a defs if the group itself is not referenced anywhere else in the file

release-notes.html

@@ -13,11 +13,12 @@
 	<header>
 		<h2><a href="#0.23">Version 0.23</a></h2>
 	</header>
-	<p>TBD</p>
+	<p>2009-01-04</p>
 	<ul>
 		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/482215">Bug 482215</a> by using os.linesep to end lines</li>
 		<li>Fix unittests to run properly in Windows</li>
 		<li>Removed default scaling of image to 100%/100% and creating a viewBox.  Added --enable-viewboxing option to explicitly turn that on</li>
+		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/503034">Bug 503034</a> by only removing children of a group if the group itself has not been referenced anywhere else in the file</li>
 	</ul>
 </section>
 

scour.py

@@ -34,6 +34,7 @@
 #    at rounded corners)
 
 # Next Up:
+# - only remove unreferenced elements if they are not children of a referenced element
 # - TODO: fix the removal of comment elements (between <?xml?> and <svg>)
 # - add an option to remove ids if they match the Inkscape-style of IDs
 # - investigate point-reducing algorithms
@@ -506,12 +507,18 @@ def removeUnusedDefs(doc, defElem, elemsToRemove=None):
 
 	identifiedElements = findElementsWithId(doc.documentElement)
 	referencedIDs = findReferencedElements(doc.documentElement)
-	
+
 	keepTags = ['font', 'style', 'metadata', 'script', 'title', 'desc']
 	for elem in defElem.childNodes:
-		if elem.nodeName == 'g' and elem.namespaceURI == NS['SVG']:
+		# we only inspect the children of a group in a defs if the group
+		# is not referenced anywhere else
+		if elem.nodeName == 'g' and elem.namespaceURI == NS['SVG'] and \
+				not elem.getAttribute('id') in referencedIDs:
 			elemsToRemove = removeUnusedDefs(doc, elem, elemsToRemove)
 			continue
+		
+		# we only remove if it is an element with a blank id and it is
+		# a direct child of the defs
 		if elem.nodeType == 1 and (elem.getAttribute('id') == '' or \
 				(not elem.getAttribute('id') in referencedIDs)) and \
 				not elem.nodeName in keepTags:

testscour.py

@@ -154,6 +154,13 @@ class RemoveUnreferencedElementInDefs(unittest.TestCase):
 		self.assertEquals(len(doc.getElementsByTagNameNS(SVGNS, 'rect')), 1,
 			'Unreferenced rect left in defs' )
 
+class DoNotRemoveChainedRefsInDefs(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/refs-in-defs.svg')
+		g = doc.getElementsByTagNameNS(SVGNS, 'g')[0]
+		self.assertEquals( g.childNodes.length >= 2, True,
+			'Chained references not honored in defs' )
+
 class KeepTitleInDefs(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/referenced-elements-1.svg')
@@ -975,7 +982,8 @@ class EnsureLineEndings(unittest.TestCase):
 		s = scour.scourString(open('unittests/whitespace-important.svg').read())
 		self.assertEquals( len(s.splitlines()), 4, 
 			'Did not output line ending character correctly')
-		
+
+# TODO: write tests for --enable-viewboxing
 # TODO; write a test for embedding rasters
 # TODO: write a test for --disable-embed-rasters
 # TODO: write tests for --keep-editor-data