From a459d629c1e4ea3e18c3480021e9ea0b32629ab3 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Tue, 17 Apr 2018 19:05:52 +0000 Subject: [PATCH 1/4] removeDefaultAttributeValue: Special-case order attribute Scour tried to handle "order" attribute as a SVGLength. However, the "order" attribute *can* consist of two integers according to the [SVG 1.1 Specification] and SVGLength is not designed to handle that. With this change, we now pretend that "order" is a string, which side steps this issue. [SVG 1.1 Specification]: https://www.w3.org/TR/SVG11/single-page.html#filters-feConvolveMatrixElementOrderAttribute Closes: #189 Signed-off-by: Niels Thykier --- scour/scour.py | 7 ++++++- testscour.py | 10 ++++++++++ unittests/remove-default-attr-order.svg | 11 +++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 unittests/remove-default-attr-order.svg diff --git a/scour/scour.py b/scour/scour.py index c6f4c75..e5346b8 100644 --- a/scour/scour.py +++ b/scour/scour.py @@ -1848,7 +1848,12 @@ default_attributes = [ DefaultAttribute('offset', 0, elements=['feFuncA', 'feFuncB', 'feFuncG', 'feFuncR']), DefaultAttribute('operator', 'over', elements=['feComposite']), DefaultAttribute('operator', 'erode', elements=['feMorphology']), - DefaultAttribute('order', 3, elements=['feConvolveMatrix']), + # We pretend order is a string (because handling it as an + # SVGLength will cause issues when order is two integers). Note + # that order must be exactly one or two integers (no units or + # fancy numbers), so working with it a string will generally just + # work. + DefaultAttribute('order', '3', elements=['feConvolveMatrix']), DefaultAttribute('pointsAtX', 0, elements=['feSpotLight']), DefaultAttribute('pointsAtY', 0, elements=['feSpotLight']), DefaultAttribute('pointsAtZ', 0, elements=['feSpotLight']), diff --git a/testscour.py b/testscour.py index b52d98f..2c25258 100755 --- a/testscour.py +++ b/testscour.py @@ -1570,6 +1570,16 @@ class RemoveDefaultGradFYValue(unittest.TestCase): 'fy matching cy not removed') +class RemoveDefaultAttributeOrderSVGLengthCrash(unittest.TestCase): + + # Triggered a crash in v0.36 + def runTest(self): + try: + scourXmlFile('unittests/remove-default-attr-order.svg') + except AttributeError: + self.fail("Processing the order attribute triggered an AttributeError ") + + class CDATAInXml(unittest.TestCase): def runTest(self): diff --git a/unittests/remove-default-attr-order.svg b/unittests/remove-default-attr-order.svg new file mode 100644 index 0000000..d65848a --- /dev/null +++ b/unittests/remove-default-attr-order.svg @@ -0,0 +1,11 @@ + + + + + + + + + + From c504891bd74044297fc82d318b8b2b5c197e13f1 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 21 Apr 2018 06:19:38 +0000 Subject: [PATCH 2/4] test: Use number-optional-number variant of kernelUnitLength Signed-off-by: Niels Thykier --- unittests/remove-default-attr-order.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/remove-default-attr-order.svg b/unittests/remove-default-attr-order.svg index d65848a..506c9ce 100644 --- a/unittests/remove-default-attr-order.svg +++ b/unittests/remove-default-attr-order.svg @@ -2,7 +2,7 @@ - + From 8a2892b458a385dedcd19e21d53d136d467efb0f Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 21 Apr 2018 06:38:28 +0000 Subject: [PATCH 3/4] Avoid crashing on stdDeviation attribute Signed-off-by: Niels Thykier --- scour/scour.py | 3 ++- testscour.py | 12 +++++++++++- unittests/remove-default-attr-std-deviation.svg | 11 +++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 unittests/remove-default-attr-std-deviation.svg diff --git a/scour/scour.py b/scour/scour.py index e5346b8..3e5a8e5 100644 --- a/scour/scour.py +++ b/scour/scour.py @@ -1862,7 +1862,8 @@ default_attributes = [ DefaultAttribute('seed', 0, elements=['feTurbulence']), DefaultAttribute('specularConstant', 1, elements=['feSpecularLighting']), DefaultAttribute('specularExponent', 1, elements=['feSpecularLighting', 'feSpotLight']), - DefaultAttribute('stdDeviation', 0, elements=['feGaussianBlur']), + # Pretend it is a string (for the same reasons as we do with "order") + DefaultAttribute('stdDeviation', '0', elements=['feGaussianBlur']), DefaultAttribute('stitchTiles', 'noStitch', elements=['feTurbulence']), DefaultAttribute('surfaceScale', 1, elements=['feDiffuseLighting', 'feSpecularLighting']), DefaultAttribute('type', 'matrix', elements=['feColorMatrix']), diff --git a/testscour.py b/testscour.py index 2c25258..c182a35 100755 --- a/testscour.py +++ b/testscour.py @@ -1577,7 +1577,17 @@ class RemoveDefaultAttributeOrderSVGLengthCrash(unittest.TestCase): try: scourXmlFile('unittests/remove-default-attr-order.svg') except AttributeError: - self.fail("Processing the order attribute triggered an AttributeError ") + self.fail("Processing the order attribute triggered an AttributeError") + + +class RemoveDefaultAttributeStdDeviationSVGLengthCrash(unittest.TestCase): + + # Triggered a crash in v0.36 + def runTest(self): + try: + scourXmlFile('unittests/remove-default-attr-std-deviation.svg') + except AttributeError: + self.fail("Processing the order attribute triggered an AttributeError") class CDATAInXml(unittest.TestCase): diff --git a/unittests/remove-default-attr-std-deviation.svg b/unittests/remove-default-attr-std-deviation.svg new file mode 100644 index 0000000..ba88368 --- /dev/null +++ b/unittests/remove-default-attr-std-deviation.svg @@ -0,0 +1,11 @@ + + + + + + + + + + From 5d579f8927c426e212e96457c3e8e23870555932 Mon Sep 17 00:00:00 2001 From: Patrick Storz Date: Sat, 30 Jun 2018 18:58:36 +0200 Subject: [PATCH 4/4] Also special-case baseFrequency and add 'radius --- scour/scour.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/scour/scour.py b/scour/scour.py index 3e5a8e5..b656e12 100644 --- a/scour/scour.py +++ b/scour/scour.py @@ -1830,9 +1830,12 @@ default_attributes = [ DefaultAttribute('spreadMethod', 'pad', elements=['linearGradient', 'radialGradient']), # filter effects + # TODO: Some numerical attributes allow an optional second value ("number-optional-number") + # and are currently handled as strings to avoid an exception in 'SVGLength', see + # https://github.com/scour-project/scour/pull/192 DefaultAttribute('amplitude', 1, elements=['feFuncA', 'feFuncB', 'feFuncG', 'feFuncR']), DefaultAttribute('azimuth', 0, elements=['feDistantLight']), - DefaultAttribute('baseFrequency', 0, elements=['feFuncA', 'feFuncB', 'feFuncG', 'feFuncR']), + DefaultAttribute('baseFrequency', '0', elements=['feFuncA', 'feFuncB', 'feFuncG', 'feFuncR']), DefaultAttribute('bias', 1, elements=['feConvolveMatrix']), DefaultAttribute('diffuseConstant', 1, elements=['feDiffuseLighting']), DefaultAttribute('edgeMode', 'duplicate', elements=['feConvolveMatrix']), @@ -1848,21 +1851,16 @@ default_attributes = [ DefaultAttribute('offset', 0, elements=['feFuncA', 'feFuncB', 'feFuncG', 'feFuncR']), DefaultAttribute('operator', 'over', elements=['feComposite']), DefaultAttribute('operator', 'erode', elements=['feMorphology']), - # We pretend order is a string (because handling it as an - # SVGLength will cause issues when order is two integers). Note - # that order must be exactly one or two integers (no units or - # fancy numbers), so working with it a string will generally just - # work. DefaultAttribute('order', '3', elements=['feConvolveMatrix']), DefaultAttribute('pointsAtX', 0, elements=['feSpotLight']), DefaultAttribute('pointsAtY', 0, elements=['feSpotLight']), DefaultAttribute('pointsAtZ', 0, elements=['feSpotLight']), DefaultAttribute('preserveAlpha', 'false', elements=['feConvolveMatrix']), + DefaultAttribute('radius', '0', elements=['feMorphology']), DefaultAttribute('scale', 0, elements=['feDisplacementMap']), DefaultAttribute('seed', 0, elements=['feTurbulence']), DefaultAttribute('specularConstant', 1, elements=['feSpecularLighting']), DefaultAttribute('specularExponent', 1, elements=['feSpecularLighting', 'feSpotLight']), - # Pretend it is a string (for the same reasons as we do with "order") DefaultAttribute('stdDeviation', '0', elements=['feGaussianBlur']), DefaultAttribute('stitchTiles', 'noStitch', elements=['feTurbulence']), DefaultAttribute('surfaceScale', 1, elements=['feDiffuseLighting', 'feSpecularLighting']),