From e4c3bf105e31edfe3c0f5d2530ef2ac5c7b75554 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 27 Feb 2021 22:14:50 +0000 Subject: [PATCH] Check xlink:href for gradientUnits="userSpaceOnUse" When considering to prune attributes set to a default values, we checked the `gradientUnits` attribute first to determine if that was valid. Unfortunately, we omitted to check whether the `gradientUnits` attribute was inherited via `xlink:ref`. This commit corrects that bug. Closes: #225 Signed-off-by: Niels Thykier --- scour/scour.py | 46 +++++++++++++++++++++++----- test_scour.py | 17 ++++++++-- unittests/gradient-default-attrs.svg | 10 ++++++ 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/scour/scour.py b/scour/scour.py index 91326c6..9b3f5fe 100644 --- a/scour/scour.py +++ b/scour/scour.py @@ -1931,6 +1931,36 @@ def mayContainTextNodes(node): return result +def _gradient_units_is_not_user_space(node): + current_node = node + doc = None + while True: + gradient_unit = node.getAttribute('gradientUnits') + if gradient_unit != '': + # The current node has a value, then it decides + break + + # The current node did not have value - check if we reference + # another node that might have it. + href = current_node.getAttributeNS(NS['XLINK'], 'href') + if href == '' or not href.startswith('#'): + return True + if doc is None: + doc = current_node.ownerDocument + # Should not happen in practise as our nodes should + # always be attached to a document. However, if it + # does, then we assume we can strip the attribute. + if doc is None: + return True + current_node = doc.getElementById(href[1:]) + if current_node is None: + # Assume we can strip the attribute when we cannot find + # the other node + return True + + return gradient_unit != 'userSpaceOnUse' + + # A list of default attributes that are safe to remove if all conditions are fulfilled # # Each default attribute is an object of type 'DefaultAttribute' with the following fields: @@ -2006,16 +2036,16 @@ def mayContainTextNodes(node): # filters and masks DefaultAttribute('x', -10, Unit.PCT, ['filter', 'mask']), DefaultAttribute('x', -0.1, Unit.NONE, ['filter', 'mask'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), DefaultAttribute('y', -10, Unit.PCT, ['filter', 'mask']), DefaultAttribute('y', -0.1, Unit.NONE, ['filter', 'mask'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), DefaultAttribute('width', 120, Unit.PCT, ['filter', 'mask']), DefaultAttribute('width', 1.2, Unit.NONE, ['filter', 'mask'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), DefaultAttribute('height', 120, Unit.PCT, ['filter', 'mask']), DefaultAttribute('height', 1.2, Unit.NONE, ['filter', 'mask'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), # gradients DefaultAttribute('x1', 0, elements=['linearGradient']), @@ -2023,7 +2053,7 @@ def mayContainTextNodes(node): DefaultAttribute('y2', 0, elements=['linearGradient']), DefaultAttribute('x2', 100, Unit.PCT, elements=['linearGradient']), DefaultAttribute('x2', 1, Unit.NONE, elements=['linearGradient'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), # remove fx/fy before cx/cy to catch the case where fx = cx = 50% or fy = cy = 50% respectively DefaultAttribute('fx', elements=['radialGradient'], conditions=lambda node: node.getAttribute('fx') == node.getAttribute('cx')), @@ -2031,13 +2061,13 @@ def mayContainTextNodes(node): conditions=lambda node: node.getAttribute('fy') == node.getAttribute('cy')), DefaultAttribute('r', 50, Unit.PCT, elements=['radialGradient']), DefaultAttribute('r', 0.5, Unit.NONE, elements=['radialGradient'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), DefaultAttribute('cx', 50, Unit.PCT, elements=['radialGradient']), DefaultAttribute('cx', 0.5, Unit.NONE, elements=['radialGradient'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), DefaultAttribute('cy', 50, Unit.PCT, elements=['radialGradient']), DefaultAttribute('cy', 0.5, Unit.NONE, elements=['radialGradient'], - conditions=lambda node: node.getAttribute('gradientUnits') != 'userSpaceOnUse'), + conditions=_gradient_units_is_not_user_space), DefaultAttribute('spreadMethod', 'pad', elements=['linearGradient', 'radialGradient']), # filter effects diff --git a/test_scour.py b/test_scour.py index 549333f..f321672 100755 --- a/test_scour.py +++ b/test_scour.py @@ -1517,8 +1517,21 @@ def runTest(self): class RemoveDefaultGradX1Value(unittest.TestCase): def runTest(self): - g = scourXmlFile('unittests/gradient-default-attrs.svg').getElementById('grad1') - self.assertEqual(g.getAttribute('x1'), '', + doc = scourXmlFile('unittests/gradient-default-attrs.svg') + g1 = doc.getElementById('grad1') + g1b = doc.getElementById('grad1b') + g1c = doc.getElementById('grad1c') + g1d = doc.getElementById('grad1d') + g1e = doc.getElementById('grad1e') + self.assertEqual(g1.getAttribute('x1'), '', + 'x1="0" not removed') + self.assertEqual(g1b.getAttribute('x1'), '', + 'x1="0" not removed') + self.assertEqual(g1c.getAttribute('x1'), '', + 'x1="0" removed (but should not be due to gradientUnits)') + self.assertEqual(g1d.getAttribute('x1'), '', + 'x1="0" removed (but should not be due to href)') + self.assertEqual(g1e.getAttribute('x1'), '', 'x1="0" not removed') diff --git a/unittests/gradient-default-attrs.svg b/unittests/gradient-default-attrs.svg index 25cdb82..6bc9762 100644 --- a/unittests/gradient-default-attrs.svg +++ b/unittests/gradient-default-attrs.svg @@ -12,10 +12,20 @@ + + + + + + + + + +