Emogrifier turns {} into %7B and %7D

[Johnny99211]

I'm currently setting up an email template which has two parts: The HTML and the CSS. To put everything together and merge the HTML with the CSS I'm using the PHP Emogrifier.

Now I've found out that when I emogrify the two elements that there is a problem with the uft-8 encoding.

All works like Ä, Ü, Ö are correctly displayed but when I put a link like this here into the content, the utf-8 encoding don't works for this element:

Before emogrify:

<a href="{password_reset_link}" id="button">Passwort zurücksetzten</a>

After emogrify:

<a href="%7Bpassword_reset_link%7D" id="button" style="color: #69f0ae; text-decoration: none; font-size: 20px; text-align: center; display: block; line-height: 1.7em !important; padding: .3em 1em;
> border: 2px solid #69f0ae; border-radius: 3px; -webkit-border-radius:
> 3px; -moz-border-radius: 3px; width: 40%; margin: auto; cursor:
> pointer !important;">Passwort zurücksetzten</a>

So as you can see the href is broken which is set with a placeholder which gets replaced with the correct link after the emogrify process:

%7Bpassword_reset_link%7D

[zoliszabo]

This is not a UTF-8 encoding issue, but the correct representation (encoding) of a href attribute value in HTML.

Depending on how you replace your template variables (placeholders), one quick solution would be to also encode the placeholder before the replace:

str_replace(rawurlencode('{placeholder}'), 'https://any.url.here');

[Johnny99211]

This is a bug in the emogrifier. I'm replacing the placeholder after the emogrify process but the placeholder is broken after merge css to the html with the help of emogrifier: #98

[Johnny99211]

I've temporarily fixed it with this workaround after the emogrify process but in my eyes this is a bug which needs to be fixed:

//Apply CSS styles inline for picky email clients.
try {
	$emogrifier = new Emogrifier( $message, $css );
	$message    = $emogrifier->emogrify();
} catch ( Exception $e ) {
	$logger = wc_get_logger();
	$logger->error( $e->getMessage(), array( 'source' => 'emogrifier' ) );
}
$message = str_replace( array( '%7B', '%7D' ), array( '{', '}' ), $message );

[zoliszabo]

This is a bug in the emogrifier. I'm replacing the placeholder after the emogrify process but the placeholder is broken after merge css to the html with the help of emogrifier: #98

Please note that Emogrifier uses DOM to work with HTML and the encoding we are talking about in this thread is a direct result of DOMDocument::saveHTML(). Please check here: DOMDocument::saveHTML() behaviour example.

You are right that this behaviour is messing up template variables, just like #98 also described this behaviour and it seems there was a solution (#170) for it back then, but apparently that code was removed from the code since then.

We will revisit this in the future... to make preserving template variables more straightforward with Emogrifier.

Thank you for your feedback!

[oliverklee]

The code from #170 was not removed - we have only refactored the corresponding unit test:
https://github.com/MyIntervals/emogrifier/blob/master/tests/Unit/EmogrifierTest.php#L243

So #170 is still fixed, and this is a separate issue with the curly braces.

@Johnny99211 Would you be willing to create a PR that extends this existing test to curly braces (and also the corresponding tests for AbstractHtmlProcessor and CssInliner) and add a fix for this?

[oliverklee]

For the record, my educated guess is that for href attributes, the value is expected to be a URL, that DOMDocument::saveHTML() properly URL-encodes the URL again. So I'm not sure there is a clean way of fixing/breaking this in Emogrifier (depending on whether creating URLs that are not properly encoded are considered a bug or a feature).

[zoliszabo]

For the record, my educated guess is that for href attributes, the value is expected to be a URL, that DOMDocument::saveHTML() properly URL-encodes the URL again. So I'm not sure there is a clean way of fixing/breaking this in Emogrifier (depending on whether creating URLs that are not properly encoded are considered a bug or a feature).

Yes, this is exactly what I was trying to describe above. It is definitely not a bug from Emogrifier, since this behaviour is inherited from DOMDocument and it actually is the right way to do it.

However, given the fact that in many cases Emogrifier is used in a combination with some sort of templating system (which may use URL-encodable characters as template variable delimiters), we should see whether there would be some straightforward solution to not "break" these template variables during processing.

[oliverklee]

Maybe we can add some HtmlProcessor subclass that has two new methods: renderWithTemplateMarkers and renderBodyWithTemplateMarkers that take a string of characters that should not be URL-encoded (i.e., for which the URL-encoded variant will be replaced with the verbatim characters), having some characters as default value for that parameter.

What do you think?

[JakeQZ]

Maybe we can add some HtmlProcessor subclass that has two new methods: ...

Architecturally this seems ok. In terms of implementation, there are a couple of possible approaches that may be viable and would avoid needing to specify which characters should not be URL-encoded:

1. Before calling saveHTML(), replace all href attributes with a uniquely-named attribute with the same value, e.g. data-emogrifier-href; then a str_replace() can be applied to the result to change these back to href (provided the chosen attribute name would not otherwise occur in text nodes or attribute values).
2. Use saveXML() (or save()) rather than saveHTML() then remove the processing instruction and perform any other conversions required to achieve the same result.

I don't know if this feature affects other attributes, such as src in img, or what happens if the attribute value is already correctly percent-encoded.

[kaaaaaaaaaaai]

i have same problems

[JakeQZ]

i have same problems

Unfortunately this is an issue with PHP DOM, which Emogrifier uses out of necessity.

There is no easy solution for this within Emogrifier. The correct place for a fix is within the PHP DOM component. The fix there would be fairly simple: remember what was read in, and if it hasn't been modified, write it out exactcly the same. I would suggest logging a bug against PHP, perhaps first looking for an existing match, at https://bugs.php.net/

Due to the intricacy of what may or may not need escaping, there is no realistic chance that we would be able to provide a solution within Emogrifier. @oliverklee, I think we should close this, for that reason, and mention it in the caveats in the readme.

Failing a fix in PHP DOM, your best bet would be one of the workarounds suggested early on in this thread.

[oliverklee]

I think we should close this, for that reason, and mention it in the caveats in the readme.

Agreed. Would you be willing to add this to the README, @JakeQZ ?

[JakeQZ]

I think we should close this, for that reason, and mention it in the caveats in the readme.

Agreed. Would you be willing to add this to the README, @JakeQZ ?

Will do. For now have added #1247 to capture this.