Skip to content

Commit

Permalink
Check xlink:href for gradientUnits="userSpaceOnUse"
Browse files Browse the repository at this point in the history
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: scour-project#225
Signed-off-by: Niels Thykier <[email protected]>
  • Loading branch information
nthykier committed Feb 27, 2021
1 parent fbf0c06 commit e4c3bf1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 10 deletions.
46 changes: 38 additions & 8 deletions scour/scour.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -2006,38 +2036,38 @@ 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']),
DefaultAttribute('y1', 0, elements=['linearGradient']),
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')),
DefaultAttribute('fy', elements=['radialGradient'],
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
Expand Down
17 changes: 15 additions & 2 deletions test_scour.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')


Expand Down
10 changes: 10 additions & 0 deletions unittests/gradient-default-attrs.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit e4c3bf1

Please sign in to comment.