Fix Bug 603988: do not commonize attributes if element is referenced elsewhere

release-notes.html

@@ -13,12 +13,13 @@
 	<header>
 		<h2><a href="#0.25">Version 0.25</a></h2>
 	</header>
-	<p>2010-05-09</p>
+	<p>2010-07-11</p>
 	<ul>
 		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/541889">Bug 541889</a> to parse polygon/polyline points missing whitespace/comma separating a negative value.  Always output points attributes as comma-separated.</li>
 		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/519698">Bug 519698</a> to properly parse move commands that have line segments.</li>
 		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/577940">Bug 577940</a> to include stroke-dasharray into list of style properties turned into XML attributes.</li>
 		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/562784">Bug 562784</a>, typo in Inkscape description</li>
+		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/603988">Bug 603988</a>, do not commonize attributes if the element is referenced elsewhere.</li>
 	</ul>
 </section>
 

scour.py

@@ -403,7 +403,10 @@ referencingProps = ['fill', 'stroke', 'filter', 'clip-path', 'mask',  'marker-st
 def findReferencedElements(node, ids=None):
 	"""
 	Returns the number of times an ID is referenced as well as all elements
-	that reference it.
+	that reference it.  node is the node at which to start the search.  The
+	return value is a map which has the id as key and each value is an array
+	where the first value is a count and the second value is a list of nodes
+	that referenced it.
 
 	Currently looks at fill, stroke, clip-path, mask, marker, and
 	xlink:href attributes.
@@ -775,12 +778,13 @@ def removeNestedGroups(node):
 			num += removeNestedGroups(child)		
 	return num
 
-def moveCommonAttributesToParentGroup(elem):
+def moveCommonAttributesToParentGroup(elem, referencedElements):
 	""" 
 	This recursively calls this function on all children of the passed in element
 	and then iterates over all child elements and removes common inheritable attributes 
 	from the children and places them in the parent group.  But only if the parent contains
-	nothing but element children and whitespace.
+	nothing but element children and whitespace.  The attributes are only removed from the
+	children if the children are not referenced by other elements in the document.
 	"""
 	num = 0
 	
@@ -788,8 +792,10 @@ def moveCommonAttributesToParentGroup(elem):
 	# recurse first into the children (depth-first)
 	for child in elem.childNodes:
 		if child.nodeType == 1:
-			childElements.append(child)
-			num += moveCommonAttributesToParentGroup(child)
+			# only add and recurse if the child is not referenced elsewhere
+			if not child.getAttribute('id') in referencedElements:
+				childElements.append(child)
+				num += moveCommonAttributesToParentGroup(child, referencedElements)
 		# else if the parent has non-whitespace text children, do not
 		# try to move common attributes
 		elif child.nodeType == 3 and child.nodeValue.strip():
@@ -2655,8 +2661,9 @@ def scourString(in_string, options=None):
 	# all have the same value for an attribute, it must not
 	# get moved to the <svg> element. The <svg> element
 	# doesn't accept fill=, stroke= etc.!
+	referencedIds = findReferencedElements(doc.documentElement)
 	for child in doc.documentElement.childNodes:
-		numAttrsRemoved += moveCommonAttributesToParentGroup(child)
+		numAttrsRemoved += moveCommonAttributesToParentGroup(child, referencedIds)
 	
 	# remove unused attributes from parent
 	numAttrsRemoved += removeUnusedAttributesOnParent(doc.documentElement)

testscour.py

@@ -1097,6 +1097,10 @@ class GroupNoCreation(unittest.TestCase):
 		self.assertEquals(doc.getElementsByTagName('g').length, 0,
 			'Created a <g> for a run of elements having dissimilar attributes')
 
+class DoNotCommonizeAttributesOnReferencedElements(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/commonized-referenced-elements.svg')
+		self.assertEquals(doc.getElementsByTagName('circle')[0].getAttribute('fill'), '#0f0')
 
 # TODO: write tests for --enable-viewboxing
 # TODO; write a test for embedding rasters

unittests/commonized-referenced-elements.svg

@@ -0,0 +1,9 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <g id="g">
+    <rect width="200" height="100" fill="#0f0"/>
+    <rect width="200" height="100" fill="#0f0"/>
+    <rect width="200" height="100" fill="#0f0"/>
+    <circle id="e" r="20" fill="#0f0"/>
+  </g>
+  <use xlink:href="#e" />
+</svg>