Skip to content

Commit 315705a

Browse files
anthrotypeanthrotype
committed
[subset] fix subsetting OT-SVG when glyph id attribute is on the root <svg> element
Fixes #2548
1 parent caa32df commit 315705a

File tree

2 files changed

+81
-10
lines changed

2 files changed

+81
-10
lines changed

‎Lib/fontTools/subset/svg.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ def xpath(path):
3636

3737

3838
def group_elements_by_id(tree: etree.Element) -> Dict[str, etree.Element]:
39-
return {el.attrib["id"]: el for el in xpath(".//svg:*[@id]")(tree)}
39+
# select all svg elements with 'id' attribute no matter where they are
40+
# including the root element itself:
41+
# https://github.com/fonttools/fonttools/issues/2548
42+
return {el.attrib["id"]: el for el in xpath("//svg:*[@id]")(tree)}
4043

4144

4245
def parse_css_declarations(style_attr: str) -> Dict[str, str]:

‎Tests/subset/svg_test.py‎

Lines changed: 77 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,32 @@ def empty_svg_font():
4343
return fb.font
4444

4545

46+
# 'simple' here means one svg document per glyph. The required 'id' attribute
47+
# containing the 'glyphXXX' indices can be either on a child of the root <svg>
48+
# or on the <svg> root itself, so we test with both.
49+
# see https://github.com/fonttools/fonttools/issues/2548
50+
51+
52+
def simple_svg_table_glyph_ids_on_children(empty_svg_font):
53+
font = empty_svg_font
54+
svg_docs = font["SVG "].docList
55+
for i in range(1, 11):
56+
svg = new_svg()
57+
etree.SubElement(svg, "path", {"id": f"glyph{i}", "d": f"M{i},{i}"})
58+
svg_docs.append((etree.tostring(svg).decode(), i, i))
59+
return font
60+
61+
62+
def simple_svg_table_glyph_ids_on_roots(empty_svg_font):
63+
font = empty_svg_font
64+
svg_docs = font["SVG "].docList
65+
for i in range(1, 11):
66+
svg = new_svg(id=f"glyph{i}")
67+
etree.SubElement(svg, "path", {"d": f"M{i},{i}"})
68+
svg_docs.append((etree.tostring(svg).decode(), i, i))
69+
return font
70+
71+
4672
def new_svg(**attrs):
4773
return etree.Element("svg", {"xmlns": NAMESPACES["svg"], **attrs})
4874

@@ -52,10 +78,11 @@ def _lines(s):
5278

5379

5480
@pytest.mark.parametrize(
55-
"gids, retain_gids, expected_xml",
81+
"add_svg_table, gids, retain_gids, expected_xml",
5682
[
5783
# keep four glyphs in total, don't retain gids, which thus get remapped
5884
(
85+
simple_svg_table_glyph_ids_on_children,
5986
"2,4-6",
6087
False,
6188
_lines(
@@ -75,8 +102,32 @@ def _lines(s):
75102
"""
76103
),
77104
),
105+
# same as above but with glyph id attribute in the root <svg> element itself
106+
# https://github.com/fonttools/fonttools/issues/2548
107+
(
108+
simple_svg_table_glyph_ids_on_roots,
109+
"2,4-6",
110+
False,
111+
_lines(
112+
"""\
113+
<svgDoc endGlyphID="1" startGlyphID="1">
114+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph1"><path d="M2,2"/></svg>]]>
115+
</svgDoc>
116+
<svgDoc endGlyphID="2" startGlyphID="2">
117+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph2"><path d="M4,4"/></svg>]]>
118+
</svgDoc>
119+
<svgDoc endGlyphID="3" startGlyphID="3">
120+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph3"><path d="M5,5"/></svg>]]>
121+
</svgDoc>
122+
<svgDoc endGlyphID="4" startGlyphID="4">
123+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph4"><path d="M6,6"/></svg>]]>
124+
</svgDoc>
125+
"""
126+
),
127+
),
78128
# same four glyphs, but we now retain gids
79129
(
130+
simple_svg_table_glyph_ids_on_children,
80131
"2,4-6",
81132
True,
82133
_lines(
@@ -96,18 +147,35 @@ def _lines(s):
96147
"""
97148
),
98149
),
150+
# retain gids like above but with glyph id attribute in the root <svg> element itself
151+
# https://github.com/fonttools/fonttools/issues/2548
152+
(
153+
simple_svg_table_glyph_ids_on_roots,
154+
"2,4-6",
155+
True,
156+
_lines(
157+
"""\
158+
<svgDoc endGlyphID="2" startGlyphID="2">
159+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph2"><path d="M2,2"/></svg>]]>
160+
</svgDoc>
161+
<svgDoc endGlyphID="4" startGlyphID="4">
162+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph4"><path d="M4,4"/></svg>]]>
163+
</svgDoc>
164+
<svgDoc endGlyphID="5" startGlyphID="5">
165+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph5"><path d="M5,5"/></svg>]]>
166+
</svgDoc>
167+
<svgDoc endGlyphID="6" startGlyphID="6">
168+
<![CDATA[<svg xmlns="http://www.w3.org/2000/svg" id="glyph6"><path d="M6,6"/></svg>]]>
169+
</svgDoc>
170+
"""
171+
),
172+
),
99173
],
100174
)
101175
def test_subset_single_glyph_per_svg(
102-
empty_svg_font, tmp_path, gids, retain_gids, expected_xml
176+
empty_svg_font, add_svg_table, tmp_path, gids, retain_gids, expected_xml
103177
):
104-
font = empty_svg_font
105-
106-
svg_docs = font["SVG "].docList
107-
for i in range(1, 11):
108-
svg = new_svg()
109-
etree.SubElement(svg, "path", {"id": f"glyph{i}", "d": f"M{i},{i}"})
110-
svg_docs.append((etree.tostring(svg).decode(), i, i))
178+
font = add_svg_table(empty_svg_font)
111179

112180
svg_font_path = tmp_path / "TestSVG.ttf"
113181
font.save(svg_font_path)

0 commit comments

Comments
 (0)