Fix replacement of duplicate gradients if "fill/stroke" contains fallbacks (#109)

(fixes #79)

scour/scour.py

@@ -1381,22 +1381,27 @@ def removeDuplicateGradients(doc):
       for dupGrad in gradientsToRemove[masterGrad]:
          # if the duplicate gradient no longer has a parent that means it was
          # already re-mapped to another master gradient
-         if not dupGrad.parentNode: continue
+         if not dupGrad.parentNode:
+            continue
+
+         # for each element that referenced the gradient we are going to replace dup_id with master_id
          dup_id = dupGrad.getAttribute('id')
-         # for each element that referenced the gradient we are going to remove
+         funcIRI = re.compile('url\([\'"]?#' + dup_id + '[\'"]?\)') # matches url(#a), url('#a') and url("#a")
          for elem in referencedIDs[dup_id][1]:
             # find out which attribute referenced the duplicate gradient
             for attr in ['fill', 'stroke']:
                v = elem.getAttribute(attr)
-               if v == 'url(#'+dup_id+')' or v == 'url("#'+dup_id+'")' or v == "url('#"+dup_id+"')":
-                  elem.setAttribute(attr, 'url(#'+master_id+')')
+               (v_new, n) = funcIRI.subn('url(#'+master_id+')', v)
+               if n > 0:
+                  elem.setAttribute(attr, v_new)
             if elem.getAttributeNS(NS['XLINK'], 'href') == '#'+dup_id:
                elem.setAttributeNS(NS['XLINK'], 'href', '#'+master_id)
             styles = _getStyle(elem)
             for style in styles:
                v = styles[style]
-               if v == 'url(#'+dup_id+')' or v == 'url("#'+dup_id+'")' or v == "url('#"+dup_id+"')":
-                  styles[style] = 'url(#'+master_id+')'
+               (v_new, n) = funcIRI.subn('url(#'+master_id+')', v)
+               if n > 0:
+                  styles[style] = v_new
             _setStyle(elem, styles)
 
          # now that all referencing elements have been re-mapped to the master

testscour.py

@@ -864,9 +864,9 @@ class RereferenceForLinearGradient(unittest.TestCase):
 		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
 		rects = svgdoc.getElementsByTagNameNS(SVGNS, 'rect')
 		self.assertEqual(rects[0].getAttribute('fill'), rects[1].getAttribute('stroke'),
-			'Rect not changed after removing duplicate linear gradient')
+			'Reference not updated after removing duplicate linear gradient')
 		self.assertEqual(rects[0].getAttribute('fill'), rects[4].getAttribute('fill'),
-			'Rect not changed after removing duplicate linear gradient')
+			'Reference not updated after removing duplicate linear gradient')
 
 class RemoveDuplicateRadialGradients(unittest.TestCase):
 	def runTest(self):
@@ -880,7 +880,15 @@ class RereferenceForRadialGradient(unittest.TestCase):
 		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
 		rects = svgdoc.getElementsByTagNameNS(SVGNS, 'rect')
 		self.assertEqual(rects[2].getAttribute('stroke'), rects[3].getAttribute('fill'),
-			'Rect not changed after removing duplicate radial gradient')
+			'Reference not updated after removing duplicate radial gradient')
+
+class RereferenceForGradientWithFallback(unittest.TestCase):
+	def runTest(self):
+		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
+		rects = svgdoc.getElementsByTagNameNS(SVGNS, 'rect')
+		self.assertEqual(rects[0].getAttribute('fill') + ' #fff', rects[5].getAttribute('fill'),
+			'Reference (with fallback) not updated after removing duplicate linear gradient')
+
 
 class CollapseSamePathPoints(unittest.TestCase):
 	def runTest(self):
@@ -1536,13 +1544,14 @@ class DuplicateGradientsUpdateStyle(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/duplicate-gradients-update-style.svg',
 			scour.parse_args(['--disable-style-to-xml']))
-		gradientTag = doc.getElementsByTagName('linearGradient')[0]
-		rectTag0 = doc.getElementsByTagName('rect')[0]
-		rectTag1 = doc.getElementsByTagName('rect')[1]
-		self.assertEqual('fill:url(#' + gradientTag.getAttribute('id') + ')', rectTag0.getAttribute('style'),
+		gradient = doc.getElementsByTagName('linearGradient')[0]
+		rects = doc.getElementsByTagName('rect')
+		self.assertEqual('fill:url(#' + gradient.getAttribute('id') + ')', rects[0].getAttribute('style'),
 			'Either of #duplicate-one or #duplicate-two was removed, but style="fill:" was not updated to reflect this')
-		self.assertEqual('fill:url(#' + gradientTag.getAttribute('id') + ')', rectTag1.getAttribute('style'),
+		self.assertEqual('fill:url(#' + gradient.getAttribute('id') + ')', rects[1].getAttribute('style'),
 			'Either of #duplicate-one or #duplicate-two was removed, but style="fill:" was not updated to reflect this')
+		self.assertEqual('fill:url(#' + gradient.getAttribute('id') + ') #fff', rects[2].getAttribute('style'),
+			'Either of #duplicate-one or #duplicate-two was removed, but style="fill:" (with fallback) was not updated to reflect this')
 
 class DocWithFlowtext(unittest.TestCase):
     def runTest(self):

unittests/duplicate-gradients-update-style.svg

@@ -12,4 +12,5 @@
 </defs>
 <rect style="fill: url(#duplicate-one)" width="200" height="200"/>
 <rect style="fill: url(#duplicate-two)" width="200" height="200" y="200"/>
+<rect style="fill: url(#duplicate-two) #fff" width="200" height="200" y="200"/>
 </svg>

unittests/remove-duplicate-gradients.svg

@@ -20,4 +20,5 @@
 	<rect id="r3" stroke="url(#g3)" width="100" height="100"/>
 	<rect id="r4" fill='url("#g4")' width="100" height="100"/>
 	<rect id="r5" fill="url(#g5)" width="100" height="100"/>
+	<rect id="r6" fill="url(#g5) #fff" width="100" height="100"/>
 </svg>