Fix bug 603994, whereby a <style> stylesheet would not count as a reference towards gradients etc., if the stylesheet was a CDATA element surrounded by whitespace nodes.

Add unit tests for this.

Add some descriptions to the failure modes of some unit tests, which would fail without any message (if they failed, but right now they succeed! ;).

scour.py

@@ -420,9 +420,14 @@ def findReferencedElements(node, ids=None):
 
 	# if this node is a style element, parse its text into CSS
 	if node.nodeName == 'style' and node.namespaceURI == NS['SVG']:
-		# node.firstChild will be either a CDATA or a Text node
+		# one stretch of text, please! (we could use node.normalize(), but
-		if node.firstChild != None:
+		# this actually modifies the node, and we don't want to keep
-			cssRules = parseCssString(node.firstChild.nodeValue)
+		# whitespace around if there's any)
+		stylesheet = ''
+		for child in node.childNodes:
+			stylesheet += child.nodeValue
+		if stylesheet != '':
+			cssRules = parseCssString(stylesheet)
 			for rule in cssRules:
 				for propname in rule['properties']:
 					propval = rule['properties'][propname]
@@ -632,14 +637,22 @@ def renameID(doc, idFrom, idTo, identifiedElements, referencedIDs):
 	for node in referringNodes[1]:
 		# if this node is a style element, parse its text into CSS
 		if node.nodeName == 'style' and node.namespaceURI == NS['SVG']:
-			# node.firstChild will be either a CDATA or a Text node
+			# node.firstChild will be either a CDATA or a Text node now
 			if node.firstChild != None:
-				oldValue = node.firstChild.nodeValue
+				# concatenate the value of all children, in case
+				# there's a CDATASection node surrounded by whitespace
+				# nodes
+				# (node.normalize() will NOT work here, it only acts on Text nodes)
+				oldValue = ''
+				for child in node.childNodes:
+					oldValue += child.nodeValue
 				# not going to reparse the whole thing
 				newValue = oldValue.replace('url(#' + idFrom + ')', 'url(#' + idTo + ')')
 				newValue = newValue.replace("url(#'" + idFrom + "')", 'url(#' + idTo + ')')
 				newValue = newValue.replace('url(#"' + idFrom + '")', 'url(#' + idTo + ')')
-				node.firstChild.nodeValue = newValue
+				# and now replace all the children with this new stylesheet.
+				# again, this is in case the stylesheet was a CDATASection
+				node.childNodes[:] = [node.ownerDocument.createTextNode(newValue)]
 				num += len(oldValue) - len(newValue)
 	
 		# if xlink:href is set to #idFrom, then change the id

testscour.py

@@ -1101,20 +1101,40 @@ class GroupNoCreation(unittest.TestCase):
 class DoNotCommonizeAttributesOnReferencedElements(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/commonized-referenced-elements.svg')
-		self.assertEquals(doc.getElementsByTagName('circle')[0].getAttribute('fill'), '#0f0')
+		self.assertEquals(doc.getElementsByTagName('circle')[0].getAttribute('fill'), '#0f0',
+			'Grouped an element referenced elsewhere into a <g>')
 
 class DoNotRemoveOverflowVisibleOnMarker(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/overflow-marker.svg')
-		self.assertEquals(doc.getElementsByTagName('marker')[0].getAttribute('overflow'), 'visible')
+		self.assertEquals(doc.getElementsByTagName('marker')[0].getAttribute('overflow'), 'visible',
-		self.assertEquals(doc.getElementsByTagName('marker')[1].getAttribute('overflow'), '')
+			'Removed the overflow attribute when it was not using the default value')
+		self.assertEquals(doc.getElementsByTagName('marker')[1].getAttribute('overflow'), '',
+			'Did not remove the overflow attribute when it was using the default value')
 
 class MarkerOnSvgElements(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/overflow-svg.svg')
-		self.assertEquals(doc.getElementsByTagName('svg')[0].getAttribute('overflow'), '')
+		self.assertEquals(doc.getElementsByTagName('svg')[0].getAttribute('overflow'), '',
-		self.assertEquals(doc.getElementsByTagName('svg')[1].getAttribute('overflow'), '')
+			'Did not remove the overflow attribute when it was using the default value')
-		self.assertEquals(doc.getElementsByTagName('svg')[2].getAttribute('overflow'), 'visible')
+		self.assertEquals(doc.getElementsByTagName('svg')[1].getAttribute('overflow'), '',
+			'Did not remove the overflow attribute when it was using the default value')
+		self.assertEquals(doc.getElementsByTagName('svg')[2].getAttribute('overflow'), 'visible',
+			'Removed the overflow attribute when it was not using the default value')
+
+class GradientReferencedByStyleCDATA(unittest.TestCase):
+	def runTest(self):
+		doc = scour.scourXmlFile('unittests/style-cdata.svg')
+		self.assertEquals(len(doc.getElementsByTagName('linearGradient')), 1,
+			'Removed a gradient referenced by an internal stylesheet')
+
+class ShortenIDsInStyleCDATA(unittest.TestCase):
+	def runTest(self):
+		docStr = file('unittests/style-cdata.svg').read()
+		docStr = scour.scourString(docStr,
+			scour.parse_args(['--shorten-ids'])[0])
+		self.assertEquals(docStr.find('somethingreallylong'), -1,
+			'Did not shorten IDs in the internal stylesheet')
 
 # TODO: write tests for --enable-viewboxing
 # TODO; write a test for embedding rasters

unittests/style-cdata.svg

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
+	<defs>
+		<style type="text/css">
+			<![CDATA[
+				rect { fill: url(#somethingreallylong); }
+			]]>
+		</style>
+		<linearGradient id="somethingreallylong" x1="0" x2="0" y1="0" y2="1">
+			<stop stop-color="red" offset="0" />
+			<stop stop-color="green" offset="1" />
+		</linearGradient>
+	</defs>
+
+	<rect width="100" height="100" />
+</svg>