posthtml-mso icon indicating copy to clipboard operation
posthtml-mso copied to clipboard

feat request for posthtml-mso

Open Scrum opened this issue 5 years ago • 19 comments

Need to use it in posthtml-mso, so this:

<outlook><table><tr><td></outlook>

results in this:

<!--[if mso|ie]><table><tr><td><![endif]-->

and not in this:

<!--[if mso|ie]><table><tr><td></td></tr></table><![endif]-->

Scrum avatar Apr 27 '20 07:04 Scrum

Additional context:

It's common when coding HTML emails to use 'ghost tables' for Outlook - this is markup written inside conditional comments that target Outlook on Windows, so that it gets used only in that email client.

The most frequent usage is to define fixed-width tables for Outlook when coding mobile-first emails - a technique also known as 'hybrid' in email coding.

The problem with the parser right now is that it automatically closes any unclosed tag it encounters, making it impossible to use posthtml-mso for hybrid emails.

Basically this:

<outlook><table><tr><td></outlook>
  <div>Example</div>
<outlook></td></tr></table></outlook>

Is output like this:

<!--[if mso|ie]><table><tr><td></td></tr></table><![endif]-->
    <div>Example</div>

Notice also how </td></tr></table> doesn't make it in the final output (empty last line).

So the correct output would actually be:

<!--[if mso|ie]><table><tr><td><![endif]-->
    <div>Example</div>
<!--[if mso|ie]></td></tr></table><![endif]-->

cossssmin avatar Apr 27 '20 07:04 cossssmin

I think this proposal is good. But I strongly belive that the core posthtml libraries (posthtml, posthtml-parser and posthtml-renderer) should follow the specs as much as possible. also I am not familiar with the mso thing . is it a standard thing for html ? (I think its not right ?)

I would prefer to something like this in posthtml-mso instead of here cause there might be other plugins which needs custom HTML structure and posthtml 's renderer can be extended to plugins. so I would prefer that.

let me know. I may miss something.

anikethsaha avatar Apr 27 '20 08:04 anikethsaha

should follow the specs as much as possible

Agree 100%. Not sure how this would be done exactly, but basically what this needs is the unparsed contents of a tag that PostHTML touches - makes sense?

cossssmin avatar Apr 27 '20 08:04 cossssmin

but basically what this needs is the unparsed contents of a tag that PostHTML touches - makes sense?

still a bit lacking. can you give some more details. ? you meant those thing which are not in AST?

anikethsaha avatar Apr 27 '20 08:04 anikethsaha

I would prefer to something like this in posthtml-mso instead of here cause there might be other plugins which needs custom HTML structure and posthtml 's renderer can be extended to plugins. so I would prefer that.

At the moment, I am of the same opinion since something similar (Downlevel-revealed because the parser does not handle correctly) I had to implement in posthtml-beautify

https://github.com/posthtml/posthtml-beautify/blob/0d60579f22c16d1baf80ae609d81a879a2e9a696/src/index.js#L55-L92

Scrum avatar Apr 27 '20 08:04 Scrum

For now, move it to posthtml-mso

Scrum avatar Apr 27 '20 08:04 Scrum

@Scrum can we support officially a way to override the renderer like it is done for parser ?

@cossssmin it might be an overhead to implement here instead of posthtml-render. let me know if you need any hand 👍

anikethsaha avatar Apr 27 '20 08:04 anikethsaha

@anikethsaha

can we support officially a way to override the renderer like it is done for parser ?

I don't see any reason why we shouldn't do this, and what do you think is missing in the current render ?

Scrum avatar Apr 27 '20 08:04 Scrum

and what do you think is missing in the current render ?

as of now, I couldn't see anything missing. but for cases like this issue, it would be better to have an option. just to increase flexibility. developers can write their own renderer from scratch if they need or they can extend the current one which is done by posthtml-beautify and mentioned in the code here.

anikethsaha avatar Apr 27 '20 08:04 anikethsaha

The decisions made in posthtml-beautify were caused by the fact that the parser didn’t process correctly Downlevel-revealed, I have a solution that processes correctly Downlevel-revealed and many other problems in the new @posthtml/parse but I still can not get together and add it))

But I like the idea of allowing users to set their parser or be able to expand the current

Scrum avatar Apr 27 '20 08:04 Scrum

Thoughts for implementation

  1. First of all, it is necessary to designate the opening and closing tag

from

<outlook><div></outlook>

to

<outlook type="open"><div><outlook type="close">

thereby we get a tree


[
  {
    tag: "outlook",
    attrs: {type="open"},
    content: [
      {
        tag: "div",
        content: [
          {
            tag: "outlook",
            attrs: {type="close"}
          }
        ]
      }
    ]
  }
]
  1. mutate the tree in

[
  {
    tag: false,
    attrs: {},
    content: [
      '<!--[if mso|ie]>',
      {
        tag: "div",
        content: [
          {
            tag: false,
            attrs: {},
            content: [
              '<![endif]-->'
            ]
          }
        ]
      }
    ]
  }
]
  1. accordingly, after rendering
<!--[if mso|ie]><div><![endif]--></div>

As far as you remember, there is now access to tree.source You can take the source, make the necessary transformations in it, and it’s best to do this in advance, apply all the plugins that you can take tree.plugins apply to the transformed source and then apply your method to the resulting tree

Scrum avatar Apr 29 '20 07:04 Scrum

I would definitely not want to force the user into defining opening/closing tags through type attributes. This is an implementation detail that they shouldn’t have to care about.

At point 3) the desired output would be

<!--[if mso|ie]><div><![endif]-->

But I suppose I can remove everything after the closing endif 👍

cossssmin avatar Apr 29 '20 07:04 cossssmin

I would definitely not want to force the user into defining opening/closing tags through type attributes. This is an implementation detail that they shouldn’t have to care about.

The notation remains the same for the user, and all the magic described above happens inside.

Scrum avatar Apr 29 '20 07:04 Scrum

@cossssmin All attempts were in vain, the absence of a closing tag takes the tree deeper, its presence makes all the content content.

Scrum avatar May 08 '20 14:05 Scrum

@cossssmin Hi, what was the reason to close this issue?

Scrum avatar Mar 09 '21 06:03 Scrum

Based on your previous comment - if we can still tackle this, my bad 😬

cossssmin avatar Mar 09 '21 08:03 cossssmin

Based on your previous comment - if we can still tackle this, my bad 😬

We can, I will try to allocate time for this.

Scrum avatar Mar 09 '21 09:03 Scrum

@cossssmin Hi, I think it's worth trying the new option using for posthtml new version of render with option closeas

Scrum avatar May 27 '21 06:05 Scrum

Getting back to this after a while, this seems to do it for opening tags:

node.content = `<!--[if mso]>${tree.render(node.content, {
        singleTags: ['table', 'tr', 'td'],
      })}<![endif]-->`

      return node

... however it doesn't work on something like this because there is no node.content:

<outlook>
  </td></tr></table>
</outlook>

Not sure what can be done here, @Scrum any ideas?

cossssmin avatar May 31 '22 11:05 cossssmin