Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions Products/CMFPlone/browser/icons.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from zope.publisher.interfaces import IPublishTraverse

import logging
import uuid


logger = logging.getLogger(__name__)
Expand All @@ -28,8 +29,9 @@ def _add_aria_title(svgtree, cfg):
title = etree.Element("title")
root.append(title)
title.text = cfg["title"]
title.attrib["id"] = cfg["hash"]
# add aria attr
root.attrib["aria-labelledby"] = "title"
root.attrib["aria-labelledby"] = cfg["hash"]


SVG_MODIFER["add_aria_title"] = _add_aria_title
Expand All @@ -49,12 +51,27 @@ def _add_css_class(svgtree, cfg):
SVG_MODIFER["add_css_class"] = _add_css_class


def _strip_id(svgtree, cfg):
for el in svgtree.getroot().xpath("//*[@id]"):
del el.attrib["id"]
def _unique_id(svgtree, cfg):
# for <use (xlink:)href="#someid" /> references to work
# the ids in the svg must be unique in the document
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you actually use this svg in a <use /> reference if its id includes an unpredictable hash?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, the idea was to:

  1. create a unique hash on each tag() call
  2. replace all id="xyz" with id="xyz-<hash>" inside the SVG
  3. replace all referencing #xyz occurrences with #xyz-<hash> inside the SVG node attributes

while this worked in my local tests it seems, that there are breaking CI tests, because calling the same template with a delay isn't equal anymore (see https://github.com/plone/Products.CMFPlone/blob/6.1.x/Products/CMFPlone/tests/testBrowserDefault.py#L67)

so, to make this hash more predictable, we could create a hash from the svg filename including a counter of the occurrence on the page.

root = svgtree.getroot()
for el in root.xpath("//*[@id]"):
_id = el.attrib["id"]
if cfg["hash"] in _id:
# might be already unique
continue
_unique_id = f"{_id}-{cfg['hash']}"
el.attrib["id"] = _unique_id
refs = root.xpath(f"//*[@* = '#{_id}' or @* = 'url(#{_id})']")
for ref in refs:
for attr, attr_val in ref.attrib.items():
# there are multiple attributes that might contain a reference
# to an id, e.g. href, xlink:href, fill, filter, clip-path, ...
if f"#{_id}" in attr_val:
ref.attrib[attr] = attr_val.replace(f"#{_id}", f"#{_unique_id}")


SVG_MODIFER["strip_id"] = _strip_id
SVG_MODIFER["unique_id"] = _unique_id


@implementer(IPublishTraverse)
Expand Down Expand Up @@ -130,8 +147,9 @@ def tag(self, name, tag_class="", tag_alt=""):
modifier_cfg = {
"cssclass": tag_class,
"title": tag_alt,
"hash": str(uuid.uuid4()),
}
for name, modifier in SVG_MODIFER.items():
__traceback_info__ = name
for mod_name, modifier in SVG_MODIFER.items():
__traceback_info__ = mod_name
modifier(svgtree, modifier_cfg)
return etree.tostring(svgtree)
3 changes: 3 additions & 0 deletions news/4224.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Make referencing IDs of inline rendered SVGs unique.
Fix ``aria-labelledby`` parameter when providing ``tag_alt``.
@petschki