[TYPO3-core] RFC #16591 The new Swift Mailer should be used by TYPO3 for sending all kinds of mail

[Ernesto Baschny [cron IT]]

Jigal van Hemert schrieb am 01.12.2010 07:28:
> Hi,
> 
> On 30-11-2010 23:31, Jeff Segars wrote:
>> +1 on reading and testing, with a few comments for discussion :)
> 
> Thanks! I'll commit today.
> 
>> * Is the tx_ prefix needed on the new class so that the hook works as
>> expected? It seems a little out of place otherwise :)
> 
> t3lib_utility_Mail::mail() calls t3lib_div::callUserFunction() without a
> fourth parameter ($checkPrefix) it defaults to 'user_' or 'tx_'; out of
> those two prefixes, I preferred 'tx_'.
> 
>> * In the setHeader() method, would it make sense to refactor the parts
>> that process mailbox headers into a common method? Even if it stays as
>> is, a few comments about what it is doing at that point might be
>> helpful :)
> 
> It would only make sense to move the part from the preg_match up to and
> including the foreach loop which parses the mailbox headers to a
> separate method. The rest has hardly anything in common to refactor.
> I'll add comments when committing and try to refactor the parsing
> routine later (no-brainer IMO and easier to make some unit test for it).
> 
>> * Would it make sense to move unEscapeShellArg() to somewhere like
>> t3lib_div since its generally useful and not specific to mail
>> functionality? I would call it unescapeShellArg() also.
> 
> RMs? This is about expanding the API after feature freeze.

Would be no problem introducing a new method if it makes sense for this
new addition. The API is still in the "last stabilizing phase".

> It's less useful than it seems. In windows you can't really undo
> escapeshellarg() because it changes '%' into a space character.
> I interpreted the name as "un-escapeshellarg" hence the capital 'E' :-)

Maybe "revertEscapeShellArg()"?

>> * And my last question is a big picture conceptual one. It seems a
>> little bit funny that t3lib_utilityMail::mail() calls a hook or falls
>> back to the built-in mail() function when the core will always be
>> setting the hook. Would it make more sense to call the new sendMail()
>> function directly in the else case of t3lib_utilityMail::mail()
> 
> Please provide a patch!
> This was the easiest way to catch all calls to
> t3lib_utility_Mail::mail() and handle them with Swift Mailer. Also those
> done by (system) extensions are handled now.

That's true, but what Jeff noted is that the "else" part in the
t3lib_utility_mail will never be reached again. But I agree that this is
the easiest way to hook in that place without changing too much.

We should deprecate t3lib_utility_Mail::mail() in 4.5 and maybe add a
very simple one shot utility method that works more like "SwiftMail" in
our new classes:

t3lib_mail_Mailer::send($to, $subject, $otherHeaders, $textBody,
$htmlBody = '');

e.g.:

t3lib_mail_Mailer::send(
	array('jigal at xs4all.nl' => 'Jigal van Hemert'),
	'My subject',
	array('From' => array('ernesto at typo3.org' => 'Ernesto Baschny')),
	'My mail',
	'<p>My Mail</p>'
);

And having some default "otherHeaders" configurable via the Install Tool
(like "Organization", "From", "Reply-To", "Return-Path", "Bcc").

Cheers,
Ernesto