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.

Sign up for GitHub

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

Jump to bottom

mj-raw incorrectly omits closing tag #319

Closed
clj opened this issue Aug 3, 2023 · 3 comments · Fixed by #320
Closed

mj-raw incorrectly omits closing tag #319

clj opened this issue Aug 3, 2023 · 3 comments · Fixed by #320
Labels
bug Something isn't working

Comments

@clj
Copy link

clj commented Aug 3, 2023 

<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
Copy link
Owner

jdrouet commented Aug 5, 2023

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
Copy link
Author

clj commented Aug 5, 2023

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
Copy link
Owner

jdrouet commented Aug 5, 2023

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

@jdrouet jdrouet added the bug Something isn't working label Aug 5, 2023
@jdrouet jdrouet mentioned this issue Aug 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(mrml-core): script elements should remain with open and close element jdrouet/mrml
2 participants
@clj @jdrouet