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

Invalid nested HTML comments in generated MSO conditionals #370

Closed
clj opened this issue Jan 31, 2024 · 1 comment · Fixed by #372
Closed

Invalid nested HTML comments in generated MSO conditionals #370

clj opened this issue Jan 31, 2024 · 1 comment · Fixed by #372

Comments

@clj
Copy link

clj commented Jan 31, 2024

I just updated mjml-python from 1.2.3 to 1.3.0, (which as far as I can gather changes the linked MRML from version 2.0.0-rc3 to 3.0.0). This caused our do our emails parse as HTML canary to trip up on output generated from MJML code like this:

<mjml>
    <mj-body>
        <mj-section>
        <!-- A comment -->
        </mj-section>
    </mj-body>
</mjml>

I have cut the generated code to only show the relevant bits, but added newlines and indentation to the MRML generated code.

mjml-python 1.3.0 (mrml 3.0.0)

<td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
  <!--[if mso | IE]><table border="0" cellpadding="0" cellspacing="0" role="presentation"><tr><!-- A comment --></tr></table><![endif]-->
</td>

mjml-python 1.2.3 (mrml 2.0.0-rc3)

<td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
  <!--[if mso | IE]><table border="0" cellpadding="0" cellspacing="0" role="presentation"><![endif]--><!--[if mso | IE]><tr><![endif]-->
  <!-- A comment -->
  <!--[if mso | IE]></tr><![endif]--><!--[if mso | IE]></table><![endif]-->
</td>

mjml.io

<td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
  <!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><![endif]-->
  <!-- A comment -->
  <!--[if mso | IE]></tr></table><![endif]-->
</td>

It looks like mjml.io's implementation detects the comment and ends the MSO conditional, and that MRML prior to version 2.0.0 had the same behaviour. I'm not sure if the behaviour change in later versions is intentional, or if a dependency caused this change. However, the new behaviour might be surprising to users since it is different from that of the reference MJML implementation (and generates a nested comment, i.e. invalid HTML).

Personally I am considering stripping HTML comments before passing the template to MRML anyway.

Thanks for a great library though!

@jdrouet
Copy link
Owner

jdrouet commented Jan 31, 2024

thanks for this! I'll take a look 😜

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

Successfully merging a pull request may close this issue.

2 participants