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.

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

<img> elements emitted by production builds are invalid XHTML (when compiler sets them using innerHTML) #5315

Closed
shirakaba opened this issue Aug 27, 2020 · 1 comment · Fixed by #5317

Comments

@shirakaba
Copy link

shirakaba commented Aug 27, 2020

Describe the bug

When building for production, <img> elements are emitted without the closing /. Note that this applies only to situations where the compiler chooses to insert them into the DOM using innerHTML.

Logs

Chrome:

Uncaught (in promise) DOMException: Failed to set the 'innerHTML' property on 'Element': The provided markup is invalid XML, and therefore cannot be inserted into an XML document.

Safari (far less useful):

Unhandled Promise Rejection: SyntaxError: The string did not match the expected pattern.

To Reproduce

First, you'll need to create a fresh Svelte project and adapt it to support XHTML. The easiest way I've found to do the latter is to convert your public/index.html file to valid XHTML:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
    <head>
        <meta charset='utf-8'/>
        <meta name='viewport' content='width=device-width,initial-scale=1'/>

        <title>Repro</title>

        <link rel='icon' type='image/png' href='./favicon.png'/>
        <link rel='stylesheet' href='./global.css'/>
        <link rel='stylesheet' href='./build/bundle.css'/>
        <script defer src='/build/bundle.js'></script>
    </head>
    <body class="hbbtv-viewport">
    </body>
</html>

... next, create a symlink named public/index.xhtml file that points to public/index.html:

ln -s public/index.html public/index.xhtml

Now insert the following code into src/App.svelte:

<div>
    <img alt="Star" width="100" height="100" src="http://mdn.mozillademos.org/files/12676/star.svg" />
</div>

You'll find that although the development build is fine, the production build will minify it to something along the lines of this (when pretty-printed):

(d = xa("div")).innerHTML = '<img alt="Star" width="100" height="100" src="http://mdn.mozillademos.org/files/12676/star.svg">',

Note how the closing / is removed from the img element when setting the innerHTML. This is what causes the XHTML syntax error.

If you visit http://localhost:5000/index.xhtml, you will see the error reproduce.

Expected behavior

Production builds should emit XHTML-valid components just like dev builds do. That is to say, the trailing / shouldn't be removed from the <img> component.

It would be acceptable to implement XHTML support as a configuration flag in Svelte, of course. Because it would increase build sizes for non-XHTML projects.

Stacktraces

See "logs" section above for the important part of the stack trace.

Information about your Svelte project:

  • Your browser and the version: Safari 13.1.2; Chrome 84.0.4147.135.

  • Your operating system: macOS 10.15.6

  • Svelte version 3.24.1

  • Whether your project uses Webpack or Rollup: Rollup

Severity

It took many hours to track down the cause of the error (Safari's error message did not indicate it was an XML issue at all), but it is not a total blocker, as a workaround is perfectly possible:

<script>
    const imgXHTMLHack = "img/logo_trimmed.png";
</script>

<div>
    <img alt="Star" width="100" height="100" src={imgXHTMLHack} />
</div>

By introducing a dependency on the <script> above, the compiler instead inserts the <img> element uses DOM operations like document.createElement('img') (which don't need to worry about XHTML validity) rather than setting innerHTML (which does require valid markup).

Additional context

From user Rainlife on the Svelte Discord (#svelte):

I think these lines

const void_element_names = /^(?:area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/;
are to blame, they define which elements are 'void', this method is then used in Element.ts to render them differently or something. Can't think of any workaround there, but maybe if you post in #internals or make a Github issue one of the maintainers can give a more comprehensive answer

@Conduitry
Copy link
Member

The optimized .innerHTML code for this should now be using /> for self-closing tags in 3.25.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants