Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SVGO creates invalid self-closing tags inside <foreignObject> elements #1728

Open
rnwst opened this issue Dec 18, 2022 · 5 comments
Open

SVGO creates invalid self-closing tags inside <foreignObject> elements #1728

rnwst opened this issue Dec 18, 2022 · 5 comments
Labels
bug

Comments

@rnwst
Copy link

rnwst commented Dec 18, 2022

Describe the bug
When an empty tag is present inside of a <foreignObject> element, SVGO converts it to a self-closing tag. This results in tags that are invalid HTML and are misinterpreted by the browser.

To Reproduce
Steps to reproduce the behavior:

  1. Run svgo test.svg --pretty --indent=2, where
    test.svg:
<svg xmlns="http://www.w3.org/2000/svg" width="100px" height="100px" viewBox="0 0 10 10">
  <foreignObject x="0" y="0" width="10" height="10" style="font-size:2px">
    <p style="margin-top:0">
      <span style="height:4px;display:inline-block"></span>
      Some text.
    </p>
  </foreignObject>
</svg>
  1. Observe the result, which is:
<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100" viewBox="0 0 10 10">
  <foreignObject width="10" height="10" x="0" y="0" style="font-size:2px">
    <p style="margin-top:0">
      <span style="height:4px;display:inline-block"/>
      Some text.
    </p>
  </foreignObject>
</svg>

Note that the empty <span> element has been converted to a self-closing tag. This results in invalid HTML, since <span> is not a void element. The browser actually interprets the self-closing tag as an opening tag, and then automatically closes the tag later on. This is essentially equivalent to:

<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100" viewBox="0 0 10 10">
  <foreignObject width="10" height="10" x="0" y="0" style="font-size:2px">
    <p style="margin-top:0">
      <span style="height:4px;display:inline-block">
      Some text.
      </span>
    </p>
  </foreignObject>
</svg>

(See here for more information.)
When viewing the SVG in a browser, keep in mind that <foreignObject> elements will not be rendered unless the SVG is inside of an HTML page. To fix this, add <html> tags and save the file as an HTML file.

Expected behavior
Inside <foreignObject> elements, empty elements should not be converted to self-closing tags.

Version info:

  • SVGO Version: 3.0.2
  • NodeJs Version: v19.2.0
  • OS: Linux
@rnwst rnwst added the bug label Dec 18, 2022
@rnwst
Copy link
Author

rnwst commented Dec 18, 2022

Turns out this is a duplicate of #1473. That issue also points out that SVGO treats whitespace inside <foreignObject> elements incorrectly, something I hadn't noticed initially, but is still an issue in version 3.0.2.

@johnkenny54
Copy link
Contributor

I don't think the self-closing tag is invalid. The content within a <foreignObject> has to be XML, which means it's XHTML, not HTML, so <span/> should be equivalent to <span></span>.

There is still a problem with whitespace being trimmed, and as pointed out in #1678, the safest solution is to preserve all white space within a <foreignObject>.

@GreLI
Copy link
Member

GreLI commented Aug 20, 2024

Yep, <foreignObject> is a different world, which SVGO knows nothing about. That's not a bug, since special handling of <foreignObject> was never implemented (as far as I concerned). PRs are welcome.

@johnkenny54 johnkenny54 mentioned this issue Aug 20, 2024
Open
@rnwst
Copy link
Author

rnwst commented Nov 1, 2024

@johnkenny54 This depends on where the SVG is located. If it is loaded as an external resource of type image/svg+xml, then you are right, the content of the foreignObject will be parsed as XML/XHTML. If, however, the SVG is inlined, then the SVG is parsed by the browser's HTML parser (which, for the purpose of parsing the SVG, switches to a 'foreign content' mode, where /> is actually meaningful). But if foreignObjects are inside inlined SVGs, they will be parsed as HTML (no longer in 'foreign content' mode, so /> is meaningless), not XHTML. This is causing the bug above.

Since SVGO appears to be built on top of an XML parser, I think the easiest fix for this bug might just be to leave foreignObject elements untouched. I believe SVGO's first priority should be correctness, not compression ratio.

@johnkenny54
Copy link
Contributor

@rnwst - thanks for this info. I went down some other rabbit holes and didn't look at this embedded in an HTML file.

I totally agree that SVGO's first priority should be correctness. Unfortunately, this is not the case. I've been working on a version that prioritizes lossless compression - https://www.npmjs.com/package/svgo-ll - the latest version of svgo-ll handles the situation raised by this issue.

As you point out, the only reliable way to deal with this is to preserve everything within the <foreignObject>. With SVGO in its current state, this is not that easy to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@GreLI @johnkenny54 @rnwst