From 3fc0877b7caf07eb9218aa60fec8891eed60b6fb Mon Sep 17 00:00:00 2001
From: Hurr Durr <cynthia@jolteon>
Date: Sun, 11 Jul 2010 16:16:45 -0400
Subject: [PATCH] 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                  | 25 +++++++++++++++++++------
 testscour.py              | 32 ++++++++++++++++++++++++++------
 unittests/style-cdata.svg | 16 ++++++++++++++++
 3 files changed, 61 insertions(+), 12 deletions(-)
 create mode 100644 unittests/style-cdata.svg

diff --git a/scour.py b/scour.py
index 6c50c786..ac7c05a 100755
--- a/scour.py
+++ b/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
-		if node.firstChild != None:
-			cssRules = parseCssString(node.firstChild.nodeValue)
+		# one stretch of text, please! (we could use node.normalize(), but
+		# this actually modifies the node, and we don't want to keep
+		# 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
diff --git a/testscour.py b/testscour.py
index 68d29bb..c467f97 100755
--- a/testscour.py
+++ b/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')[1].getAttribute('overflow'), '')
+		self.assertEquals(doc.getElementsByTagName('marker')[0].getAttribute('overflow'), 'visible',
+			'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')[1].getAttribute('overflow'), '')
-		self.assertEquals(doc.getElementsByTagName('svg')[2].getAttribute('overflow'), 'visible')
+		self.assertEquals(doc.getElementsByTagName('svg')[0].getAttribute('overflow'), '',
+			'Did not remove the overflow attribute when it was using the default value')
+		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
diff --git a/unittests/style-cdata.svg b/unittests/style-cdata.svg
new file mode 100644
index 0000000..4740da9
--- /dev/null
+++ b/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>