search

jdrouet / mrml

Implementation of mjml in rust
MIT License
336 stars 21 forks source link

mj-raw incorrectly omits closing tag #319

Closed clj closed 1 year ago

clj commented 1 year ago 
<mjml>
  <mj-body>
    <mj-section>
      <mj-column>
        <mj-raw><script src="http://example.com/hello.js"></script></mj-raw>
        <mj-text>
          Hello World!
        </mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>

mjml.io output fragment:

<script src="http://example.com/hello.js"></script>

mrml output fragment

<script src="http://example.com/hello.js" />

The documentation states that for script tags:

Neither tag is omissible.

This can be worked around by using inline documentation for external scripts:

<mj-raw><script src="http://example.com/hello.js">//</script></mj-raw>

which causes the end tag to remain when rendering using mrml. This is however inconsistent with the behaviour of mjml.io.

(Before someone tells me not to include script tags in emails, I'm not! 😄 But I am displaying rendered content in a browser, where I use script tags)

jdrouet commented 1 year ago

Yeah, the implemented html renderer will write a closing element if there is no children. I understand that it's not consistent in term of html but in term of visual, it should stay the same. I should mention that in the readme though 👍

If you have another point of view about it, feel free to share it with me, I'd be happy to reconsider my choice 😉

clj commented 1 year ago

Chrome (and Safari, I guess WebKit, haven't tried Firefox) disagrees on the visual side 😄 (sorry I should have added this to the original ticket)

Here is the HTML that Chrome constructs when rendering the HTML generated by mrml from the above snippet:

<body style="word-spacing:normal;">
  <div>
    <!--[if mso | IE]><table align="center" cellpadding="0" width="600" border="0" role="presentation" cellspacing="0" style="width:600px;"><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]-->
    <div style="margin:0px auto;max-width:600px;">
      <table cellspacing="0" align="center" role="presentation" cellpadding="0" border="0" style="width:100%;">
        <tbody>
          <tr>
            <td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
              <!--[if mso | IE]><table border="0" cellpadding="0" role="presentation" cellspacing="0"><![endif]--><!--[if mso | IE]><tr><![endif]--><!--[if mso | IE]><td><![endif]-->
              <script
                src="http://example.com/hello.js"><!--[if mso | IE]></td><![endif]--> <!--[if mso | IE]><td style="vertical-align:top;width:300px;"><![endif]-->
              <div class="mj-outlook-group-fix mj-column-per-50"
                style="font-size:0px;text-align:left;direction:ltr;display:inline-block;vertical-align:top;width:100%;">
                <table cellspacing="0" role="presentation" width="100%" border="0" cellpadding="0"
                  style="vertical-align:top;">
                  <tbody>
                    <tr>
                      <td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
                        <div
                          style="font-family:Ubuntu, Helvetica, Arial, sans-serif;font-size:13px;line-height:1;text-align:left;color:#000000;">
                          Hello World!
                        </div>
                      </td>
                    </tr>
                  </tbody>
                </table>
              </div>
              <!--[if mso | IE]></td><![endif]-- >< !--[if mso | IE]></tr >< ![endif]-- >< !--[if mso | IE]></table >< ![endif]-- >
            </td >
          </tr >
        </tbody >
      </table >
    </div >< !--[if mso | IE]></td ></tr ></table >< ![endif]-- >
  </div >
</body >

</html >
                </script>
</td>
</tr>
</tbody>
</table>
</div>
</div>
</body>

The browser tries to close the script tag, but in doing so captures the rest of the content. The html generated by mrml therefore renders as a blank page instead of showing "Hello World".

jdrouet commented 1 year ago

interesting! I will move that up then. I should also put in place some browser testing on the CI