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

[Jigal van Hemert]

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.

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' :-)

> * 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.

Together with Ernesto's mbox transport you can now catch any mails sent 
by TYPO3 (except those which call PHP mail() directly or have their own 
SMTP routines) and prevent them from being actually sent.

It would be nice to trace the t3lib_utility_Mail::mail() calls and use 
Swift Mailer directly. This will take some labour because the code 
creates its own mail headers, uses invalid sender addresses, etc.

Thanks for the review!

-- 
Kind regards / met vriendelijke groet,

Jigal van Hemert
skype:jigal.van.hemert
msn: jigal at xs4all.nl
http://twitter.com/jigalvh