Skip to content

Commit 2740360

Browse files
FelixSchwarzFelixSchwarz
committed
keep comments by default (like the JS reference implementation)
1 parent d2b1647 commit 2740360

File tree

4 files changed

+190
-5
lines changed

4 files changed

+190
-5
lines changed

‎mjml/mjml2html.py‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from pathlib import Path, PurePath
44
from typing import TYPE_CHECKING, Any, NamedTuple, Optional, TypeVar, Union
55

6-
from bs4 import BeautifulSoup
6+
from bs4 import BeautifulSoup, Comment
77
from dotmap import DotMap
88

99
from mjml.elements.head._head_base import HeadComponent
@@ -34,6 +34,7 @@ def mjml_to_html(
3434
skeleton: Optional[str] = None,
3535
template_dir: Optional["StrPath"] = None,
3636
custom_components: Optional[Sequence[type["Component"]]] = None,
37+
keep_comments: bool = True,
3738
) -> ParseResult:
3839
register_core_components()
3940

@@ -127,10 +128,18 @@ def applyAttributes(mjml_element: Any) -> dict[str, Any]:
127128

128129
def parse(_mjml, parentMjClass: str='', *, template_dir: str) -> Any:
129130
tagName = _mjml.name
130-
is_comment = not isinstance(tagName, str)
131-
if is_comment:
132-
# XML comment: <cyfunction Comment at 0x…>
133-
# (this needs to be extended when "keepComments" should be implemented)
131+
if isinstance(_mjml, Comment) and keep_comments:
132+
comment_text = str(_mjml)
133+
return {
134+
'tagName': 'mj-raw',
135+
'content': f'<!--{comment_text}-->',
136+
'attributes': {},
137+
'globalAttributes': {},
138+
'children': [],
139+
}
140+
is_tag = isinstance(tagName, str)
141+
if not is_tag:
142+
# could be NavigableString (text/whitespace), etc.
134143
return None
135144
attributes = _mjml.attrs
136145
children = [child for child in _mjml]

‎tests/testdata/mjml-comment-merging-expected.html‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
<!doctype html>
2+
<html lang="und" dir="auto" xmlns="http://www.w3.org/1999/xhtml" xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office">
3+
4+
<head>
5+
<title></title>
6+
<!--[if !mso]><!-->
7+
<meta http-equiv="X-UA-Compatible" content="IE=edge">
8+
<!--<![endif]-->
9+
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
10+
<meta name="viewport" content="width=device-width, initial-scale=1">
11+
<style type="text/css">
12+
#outlook a {
13+
padding: 0;
14+
}
15+
16+
body {
17+
margin: 0;
18+
padding: 0;
19+
-webkit-text-size-adjust: 100%;
20+
-ms-text-size-adjust: 100%;
21+
}
22+
23+
table,
24+
td {
25+
border-collapse: collapse;
26+
mso-table-lspace: 0pt;
27+
mso-table-rspace: 0pt;
28+
}
29+
30+
img {
31+
border: 0;
32+
height: auto;
33+
line-height: 100%;
34+
outline: none;
35+
text-decoration: none;
36+
-ms-interpolation-mode: bicubic;
37+
}
38+
39+
p {
40+
display: block;
41+
margin: 13px 0;
42+
}
43+
44+
</style>
45+
<!--[if mso]>
46+
<noscript>
47+
<xml>
48+
<o:OfficeDocumentSettings>
49+
<o:AllowPNG/>
50+
<o:PixelsPerInch>96</o:PixelsPerInch>
51+
</o:OfficeDocumentSettings>
52+
</xml>
53+
</noscript>
54+
<![endif]-->
55+
<!--[if lte mso 11]>
56+
<style type="text/css">
57+
.mj-outlook-group-fix { width:100% !important; }
58+
</style>
59+
<![endif]-->
60+
<!--[if !mso]><!-->
61+
<link href="https://fonts.googleapis.com/css?family=Ubuntu:300,400,500,700" rel="stylesheet" type="text/css">
62+
<style type="text/css">
63+
@import url(https://fonts.googleapis.com/css?family=Ubuntu:300,400,500,700);
64+
65+
</style>
66+
<!--<![endif]-->
67+
<style type="text/css">
68+
@media only screen and (min-width:480px) {
69+
.mj-column-per-100 {
70+
width: 100% !important;
71+
max-width: 100%;
72+
}
73+
}
74+
75+
</style>
76+
<style media="screen and (min-width:480px)">
77+
.moz-text-html .mj-column-per-100 {
78+
width: 100% !important;
79+
max-width: 100%;
80+
}
81+
82+
</style>
83+
</head>
84+
85+
<body style="word-spacing:normal;">
86+
<div aria-roledescription="email" style="" role="article" lang="und" dir="auto">
87+
<!-- Comment 1 -->
88+
<!--[if mso | IE]><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]-->
89+
<div style="margin:0px auto;max-width:600px;">
90+
<table align="center" border="0" cellpadding="0" cellspacing="0" role="presentation" style="width:100%;">
91+
<tbody>
92+
<tr>
93+
<td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
94+
<!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="width:600px;" ><![endif]-->
95+
<div class="mj-column-per-100 mj-outlook-group-fix" style="font-size:0;line-height:0;text-align:left;display:inline-block;width:100%;direction:ltr;">
96+
<!--[if mso | IE]><table border="0" cellpadding="0" cellspacing="0" role="presentation" ><tr><td style="vertical-align:top;width:600px;" ><![endif]-->
97+
<div class="mj-column-per-100 mj-outlook-group-fix" style="font-size:0px;text-align:left;direction:ltr;display:inline-block;vertical-align:top;width:100%;">
98+
<table border="0" cellpadding="0" cellspacing="0" role="presentation" style="vertical-align:top;" width="100%">
99+
<tbody>
100+
<tr>
101+
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
102+
<div style="font-family:Ubuntu, Helvetica, Arial, sans-serif;font-size:13px;line-height:1;text-align:left;color:#000000;">lorem</div>
103+
</td>
104+
</tr>
105+
</tbody>
106+
</table>
107+
</div>
108+
<!--[if mso | IE]></td></tr></table><![endif]-->
109+
</div>
110+
<!--[if mso | IE]></td></tr></table><![endif]-->
111+
</td>
112+
</tr>
113+
</tbody>
114+
</table>
115+
</div>
116+
<!--[if mso | IE]></td></tr></table><![endif]-->
117+
<!-- Comment 2 -->
118+
<!--[if mso | IE]><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]-->
119+
<div style="margin:0px auto;max-width:600px;">
120+
<table align="center" border="0" cellpadding="0" cellspacing="0" role="presentation" style="width:100%;">
121+
<tbody>
122+
<tr>
123+
<td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
124+
<!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="vertical-align:top;width:600px;" ><![endif]-->
125+
<div class="mj-column-per-100 mj-outlook-group-fix" style="font-size:0px;text-align:left;direction:ltr;display:inline-block;vertical-align:top;width:100%;">
126+
<table border="0" cellpadding="0" cellspacing="0" role="presentation" style="vertical-align:top;" width="100%">
127+
<tbody>
128+
<tr>
129+
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
130+
<div style="font-family:Ubuntu, Helvetica, Arial, sans-serif;font-size:13px;line-height:1;text-align:left;color:#000000;">ipsum</div>
131+
</td>
132+
</tr>
133+
</tbody>
134+
</table>
135+
</div>
136+
<!--[if mso | IE]></td></tr></table><![endif]-->
137+
</td>
138+
</tr>
139+
</tbody>
140+
</table>
141+
</div>
142+
<!--[if mso | IE]></td></tr></table><![endif]-->
143+
</div>
144+
</body>
145+
146+
</html>

‎tests/testdata/mjml-comment-merging.mjml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<mjml>
2+
<!--
3+
This file ensures comments can be kept.
4+
If our implementation drops the comments and the reference implementation does
5+
not, some conditional comments will be merged, triggering a failed test.
6+
7+
In order to trigger that behavior we need two sections each with a comment
8+
preceding it.
9+
-->
10+
<mj-body>
11+
12+
<!-- Comment 1 -->
13+
<mj-section>
14+
<mj-group>
15+
<mj-column>
16+
<mj-text>lorem</mj-text>
17+
</mj-column>
18+
</mj-group>
19+
</mj-section>
20+
21+
<!-- Comment 2 -->
22+
<mj-section>
23+
<mj-column>
24+
<mj-text>ipsum</mj-text>
25+
</mj-column>
26+
</mj-section>
27+
28+
</mj-body>
29+
</mjml>

‎tests/upstream_alignment_test.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ def fixed_random_seed():
6767
'mjml-lang-attribute',
6868
'mjml-dir-attribute',
6969
'missing-whitespace-before-tag',
70+
'mjml-comment-merging',
7071
)
7172

7273
@pytest.mark.parametrize('test_id', TEST_IDS)

0 commit comments

Comments
 (0)