scour icon indicating copy to clipboard operation
scour copied to clipboard

xml.dom.NotFoundErr in animate-elem-31-t.svg from the SVG 1.1 test suite

Open nthykier opened this issue 7 years ago • 3 comments

scour on the current master branch fails to process several of the SVG 1.1 reference SVGs. Below is one of these failures:

Processing animate-elem-31-t.svg
Traceback (most recent call last):
  File "/usr/lib/python3.6/xml/dom/minidom.py", line 800, in removeAttribute
    attr = self._attrs[name]
KeyError: 'stroke'

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "scour.py", line 3, in <module>
    run()
  File "$GIT_DIR/scour/scour.py", line 3928, in run
    start(options, input, output)
  File "$GIT_DIR/scour/scour.py", line 3896, in start
    out_string = scourString(in_string, options).encode("UTF-8")
  File "$GIT_DIR/scour/scour.py", line 3519, in scourString
    _num_attributes_removed += moveCommonAttributesToParentGroup(child, referencedIds)
  File "$GIT_DIR/scour/scour.py", line 1000, in moveCommonAttributesToParentGroup
    num += moveCommonAttributesToParentGroup(child, referencedElements)
  File "$GIT_DIR/scour/scour.py", line 1000, in moveCommonAttributesToParentGroup
    num += moveCommonAttributesToParentGroup(child, referencedElements)
  File "$GIT_DIR/scour/scour.py", line 1058, in moveCommonAttributesToParentGroup
    child.removeAttribute(name)
  File "/usr/lib/python3.6/xml/dom/minidom.py", line 802, in removeAttribute
    raise xml.dom.NotFoundErr()
xml.dom.NotFoundErr

Test data is from: https://www.w3.org/Graphics/SVG/Test/20110816/

nthykier avatar Apr 17 '18 06:04 nthykier

This is a reoccuring error (with different attributes). Options used:

scour.py --enable-id-stripping --enable-comment-stripping --shorten-ids --indent=none  --no-line-breaks --strip-xml-prolog --remove-descriptive-elements --set-precision=8 --set-c-precision=8 --create-groups --remove-titles --remove-descriptions --remove-metadata

nthykier avatar Apr 17 '18 06:04 nthykier

This triggers in the following construction

      <g display="none">
        <circle display="inline" cx="80" cy="30" r="20" fill="green" stroke="black" stroke-width="5"/>
        <animate attributeName="display" from="none" to="inline" begin="0" dur="3" fill="freeze"/>
      </g>

In the code we have:

    # for each subsequent child element
    for childNum in range(len(childElements)):
        # skip first child
        if childNum == 0:
            continue

        child = childElements[childNum]
        # if we are on an animateXXX/set element, ignore it (due to the 'fill' attribute)
        if child.localName in ['set', 'animate', 'animateColor', 'animateTransform', 'animateMotion']:
            continue
...

    # commonAttrs now has all the inheritable attributes which are common among all child elements
    for name in commonAttrs:
        for child in childElements:
            child.removeAttribute(name)

So we ignore the animate node when determining whether we can move "up" an attribute and then later assume that we can remove it.

@Ede123 Should we just ignore animate et al again in the remove loop ? (Possibly only if it does not have the attribute)

nthykier avatar Apr 21 '18 08:04 nthykier

This seems also related to #123.

In principle we need to make sure that the fill attribute of animate elements does not interfere with the detection/removal logic of the "default" fill attribute.

Probably making sure it's properly ignored everywhere should suffice, but I'd have to fully investigate first.

Ede123 avatar Jun 30 '18 17:06 Ede123