From 4657cb7515227ee874e8aad835230eecc5163b7f Mon Sep 17 00:00:00 2001 From: Louis Simard Date: Fri, 11 Feb 2011 12:29:20 -0500 Subject: [PATCH] Apply a modified patch submitted by Jan Thor in bug 717254 to fix a bug whereby Scour would keep the element if it was there but had only whitespace or unreferenced children. --- scour.py | 22 ++++++++++++---------- testscour.py | 32 ++++++++++++++++++++++---------- unittests/useless-defs.svg | 21 +++++++++++++++++++++ unittests/whitespace-defs.svg | 6 ++++++ 4 files changed, 61 insertions(+), 20 deletions(-) create mode 100644 unittests/useless-defs.svg create mode 100644 unittests/whitespace-defs.svg diff --git a/scour.py b/scour.py index 4003466..5195379 100755 --- a/scour.py +++ b/scour.py @@ -596,8 +596,9 @@ def removeUnreferencedElements(doc): """ global numElemsRemoved num = 0 + + # Remove certain unreferenced elements outside of defs removeTags = ['linearGradient', 'radialGradient', 'pattern'] - identifiedElements = findElementsWithId(doc.documentElement) referencedIDs = findReferencedElements(doc.documentElement) @@ -609,8 +610,7 @@ def removeUnreferencedElements(doc): num += 1 numElemsRemoved += 1 - # TODO: should also go through defs and vacuum it - num = 0 + # Remove most unreferenced elements inside defs defs = doc.documentElement.getElementsByTagName('defs') for aDef in defs: elemsToRemove = removeUnusedDefs(doc, aDef) @@ -2784,15 +2784,21 @@ def scourString(in_string, options=None): if options.remove_metadata: removeMetadataElements(doc) + # remove unreferenced gradients/patterns outside of defs + # and most unreferenced elements inside of defs + while removeUnreferencedElements(doc) > 0: + pass + # remove empty defs, metadata, g - # NOTE: these elements will be removed even if they have (invalid) text nodes - elemsToRemove = [] + # NOTE: these elements will be removed if they just have whitespace-only text nodes for tag in ['defs', 'metadata', 'g'] : for elem in doc.documentElement.getElementsByTagName(tag) : removeElem = not elem.hasChildNodes() if removeElem == False : for child in elem.childNodes : - if child.nodeType in [1, 3, 4, 8] : + if child.nodeType in [1, 4, 8]: + break + elif child.nodeType == 3 and not child.nodeValue.isspace(): break else: removeElem = True @@ -2800,10 +2806,6 @@ def scourString(in_string, options=None): elem.parentNode.removeChild(elem) numElemsRemoved += 1 - # remove unreferenced gradients/patterns outside of defs - while removeUnreferencedElements(doc) > 0: - pass - if options.strip_ids: bContinueLooping = True while bContinueLooping: diff --git a/testscour.py b/testscour.py index ff35986..a4d0d2d 100755 --- a/testscour.py +++ b/testscour.py @@ -1148,20 +1148,20 @@ class StyleToAttr(unittest.TestCase): class PathEmptyMove(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/path-empty-move.svg') - self.assertEquals(doc.getElementsByTagName('path')[0].getAttribute('d'), 'm100 100l200 100z'); - self.assertEquals(doc.getElementsByTagName('path')[1].getAttribute('d'), 'm100 100v200l100 100z'); + self.assertEquals(doc.getElementsByTagName('path')[0].getAttribute('d'), 'm100 100l200 100z') + self.assertEquals(doc.getElementsByTagName('path')[1].getAttribute('d'), 'm100 100v200l100 100z') class DefaultsRemovalToplevel(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[1].getAttribute('fill-rule'), '', - 'Default attribute fill-rule:nonzero not removed'); + 'Default attribute fill-rule:nonzero not removed') class DefaultsRemovalToplevelInverse(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[0].getAttribute('fill-rule'), 'evenodd', - 'Non-Default attribute fill-rule:evenodd removed'); + 'Non-Default attribute fill-rule:evenodd removed') class DefaultsRemovalToplevelFormat(unittest.TestCase): def runTest(self): @@ -1173,37 +1173,49 @@ class DefaultsRemovalInherited(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[3].getAttribute('fill-rule'), '', - 'Default attribute fill-rule:nonzero not removed in child'); + 'Default attribute fill-rule:nonzero not removed in child') class DefaultsRemovalInheritedInverse(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[2].getAttribute('fill-rule'), 'evenodd', - 'Non-Default attribute fill-rule:evenodd removed in child'); + 'Non-Default attribute fill-rule:evenodd removed in child') class DefaultsRemovalInheritedFormat(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[2].getAttribute('stroke-width'), '', - 'Default attribute stroke-width:1.00 not removed in child'); + 'Default attribute stroke-width:1.00 not removed in child') class DefaultsRemovalOverwrite(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[5].getAttribute('fill-rule'), 'nonzero', - 'Default attribute removed, although it overwrites parent element'); + 'Default attribute removed, although it overwrites parent element') class DefaultsRemovalOverwriteMarker(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[4].getAttribute('marker-start'), 'none', - 'Default marker attribute removed, although it overwrites parent element'); + 'Default marker attribute removed, although it overwrites parent element') class DefaultsRemovalNonOverwrite(unittest.TestCase): def runTest(self): doc = scour.scourXmlFile('unittests/cascading-default-attribute-removal.svg') self.assertEquals(doc.getElementsByTagName('path')[10].getAttribute('fill-rule'), '', - 'Default attribute not removed, although its parent used default'); + 'Default attribute not removed, although its parent used default') + +class RemoveDefsWithUnreferencedElements(unittest.TestCase): + def runTest(self): + doc = scour.scourXmlFile('unittests/useless-defs.svg') + self.assertEquals(doc.getElementsByTagName('defs').length, 0, + 'Kept defs, although it contains only unreferenced elements') + +class RemoveDefsWithWhitespace(unittest.TestCase): + def runTest(self): + doc = scour.scourXmlFile('unittests/whitespace-defs.svg') + self.assertEquals(doc.getElementsByTagName('defs').length, 0, + 'Kept defs, although it contains only whitespace or is ') # TODO: write tests for --enable-viewboxing # TODO; write a test for embedding rasters diff --git a/unittests/useless-defs.svg b/unittests/useless-defs.svg new file mode 100644 index 0000000..f4663ff --- /dev/null +++ b/unittests/useless-defs.svg @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/unittests/whitespace-defs.svg b/unittests/whitespace-defs.svg new file mode 100644 index 0000000..a32fcb4 --- /dev/null +++ b/unittests/whitespace-defs.svg @@ -0,0 +1,6 @@ + + + + + +