Merge 8c9c6253b9 into 5360db86d9

scour/scour.py

@@ -549,18 +549,23 @@ def findElementsWithId(node, elems=None):
 referencingProps = ['fill', 'stroke', 'filter', 'clip-path', 'mask',  'marker-start', 'marker-end', 'marker-mid']
 
 
-def findReferencedElements(node, ids=None):
+def findReferencedElements(start_node):
     """
     Returns IDs of all referenced elements
-    - node is the node at which to start the search.
+    - start_node is the node at which to start the search.
     - returns a map which has the id as key and
       each value is is a list of nodes
 
     Currently looks at 'xlink:href' and all attributes in 'referencingProps'
     """
     global referencingProps
-    if ids is None:
     ids = {}
+
+    # Search all nodes in depth-first search
+    stack = [start_node]
+
+    while stack:
+        node = stack.pop()
         # TODO: input argument ids is clunky here (see below how it is called)
         # GZ: alternative to passing dict, use **kwargs
 
@@ -576,7 +581,7 @@ def findReferencedElements(node, ids=None):
                     for propname in rule['properties']:
                         propval = rule['properties'][propname]
                         findReferencingProperty(node, propname, propval, ids)
-        return ids
+            continue
 
         # else if xlink:href is set, then grab the id
         href = node.getAttributeNS(NS['XLINK'], 'href')
@@ -605,9 +610,8 @@ def findReferencedElements(node, ids=None):
             findReferencingProperty(node, attr, val, ids)
 
         if node.hasChildNodes():
-        for child in node.childNodes:
+            stack.extend(n for n in node.childNodes if n.nodeType == Node.ELEMENT_NODE)
-            if child.nodeType == Node.ELEMENT_NODE:
+
-                findReferencedElements(child, ids)
     return ids
 
 
@@ -1295,14 +1299,14 @@ def removeDuplicateGradientStops(doc):
     return num
 
 
-def collapseSinglyReferencedGradients(doc):
+def collapseSinglyReferencedGradients(doc, referencedIDs):
     global _num_elements_removed
     num = 0
 
     identifiedElements = findElementsWithId(doc.documentElement)
 
     # make sure to reset the ref'ed ids for when we are running this in testscour
-    for rid, nodes in six.iteritems(findReferencedElements(doc.documentElement)):
+    for rid, nodes in six.iteritems(referencedIDs):
         # Make sure that there's actually a defining element for the current ID name.
         # (Cyn: I've seen documents with #id references but no element with that ID!)
         if len(nodes) == 1 and rid in identifiedElements:
@@ -1355,7 +1359,7 @@ def collapseSinglyReferencedGradients(doc):
     return num
 
 
-def removeDuplicateGradients(doc):
+def removeDuplicateGradients(doc, referencedIDs):
     global _num_elements_removed
     num = 0
 
@@ -1417,8 +1421,6 @@ def removeDuplicateGradients(doc):
                         gradientsToRemove[grad].append(ograd)
                         duplicateToMaster[ograd] = grad
 
-    # get a collection of all elements that are referenced and their referencing elements
-    referencedIDs = findReferencedElements(doc.documentElement)
     for masterGrad in gradientsToRemove:
         master_id = masterGrad.getAttribute('id')
         for dupGrad in gradientsToRemove[masterGrad]:
@@ -1894,22 +1896,25 @@ def taint(taintedSet, taintedAttribute):
     return taintedSet
 
 
-def removeDefaultAttributeValue(node, attribute):
+def removeDefaultAttributeValuesFromNode(node, attributes):
     """
-    Removes the DefaultAttribute 'attribute' from 'node' if specified conditions are fulfilled
+    Removes each of the 'DefaultAttribute's from 'node' if specified conditions are fulfilled
     """
+    count = 0
+    for attribute in attributes:
         if not node.hasAttribute(attribute.name):
-        return 0
+            continue
 
         if (attribute.elements is not None) and (node.nodeName not in attribute.elements):
-        return 0
+            continue
 
         # differentiate between text and numeric values
         if isinstance(attribute.value, str):
             if node.getAttribute(attribute.name) == attribute.value:
                 if (attribute.conditions is None) or attribute.conditions(node):
                     node.removeAttribute(attribute.name)
-                return 1
+                    count += 1
+                    continue
         else:
             nodeValue = SVGLength(node.getAttribute(attribute.name))
             if ((attribute.value is None)
@@ -1919,9 +1924,10 @@ def removeDefaultAttributeValue(node, attribute):
                         or (isinstance(attribute.units, list) and nodeValue.units in attribute.units)):
                     if (attribute.conditions is None) or attribute.conditions(node):
                         node.removeAttribute(attribute.name)
-                    return 1
+                        count += 1
+                        continue
 
-    return 0
+    return count
 
 
 def removeDefaultAttributeValues(node, options, tainted=set()):
@@ -1933,8 +1939,7 @@ def removeDefaultAttributeValues(node, options, tainted=set()):
         return 0
 
     # Conditionally remove all default attributes defined in 'default_attributes' (a list of 'DefaultAttribute's)
-    for attribute in default_attributes:
+    num += removeDefaultAttributeValuesFromNode(node, default_attributes)
-        num += removeDefaultAttributeValue(node, attribute)
 
     # Summarily get rid of default properties
     attributes = [node.attributes.item(i).nodeName for i in range(node.attributes.length)]
@@ -3470,27 +3475,37 @@ def scourString(in_string, options=None):
                 elem.parentNode.removeChild(elem)
                 _num_elements_removed += 1
 
+    referencedIDs = None
     if options.strip_ids:
+        referencedIDs = findReferencedElements(doc.documentElement)
         bContinueLooping = True
         while bContinueLooping:
             identifiedElements = unprotected_ids(doc, options)
-            referencedIDs = findReferencedElements(doc.documentElement)
             bContinueLooping = (removeUnreferencedIDs(referencedIDs, identifiedElements) > 0)
+            referencedIDs = findReferencedElements(doc.documentElement)
 
     while removeDuplicateGradientStops(doc) > 0:
-        pass
+        # removeDuplicateGradientStops does not need referencedIDs, but it can
+        # remove nodes.  Make no assumptions about referencedIDs still being valid
+        # after this call.
+        referencedIDs = None
+
+    if referencedIDs is None:
+        referencedIDs = findReferencedElements(doc.documentElement)
 
     # remove gradients that are only referenced by one other gradient
-    while collapseSinglyReferencedGradients(doc) > 0:
+    while collapseSinglyReferencedGradients(doc, referencedIDs) > 0:
-        pass
+        referencedIDs = findReferencedElements(doc.documentElement)
 
     # remove duplicate gradients
-    while removeDuplicateGradients(doc) > 0:
+    while removeDuplicateGradients(doc, referencedIDs) > 0:
-        pass
+        referencedIDs = findReferencedElements(doc.documentElement)
 
     # create <g> elements if there are runs of elements with the same attributes.
     # this MUST be before moveCommonAttributesToParentGroup.
     if options.group_create:
+        # This does remove nodes, so referencedIDs should remain
+        # valid after this.
         createGroupsForCommonAttributes(doc.documentElement)
 
     # move common attributes to parent group
@@ -3498,9 +3513,8 @@ 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:
-        _num_attributes_removed += moveCommonAttributesToParentGroup(child, referencedIds)
+        _num_attributes_removed += moveCommonAttributesToParentGroup(child, referencedIDs)
 
     # remove unused attributes from parent
     _num_attributes_removed += removeUnusedAttributesOnParent(doc.documentElement)