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

[BUG] background image quoted url turned into html entities #442

Closed
haxxxton opened this issue Feb 4, 2023 · 25 comments
Closed

[BUG] background image quoted url turned into html entities #442

haxxxton opened this issue Feb 4, 2023 · 25 comments
Labels
Help needed Issue that we need contributors for Package: @react-email/render Type: Bug Confirmed bug

Comments

@haxxxton
Copy link

haxxxton commented Feb 4, 2023

Some older email clients only resepect background image urls if the url is quoted (ie. background-image: url(./foo.jpg) will not work, but background-image: url("./foo.jpg") will). When using quoted urls in provided styles, the output from email export replaces either double quotes " or single quotes ' with html entities (" and ' respectively), breaking the inline style.

Steps to reproduce:

  • create a template with an inline style that contains a style that includes backgroundImage with a quoted url. eg. backgroundImage: "url('path/to/my/image.jpg')"
  • run email export
  • note that style is replaced with background-image: url('path/to/my/image.jpg')
@heysujal
Copy link

heysujal commented Feb 5, 2023

@zenorocha Can I work on this issue ? Can you guide me how to get started with this project ?

@bukinoshita
Copy link
Member

Feel free to grab @heysujal

@heysujal
Copy link

heysujal commented Feb 8, 2023

@bukinoshita When I make a react-email-stater using npx create-email@latest then it keeps showing Generating emails preview and keeps loading.
image
image
image
image

Can you guide me how to go about fixing this issue once I run this in local ?

@bastiaanv
Copy link
Contributor

@heysujal Can you try to place the react-email-starter-folder in a directory without spaces? Currently, the project is placed inside the New folder (2), I am guessing that the whitespaces in the path is the problem

@heysujal
Copy link

heysujal commented Feb 13, 2023

@bastiaanv thank you , I am able to run it my local system . Now, I am working to find a fix for the bug.

@heysujal
Copy link

heysujal commented Feb 13, 2023

@haxxxton I am able to reproduce the bug now. When the inline styles has url enclosed in double quote " like backgroundImage: 'url("http://localhost:3000/static/notion-logo.png")' then it does not produce any html entities.
But if we use single quote ' for the url something like this backgroundImage: "url('http://localhost:3000/static/notion-logo.png')" then the single quotes are replaced by html entities.
So, in my case I am only able to reproduce the bug when background image property is used in this way
backgroundImage: "url('http://localhost:3000/static/notion-logo.png')" .
While this is a bug that need to be fixed it still doesn't affect the output for me as I am able to export email which is same in both cases.
Let me know your thoughts on this.

@haxxxton
Copy link
Author

haxxxton commented Feb 14, 2023

@heysujal , i dont find that myself. when i do something with double quotes i end up with html entities like " instead.

For example:

// input
<div style={{ background: `no-repeat center bottom/contain url("${baseUrl}/foo.jpg") #000` }} />
<!-- output -->
<div style="background:no-repeat center bottom/contain url(&quot;https://example.com/foo.jpg&quot;) #000000"

This then breaks the rendered output such that no background image is displayed

@heysujal
Copy link

I think the string template literal that you are using is causing the problem which I understand, it will be used when we have to use variables in a string. I only tested using simple quote outside url and double quote for enclosing the url which does not break the output.
Now I will try to reproduce bug using template literal the way you mentioned in above comment.

@bukinoshita
Copy link
Member

Would be great if anyone could help on this 🙏

@heysujal
Copy link

heysujal commented Feb 26, 2023

Would be great if anyone could help on this 🙏

@bukinoshita
I asked for help in discord and explained my issue as well. Can you have a look again at my message if I give you the link

@bukinoshita
Copy link
Member

Would be great if anyone could help on this 🙏

@bukinoshita I asked for help in discord and explained my issue as well. Can you have a look again at my message if I give you the link

Can you ping me there again, please? I probably missed. Or maybe we can add the case here as well so other people can related to the issue.

@haxxxton
Copy link
Author

haxxxton commented Feb 26, 2023

my work around at the moment, is a second typescript step to post-process the built file output from react-email. let me know if you'd like a gist of it.. its not the most elegant.. but it works

@bukinoshita
Copy link
Member

my work around at the moment, is a second typescript step to post-process the built file output from react-email. let me know if you'd like a gist of it.. its not the most elegant.. but it works

Would love to see so we can fix this, if you can share

@haxxxton
Copy link
Author

@bukinoshita @heysujal heres the fixer ts file. i have included how i run it, and my current package.json file. it basically just does regex replacements within the file

https://gist.github.com/haxxxton/1ae485b1896eed22d8f16808de696a02

@bukinoshita
Copy link
Member

@heysujal Can you take a look that and maybe this should be part of the export command?

@heysujal
Copy link

heysujal commented Mar 1, 2023

@bukinoshita I am looking into it .My idea is to use unescape() function which is provided by typescript itself or else we can use lodash for the same.
@haxxxton can we connect on discord as I am looking for help to fix this issue ?
here is the discord thread https://discord.com/channels/1022242959736983613/1076557201482711180

@heysujal
Copy link

heysujal commented Mar 2, 2023

A similar bug was there in Signal App which I fixed.
@bukinoshita I think we can use unescape function like I did here.
But the only difficulty I am facing here is when I make a change in render package then how can I see if my changes are working ?

@bukinoshita
Copy link
Member

A similar bug was there in Signal App which I fixed.
@bukinoshita I think we can use unescape function like I did here.
But the only difficulty I am facing here is when I make a change in render package then how can I see if my changes are working ?

Here's for reference #504. Let me know if that works

@heysujal
Copy link

heysujal commented Mar 4, 2023

@bukinoshita that didn't work for me. We need clear instructions on how to build the project in the local system.

@bukinoshita
Copy link
Member

Yeah, you'll need to

  1. make the changes in the render component
  2. run yarn build to build the project
  3. yalc publish
  4. In a different project, run yalc add @react-email/render, check the package.json if it's referencing yalc package

@tdg5
Copy link
Contributor

tdg5 commented Jun 7, 2023

I may be mistaken, but I think this is a react-dom/server issue, not a react-email issue. The relevant issue is this one I think: facebook/react#13838.

Though I might ask, in what context is the background-image style broken? Is there a particular email client that has a problem with it? I tried exploring some and found that an element with an inline style like this seemed to work fine in most contexts:

<div style="background-image:url(&#x27;https://www.iana.org/_img/2022/iana-logo-header.svg&#x27;)"></div>

It's not an email client, but I've created this JSFiddle to demonstrate that HTML entities should be cooperative for inline styles, but are not cooperative for <style> elements: https://jsfiddle.net/c7dxbset/

A reference to an old spec that supports this notion:

Please note that style sheet data that is element content may not contain character references, but style sheet data that is the value of an attribute may contain them.
Basic HTML data types.

@aliraza556
Copy link

@haxxxton @nabby27 issue is available?

@nabby27
Copy link

nabby27 commented Sep 4, 2024

The bounty is available if the problem exists

@haxxxton
Copy link
Author

haxxxton commented Sep 5, 2024

@aliraza556 go for it my dude. I havent looked at this since i added my "fixer" script to modify the output, but i would assume it still an issue as the issue @tdg5 identified is still open

@gabrielmfern
Copy link
Collaborator

Closing in favor of #1767

@gabrielmfern gabrielmfern closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help needed Issue that we need contributors for Package: @react-email/render Type: Bug Confirmed bug
Projects
None yet
9 participants