\Nette\Mail: Returned subject empty if set via <title> tag (Nette 2.0.12)

JanB

Hi guys,

If e-mail subject is set via <title></title> via this command:

$mail->setHtmlBody($template);

Then any of below commands won't return subject set in $template by using <title> tag:

$mail->getSubject();
$mail->getHeader('Subject');

Template:

<html>
    <head>
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
        <title>test subject</title>
    </head>
    <body>test</body>
</html>

However sent e-mail has subject, only mentioned methods do not return it. Can you please fix it?

Thank you very much,

J.

Last edited by JanB (2013-10-15 20:18)

Jan Tvrdík

@JanB: This is probably a feature.

Majkl578

I think it should behave like JanB expects though i don't know at which point extraction should happen.

JanB

Hi guys,

I'm checking subject after “$this->mailer->send($mail);”. I want to display to the admin what e-mail has been sent to the user. All works but the subject…

I found this in \nette\Nette\Mail\Message.php:

	/**
	 * Builds HTML content.
	 * @return void
	 */
	protected function buildHtml()
	{
...<cut>...
		if (!$this->getSubject() && $matches = Strings::match($this->html, '#<title>(.+?)</title>#is')) {
			$this->setSubject(html_entity_decode($matches[1], ENT_QUOTES, 'UTF-8'));
		}
	}

As I'm only nette beginner, I'm not sure how to move on…
But as a workaround I'm thinking to use this code separately to get the subject…

Let me please know if you have any better ideas.

Thank you!

J.

duke

Majkl578 wrote:

I think it should behave like JanB expects though i don't know at which point extraction should happen.

I disagree. The subject has to remain unset, so that eventual change in template is reflected in actually generated subject.

JanB's workaround seems correct to me, but I guess it would help (it would remove redundant code) if that code (which is copied from Nette\Mail\Message::buildHtml method) was separated into special public method (e.g. Nette\Mail\Message::resolveSubject) which could then be called from outside.

Majkl578

duke wrote:

The subject has to remain unset, so that eventual change in template is reflected in actually generated subject.

But it is set. The only difference is it is set in the template.

duke

What I meant is that the $message->subject variable has to remain unset.
Theoretically method Message::getSubject() could contain this html-title extraction magic, but I would still prefer to use special method (such as Message::resolveSubject()) for that.

JanB

My (not nice but functional) workaround to get subject set by setSubject or by <title> is here:

$subject=$mail->getSubject();
if (!$subject && $matches = Strings::match($mail->getHtmlBody(), '#<title>(.+?)</title>#is')) {
	$subject=(html_entity_decode($matches[1], ENT_QUOTES, 'UTF-8'));
}

I think that title should be set by setHtmlBody() and not by send() method and if it supposed to be set by send() then send() should modify mail object so getSubject returns correct value if subject is set by <title>.

By the way duke you wrote:
“Message::getSubject() could contain this html-title extraction magic…”

I don't think so, I think “extraction magic” should be in the set function (setHtmlBody) so extraction does not have to be used every time subject is needed – it is a bit more efficient in this way…

Last edited by JanB (2013-10-29 08:46)

duke

JanB wrote:

My (not nice but functional) workaround to get subject set by setSubject or by <title> is here:
$subject=$mail->getSubject();
if (!$subject && $matches = Strings::match($mail->getHtmlBody(), '#<title>(.+?)</title>#is')) {
	$subject=(html_entity_decode($matches[1], ENT_QUOTES, 'UTF-8'));
}

I figured as much, as this is exactly how Nette does it internally. This is why I suggested that this logic is separated into resolveSubject method, so you could then do:

$subject = $mail->resolveSubject();

I think that title should be set by setHtmlBody() and not by send() method…

The setHtmlBody() method should do exactly what its name says, and nothing more. Also don't forget that the argument of this method can be instance of Nette\Templating\ITemplate and title in that template can be realized using a template variable, that can be changed afterwards.

…if it supposed to be set by send() then send() should modify mail object so getSubject returns correct value if subject is set by <title>.

Again, send() method should do exactly what its name says, ie. send the message, it definitely shouldn't modify the message.

By the way duke you wrote:
“Message::getSubject() could contain this html-title extraction magic…”

I don't think so, I think “extraction magic” should be in the set function (setHtmlBody) so extraction does not have to be used every time subject is needed – it is a bit more efficient in this way…

It can't be in setter, because then method setHtmlBody() would do something nobody expects from it (it would be magic) and because it would remove the feature related to templates which I spoke about already. Ideally it shouldn't be in getter either (hence I suggested separate method), but I would see that as a lesser evil than putting it in setHtmlBody() or send().

Principially I am happy with how it works now, except for one thing, which is that if you want to extract the actually used subject from Message with unassigned subject, you need to copy the inner code of Message class (ie. workaround you are using now). This leads to redundant code. To avoid the need for this redundancy, resolveSubject method could be used.

Last edited by duke (2013-10-29 19:10)