Skip to content
GitLab
  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
    • Switch to GitLab Next
  • Sign in / Register
  • inkscape inkscape
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 1,525
    • Issues 1,525
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
  • Merge requests 135
    • Merge requests 135
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Deployments
    • Deployments
    • Releases
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Code review
    • Insights
    • Issue
    • Repository
  • External wiki
    • External wiki
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Inkscape
  • inkscapeinkscape
  • Merge requests
  • !3147

Fix SVG embedding

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Matthew Woehlke requested to merge mwoehlke/inkscape:fix-svg-embedding into master Apr 29, 2021
  • Overview 50
  • Commits 4
  • Pipelines 10
  • Changes 10

See commit messages for details. This fixes #1757 (closed) and a related problem:

  • An embedded (via a <use>) SVG cannot resolve hrefs to elements in the embedded SVG.
  • The child document's CSS styling is not applied.

Both of these behaviors are prescribed¹ by the specification.

That said, note that (as I understand the spec), this implementation is non-conforming, because it will resolve references anywhere in the embedded document, not just within the selected subtree. However, a) from my (admittedly limited) understanding of the code, I believe it is somewhat challenging to implement this strictly according to the spec², and would meanwhile like it to be less broken, and b) I suspect this behavior is actually more useful anyway. (In particular, allowing references outside the referenced subtree makes it possible to embed multiple parts of an SVG that share definitions, which would otherwise require duplicating those definitions in the embedding document.)

I'm very new to Inkscape's code, and this may not be the best approach. Also, it seems to be causing an assertion failure. However, I would appreciate if someone more knowledgeable can perhaps take a look and help get this into shape where it could be merged.

Note: Passing the SPUse to URIReference::attach seems fairly kludgy, but it was the most obvious way to ensure that the child document gets bound to the SPUse only when resolving the href and not, say, a url in the <use>'s style (as might happen if we just tried to dynamic_cast the reference's owner). Thoughts on a better way to handle this would be welcome.

(¹ Strictly speaking, as I understand the spec, <style> from the embedded document should only be used if part of the referenced subtree. However, CSS from the child was never being used, even if the subtree was actually the <svg>. It's not 100% clear that referencing the <svg> is supposed to be allowed, but it isn't clearly proscribed either, and in any case, doing so works since at least Inkscape 1.0.2.)

(² It would appear to require rebuilding the child SPDocument to contain only the referenced subtree. As I believe some of the stuff from the <svg> is required, this seems like it may require a non-trivial effort of combining bits of the root <svg> and the referenced subtree. Moreover, as noted, I would be inclined to want to put such logic behind some sort of 'strict conformance' option.)

Assignee
Assign to
Reviewer
Request review from
Time tracking
Source branch: fix-svg-embedding