mjmlio / mjml

MJML: the only framework that makes responsive-email easy
https://mjml.io
MIT License
17.03k stars 957 forks source link

Bug in rendering when using void tags inside <mj-text> with defined <mj-html-attributes> #2437

Open MichaelPetrinolis opened 2 years ago

MichaelPetrinolis commented 2 years ago

Describe the bug If you add void tags eg. <br> inside an <mj-text> in a template with <mj-html-attribute>, the html produced adds as many </br>'s as the ones in <mj-text> at the end of the <mj-text> contents. This causes rendering issues in the produced html

To Reproduce copy the following in https://mjml.io/try-it-live check the produced html. Delete the mj-html-attribute, the produced html changes.

<mjml>
  <mj-head>
    <mj-html-attributes>
      <mj-selector path=".test-unused-class">
        <mj-html-attribute name="test-attribute">true</mj-html-attribute>
      </mj-selector>  
    </mj-html-attributes>
  </mj-head>
  <mj-body>
    <mj-section>
     <mj-column>
      <mj-text> 
          <br>
           <hr>Between 1 BR<hr>
          <br>
      </mj-text>
     </mj-column>
    </mj-section>
    <mj-section>
     <mj-column>
      <mj-text> 
          <br>
          <br>
           <hr>
             Between 2 BRs and 2 HRs
           <hr>
          <br>
          <br>
      </mj-text>
     </mj-column>
    </mj-section>      
  </mj-body>
</mjml>

Expected behavior It should not render the additional closing tags. Use the following template (which uses void tags as self closing) and delete the mj-html-attributes , the additional tags are not rendered and there is no change in the rendered html

<mjml>
  <mj-head>
    <mj-html-attributes>
      <mj-selector path=".test-unused-class">
        <mj-html-attribute name="test-attribute">true</mj-html-attribute>
      </mj-selector>  
    </mj-html-attributes>
  </mj-head>
  <mj-body>
    <mj-section>
     <mj-column>
      <mj-text> 
          <br/>
           <hr/>Between 1 BR<hr/>
          <br/>
      </mj-text>
     </mj-column>
    </mj-section>
    <mj-section>
     <mj-column>
      <mj-text> 
          <br/>
          <br/>
           <hr/>
             Between 2 BRs and 2 HRs
           <hr/>
          <br/>
          <br/>
      </mj-text>
     </mj-column>
    </mj-section>      
  </mj-body>
</mjml>

MJML environment (please complete the following information):

Additional context Our templates where rendered with issues, after adding the mj-html-attributes tag in mj-head

MichaelPetrinolis commented 2 years ago

after some debug, it seems that we parse the content in cheerio using xmlMode https://github.com/mjmlio/mjml/blob/f74b71880261b9550c8038323e88f0e099d598d1/packages/mjml-core/src/index.js#L343 although the existing content is valid html and not xml. Setting xmlmode to false does not reproduce the issue. Should I send a PR or this will break something else ?

iRyusa commented 2 years ago

That's a sad and known issue with html-attributes. We should add a warning for that (and won't work too with templating langage with lookalike tag). I'm not sure about the non-xml mode as it does really weird things with templating stuff if I recall. Maybe it's worth investigate to see if we have a better solution :/

jarodium commented 2 years ago

Is it possible to use a custom element framework to render stuff?

I was thinking along the lines of having puppetter & https://minze.dev/ ( for example ) do the rendering ( i am aware of the implications; refactoring, memory and space consumptions, performance ).

iRyusa commented 2 years ago

I don't think our current architecture would support such thing

ehsky commented 1 year ago

Any updates on this issue?

iRyusa commented 1 year ago

Cheerio has no alternatives sadly, so there's nothing we can do yet