Incorrect closing inline SVG elements after compilation with static-i18n

sergey-pimenov commented 6 years ago

Hi! This tool awesome, but I found strange bug: SVG elements like path lose closing '/' after compilation. This problem also exist in gulp plugin.

Example.

This code:

<!-- i18n -->
<h1 data-t="my.key"></h1>
<p data-t>other.key</p>
<input type="submit" data-attr-t value-t="other.ok">

<!-- Some inline SVG -->
<svg width="200" height="200">
      <path d="M10 10"/>
      <circle cx="10" cy="10" r="2" fill="red"/>
      <ellipse cx="60" cy="60" rx="50" ry="25"/>
</svg>

Will compiled to:

<!-- i18n -->
<h1>Salut</h1>
<p>mec</p>
<input type="submit" value="confirmer">

<!-- Some inline SVG -->
<svg width="200" height="200">
      <path d="M10 10">
      <circle cx="10" cy="10" r="2" fill="red">
       <ellipse cx="60" cy="60" rx="50" ry="25">
</svg>

Link to example

It's not valid HTML and so it can cause some errors in browsers. It would be great if it's fixed.

danhper commented 6 years ago

The issue seems to come from https://github.com/fb55/htmlparser2 I added a --xml option, could you try to see if you get the output you expect when passing it please? Thanks!

sergey-pimenov commented 6 years ago

I try to repeate compile with your fix and unfortunately it's not work for me.

This:

<html>
  <head>
    <title data-t data-t-interpolate>{{main.chapter}} 1</title>
    <script src="js/app.js"></script>
  </head>
  <body>
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 68 65">
      <path d="M14 27v-20c0-3.7-3.3-7-7-7s-7 3.3-7 7v41c0 8.2 9.2 17 20 17s20-9.2 20-20c0-13.3-13.4-21.8-26-18zm6 25c-3.9 0-7-3.1-7-7s3.1-7 7-7 7 3.1 7 7-3.1 7-7 7z"/>
    </svg>
  </body>
</html>

Compile to:

<html>
  <head>
    <title>chapter 1</title>
    <script src="js/app.js"></script>
  </head>
  <body>
    <svg xmlns="http://www.w3.org/2000/svg" viewbox="0 0 68 65">
      <path d="M14 27v-20c0-3.7-3.3-7-7-7s-7 3.3-7 7v41c0 8.2 9.2 17 20 17s20-9.2 20-20c0-13.3-13.4-21.8-26-18zm6 25c-3.9 0-7-3.1-7-7s3.1-7 7-7 7 3.1 7 7-3.1 7-7 7z">
    </svg>
  </body>
</html>

I'm not familiar with the syntax npm-scripts (before I used gulp plugin) so i tried several variants:

static-i18n --xml -l en -i en -i fr www
static-i18n --xml true -l en -i en -i fr www
static-i18n --xml:true -l en -i en -i fr www
static-i18n --xml=true -l en -i en -i fr www

As I understand the first or second variant is correct, but I tried other varians too. Am I doing something wrong?

Also i try to change default settings in your tool (file: static-i18n.js):

default: {
    useAttr: true,
    fixPaths: true,
    replace: false,
    removeAttr: true,
    xml: true // Change this
  }

But this also didn't help.

danhper commented 6 years ago

Thank you for the detailed report. Sorry, it was my mistake. It should now be fixed, please try to upgrade to the newest version.

sergey-pimenov commented 6 years ago

Great job, thank you very much for your attention! Now everything works fine. Do I understand correctly that Gulp and Grunt plugins automatically will use latest version of node-static-i18n?

hohertz commented 6 years ago

Unfortunately, the problem is not solved with Fix bug with xmlmode. As @tuvistavie has already written, the issue comes from htmlparser2.

There are already numerous open issues, for example:

svg parsing

... and possibly a solution that has not yet been merged:

Use xmlmode for svg

In short: Inline SVG should always be parsed with the XML option enabled even if the option is disabled for the rest of the HTML page.

connected commented 6 years ago

It's an issue with the dom-serializer that is used in cheeriojs module which is used in static-i18n to parse/modify DOM of the html source files.

I was able to fix this issue by forcing xml mode on svg tag children.

Please see my PR for more details. Not sure if it will be merged, but it's worth a shot.

I was not able to override module deps. using lock file, but as for temporary solution you can patch faulty module and hot-swap using local mock.

require('mock-require')('dom-serializer', require('./vendor/dom-serializer'));

osdiab commented 6 years ago

While --xml does fix the SVG tags, it also causes script tags to be self closing, which doesn't work and therefore causes my site to not render properly. Any thoughts? I need to have script tags since I'm using this for an AMP website, which requires script tags in the document.

danhper commented 6 years ago

@osdiab I think the workaround proposed by @connected is the best shot for now. Let's hope the PR gets merge.

connected commented 5 years ago

I think this issue can be closed because it has been fixed since static-i18n updated cheerio dependency to v0.22.0.

Be careful with the gulp package though, it (v0.3.0) still uses the old static-i18n version which has this SVG bug.

claudetech / node-static-i18n

Incorrect closing inline SVG elements after compilation with static-i18n #20