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

Local images are never integrated but always treated as external #388

Closed
sdamrau opened this issue Dec 18, 2024 · 1 comment · Fixed by #389
Closed

Local images are never integrated but always treated as external #388

sdamrau opened this issue Dec 18, 2024 · 1 comment · Fixed by #389
Labels

Comments

@sdamrau
Copy link

sdamrau commented Dec 18, 2024

Contao: 4.13.50
NC: 2.2.3 (+NC pro 1.03)
PHP: 8.2.0

Local images in the email_html, like
<img src="files/imgs/logo.png">
are always converted into external paths, e.g.,
<img src="https://domain.tld/files/imgs/logo.png">.

Since there is no longer an option to explicitly select "external images," I would expect all local images to be embedded by default.

However, as I can see in the source code:

$html = $this->renderEmailTemplate($parcel);
$text = Html2Text::convert($html);
break;
case 'textAndHtml':
$html = $this->renderEmailTemplate($parcel);
$text = $this->replaceTokensAndInsertTags($parcel, $languageConfig->getString('email_text'));
break;
}
$stamp = $stamp->withText($text);
if ($html) {
$html = $this->embedImages($html, $stamp);
$stamp = $stamp->withHtml($html);
}

The method embedImages is called after renderEmailTemplate.
In renderEmailTemplate, the convertRelativeUrls function makes all URLs absolute. This causes the Virtual File System (VFS) to fail later when it tries to embed the images, as the files cannot be located anymore.

Wouldn't it make sense to call embedImages before convertRelativeUrls? Or alternatively, exclude local image paths from being converted to absolute URLs?

@Toflar
Copy link
Member

Toflar commented Dec 19, 2024

Fixed in #389.

@Toflar Toflar closed this as completed Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants