Using StyledHtmlEmail breaks ForgotPasswordEmail

[bummzack]

When using the Injector to substitute Email with StyledHtmlEmail, trying to send a forgot password Email results in an empty mail with strange subject. Example: _=?UTF-8?B??=_

Seems to be an encoding issue, I'm trying to figure out what causes this.

[markguinn]

Do you have the charset set to UTF-8 in your SmtpMailer config?

[bummzack]

I'm not using SmtpMailer in this case.

[markguinn]

Ah. Emogrifier doesn't seem to play very well with non-utf8 email. I had similary types of issues until I explicitly set the charset on my mailer (SmtpMailer in my case).

[bummzack]

It seems to be another issue. Emogrifier should not be used at all here, since the password reset email template doesn't contain any styles.

And E-Mails with styles (eg. order receipts etc.) work fine.

[bummzack]

I think this is an injector issue… seems like replacing Email with StyledHtmlEmail messes with subclasses of Email, such as: Member_ForgotPasswordEmail. Instantiating a Member_ForgotPasswordEmail creates a StyledHtmlEmail instead.

$test = Member_ForgotPasswordEmail::create();
Debug::dump($test->class); // This will dump "StyledHtmlEmail"

So all the settings applied via Member_ForgotPasswordEmail (template and subject) are being discarded.

I guess this is more of a framework issue… but we could also create injector overrides for the email classes: Member_ChangePasswordEmail and Member_ForgotPasswordEmail. I'll give this a try and see if it works.

[bummzack]

Can confirm, that this is the issue.

Using something like this solves the problem:

// custom classes that extend StyledHtmlEmail
class ChangePasswordEmail extends StyledHtmlEmail {
    protected $from = '';   // setting a blank from address uses the site's default administrator email
    protected $subject = '';
    protected $ss_template = 'ChangePasswordEmail';

    public function __construct() {
        parent::__construct();
        $this->subject = _t('Member.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject');
    }
}

class ForgotPasswordEmail extends StyledHtmlEmail {
    protected $from = '';  // setting a blank from address uses the site's default administrator email
    protected $subject = '';
    protected $ss_template = 'ForgotPasswordEmail';

    public function __construct() {
        parent::__construct();
        $this->subject = _t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject');
    }
}

And then updating the injector configs like so:

Injector:
  Email:
    class: 'StyledHtmlEmail'
  Member_ChangePasswordEmail:
    class: 'ChangePasswordEmail'
  Member_ForgotPasswordEmail:
    class: 'ForgotPasswordEmail'

[bummzack]

I think something like this should be part of shop, otherwise it'll break password-reset mails for all shop instances?

[markguinn]

Shop is not replacing email. The injector service is called "ShopEmail" instead of "Email".

[markguinn]

See: https://github.com/silvershop/silvershop-core/blob/2.0/_config/config.yml#L23

[bummzack]

Ah you're right. Very well then. Maybe it would be still be worthwhile to raise a framework issue (so that framework doesn't use E-Mail subclasses). But that's not of your concern.

Should I close this one then?

[markguinn]

Why don't we leave it open both as documentation and as note that we may want to provide those subclasses for the system emails.