search

mjmlio / mjml

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

MSO 2007 left aligns what should be centered if table style attr is valueless #1540

Open pixelsmithdev opened 5 years ago

pixelsmithdev commented 5 years ago

Describe the bug Valueless style attributes break centering in MSO 2007.

To be very clear it was breaking when it had a table with a style attr on it. Litmus testing without the style attribute and with an empty attribute both worked. style = bad style="" = good (no style) = good

To Reproduce Steps to reproduce the behavior:

  1. Create a file with this MJML code: 

    <mjml owa="desktop">

<mj-head>
    <mj-style inline="inline"></mj-style>
    <mj-attributes>
        <mj-all padding="0" />
    </mj-attributes>
    <mj-title>Broken rendering</mj-title>
</mj-head>

<mj-body width="600px" background-color="#e0e6ef">
    <mj-section
        text-align="center"
        background-color="#e0e6ef">
      <mj-column
        border="1px solid #000000">
        <mj-text
            padding="15px"
            font-size="13px"
            line-height="16px"
            align="center">
            Text with a border to show centering.
        </mj-text>
      </mj-column>
    </mj-section>
</mj-body>
</mjml>

    -- something to do with the combination of inline style/attribute settings in the head? altering that sometimes toggles the output style.

  2. Render it to HTML by doing: mjml whatever.mjml -o whatever.html
  3. Litmus test for MSO 2007

Expected behavior An output of tables with empty strings, or without the style attribute would solve this.

MJML environment (please complete the following information):

Email sending environment(for rendering issues):

Affected email clients (for rendering issues):

Screenshots MSO 2007: https://ol2007.capture.litmuscdn.com/8c418657-8d14-4b28-a50e-265496987433/results/ol2007-vertical-allowed-1366.png

MSO 2010 (and others): https://ol2010.capture.litmuscdn.com/8c418657-8d14-4b28-a50e-265496987433/results/ol2010-vertical-allowed-1366.png

General Preview: https://litmus.com/checklist/emails/public/68f7c9e Builder Code: https://litmus.com/builder/5fecca1

iRyusa commented 5 years ago

Hi,

I'm not able to reproduce your issue : https://mjml.io/try-it-live/HkNhYCPvV ? I don't see any style without content here, can you share the HTML ?

pixelsmithdev commented 5 years ago

Yeah, it occurs on line 109 of the builder code shown: https://litmus.com/builder/documents/3051831

I agree its not happening on your live preview, not sure what settings/ options would be different. https://www.youtube.com/watch?v=GRHfZYtzxdY&feature=youtu.be

Edit: better quality video https://www.youtube.com/watch?v=SATRoK2Lx6U&feature=youtu.be

This shows my MJML version and exactly the steps I use to recreate it. Sorry the quality is so bad. The terminal line starts by showing my MJML version is 4.3.1 and then I run mjml test.mjml -o test.html

I then search that file for style (with spaces around it) and find it on line 134.

iRyusa commented 5 years ago

In fact this is reproductible on 4.3.1, try it live was on 4.3.0, @kmcb777 can you check if we can fix this for next 4.4 beta ?

kmcb777 commented 5 years ago

i tried to reproduce with mjml4.4 and with the app which is on 4.3.1, but i always get style="", i never get just style, how did you reproduce exactly @iRyusa ?

iRyusa commented 5 years ago

Just installed the 4.3.1 locally and compiled with cli

DavideSuit commented 5 years ago

Just got the same issue (4.4.1): Screenshot 2019-08-09 at 15 35 28 Have a mj-include, including mj-style inline="inline" in the mj-head before mj-attributes. In the attributes I have mj-all with padding="0px". Removing the padding or the mj-style inline="inline" from the include make it works. Is not a fix by the way cause, of course, both are needed.

iRyusa commented 3 years ago

cc @kmcb777 We should remove that style attr if it's empty shouldn't be hard to do I think ?