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

Proposal: do not validate href attribute #1020

Closed
lessless opened this issue Mar 6, 2018 · 17 comments
Closed

Proposal: do not validate href attribute #1020

lessless opened this issue Mar 6, 2018 · 17 comments
Milestone

Comments

@lessless
Copy link

lessless commented Mar 6, 2018

Hello,

We are using mjml output as a template which should be populated with specific details for each recipient. Our template engine uses very common <%= %> notation to denote variables that should be populated later on. Also we want to use strict validation mode to know if something went wrong and our system produced invalid mjml but it seems to not be possible in v4.
Update: actually using <%= %> in the href attribute will cause a failure in any validation level, not only in strict

Reproduction Steps:

  1. Validate mjml containing

Expected behavior:

MJML should be valid

Observed behavior:

{"message":"URI malformed"} error is given

MJML version:

v 4.0

@iRyusa
Copy link
Member

iRyusa commented Mar 6, 2018

Hi @lessless

Thanks for reporting, It looks like a runtime error rather than a validation issue. We'll see what we can do about this

@MrJoeXu
Copy link

MrJoeXu commented Mar 12, 2018

Had the exact same issue here. This is a limitation when trying to generate any dynamic link in the template.

@trentclowater
Copy link

I also had the same issue, also when trying to generate a dynamic link in the template.

@batman774
Copy link

Hello.
Do you know when this issue will be solve ?

@lessless
Copy link
Author

Temporary workaround is to swap <%/%> to {{/}} and back

@iRyusa
Copy link
Member

iRyusa commented Mar 23, 2018

We're working on a fix for the parser, but it can leads to lot of issues.

So let's hope we can push the fix in 4.1 release. We might need to push an "experimental" tag to let people try before ship it to the release branch.

So no ETA about it yet

@batman774
Copy link

Thanks for your quick responses. I'm ok if you need, to test the 4.1 release on my template before push in the release branch

@batman774
Copy link

Hello @iRyusa , do you have any news about the fix ?
Thanks

@batman774
Copy link

Hello, do you have any update about this issue ?

@flozero
Copy link
Contributor

flozero commented Apr 15, 2018

you are on 4.0 on 4.0.3 ?

@batman774
Copy link

I try with
mjml-core: 4.0.1
mjml-cli: 4.0.3

There is the code generate the URIError : URI malformed
href="?referer=sitemobile&utm_source=<%= delivery.internalName.toLowerCase() %>"

@kmcb777
Copy link
Collaborator

kmcb777 commented Apr 17, 2018

Hi @batman774 , we changed the way we handle attributes so as to allow anything in attribute values. This will fix this issue.
This change will be included in the 4.1.0 version, which should be released within one or two weeks

@batman774
Copy link

Hi @kmcb777 , thanks for your quick response. I'm waiting :)

@ngarnier ngarnier added this to the 4.1.0 milestone Apr 24, 2018
@ngarnier ngarnier added this to Soon to be released in MJML Roadmap Apr 24, 2018
@batman774
Copy link

Hello @kmcb777
Any news about the 4.1.0 release ?

@ngarnier
Copy link
Member

Asking again and again doesn't help... we're doing our best, already providing releases at a quick pace: https://github.com/mjmlio/mjml/releases.

We just released a new version yesterday and have others in the pipe, you can check the milestones to see what's up: https://github.com/mjmlio/mjml/milestones.

No ETA.

@batman774
Copy link

batman774 commented Apr 26, 2018

Sorry for asking again. I'm sure you're doing your best
I'll follow the progression of the release on your milestones link

@iRyusa
Copy link
Member

iRyusa commented Jun 1, 2018

Beta is now available on NPM with yarn add mjml@next or npm install mjml@next please clean up your installation before installing some beta because some package aren't updated properly sometime with npm.

@batman774 @lessless feel free to test it and provide feedback about the fix. We need to check if we don't break anything on parser because it's really a critic point of MJML. So we'll release it when we're confident enough about the behavior

@iRyusa iRyusa closed this as completed in 5dcbfd0 Jun 28, 2018
MJML Roadmap automation moved this from Soon to be released to Released Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
MJML Roadmap
  
Released
Development

No branches or pull requests

8 participants