Skip to content

Commit

Permalink
fix line length for templated emails
Browse files Browse the repository at this point in the history
  • Loading branch information
euromark committed Apr 29, 2013
1 parent 65b1a94 commit ff0aa70
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/Cake/Network/Email/CakeEmail.php
View file
Expand Up @@ -319,8 +319,8 @@ class CakeEmail {

/**
* Constructor
* @param array|string $config Array of configs, or string to load configs from email.php
*
* @param array|string $config Array of configs, or string to load configs from email.php
*/
public function __construct($config = null) {
$this->_appCharset = Configure::read('App.encoding');
Expand Down Expand Up @@ -1566,6 +1566,12 @@ protected function _renderTemplates($content) {
$render = str_replace(array("\r\n", "\r"), "\n", $render);
$rendered[$type] = $this->_encodeString($render, $this->charset);
}

foreach ($rendered as $type => $content) {
$rendered[$type] = $this->_wrap($content);

This comment has been minimized.

Sign in to view

Copy link
@SimonEast

SimonEast May 23, 2013

It seems that since wrapping was introduced here that email templates break when tags are split across lines, e.g.

<th align="right" valign="top"
        style="font-weight: bold">Event</th>

These kind of tags now get completely mangled.

This comment has been minimized.

Sign in to view

Copy link
@dereuromark

dereuromark May 23, 2013

Member

What exactly is happening? Please post the result or test case to proof that. I just wrote a test with your snippet and it seems to be working just fine. I will submit it later as test case.

But what I could find out is, that wrapping (stay below required line length) does not work for this line if it is too long. Lines starting with whitespace seem to wrap too late.
see #1297

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory May 23, 2013

Member

@SimonEast That is valid HTML though.

This comment has been minimized.

Sign in to view

Copy link
@SimonEast

SimonEast May 23, 2013

Yes, it is valid HTML.

Anyway, I did further investigation this morning, and it's broken in v2.3.5 (completely mangles many of our emails), but fixed on Github master, appears to have been fixed inadvertently in a562d9c.

Here is some test code that demonstrates the bug. Not sure whether it needs to be added to the test cases.

App::uses('CakeEmail', 'Network/Email');
class CakeEmail2 extends CakeEmail {
    // Just to make this function public
    public function _wrap($message, $wrapLength = CakeEmail::LINE_LENGTH_MUST) {
        return parent::_wrap($message, $wrapLength);
    }
}

debug(CakeEmail2::_wrap('<a href="someLink"
style="font-weight: bold; color: red; margin: 0"></a>'));

...completely misses the opening <a> tag, and produces...

array(
    (int) 0 => 'style="font-weight: bold; color: red; margin: 0"></a>',
    (int) 1 => ''
)

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory May 24, 2013

Member

If it is fixed on master, it'll be part of the next release, which is only a week or so away.

This comment has been minimized.

Sign in to view

Copy link
@SimonEast

SimonEast May 24, 2013

Awesome. Thanks Mark. 👍
$rendered[$type] = implode("\n", $rendered[$type]);
$rendered[$type] = rtrim($rendered[$type], "\n");
}
return $rendered;
}

Expand Down
24 changes: 24 additions & 0 deletions lib/Cake/Test/Case/Network/Email/CakeEmailTest.php
View file
Expand Up @@ -1081,6 +1081,30 @@ public function testSendRenderThemed() {
$this->assertContains('To: ', $result['headers']);
}

/**
* testSendRenderWithHTML method and assert line length is kept below the required limit
*
* @return void
*/
public function testSendRenderWithHTML() {
$this->CakeEmail->reset();
$this->CakeEmail->transport('debug');

$this->CakeEmail->from('cake@cakephp.org');
$this->CakeEmail->to(array('you@cakephp.org' => 'You'));
$this->CakeEmail->subject('My title');
$this->CakeEmail->config(array('empty'));
$this->CakeEmail->emailFormat('html');
$this->CakeEmail->template('html', 'default');
$result = $this->CakeEmail->send();

$this->assertTextContains('<h1>HTML Ipsum Presents</h1>', $result['message']);
$lines = explode("\n", $result['message']);
foreach ($lines as $line) {
$this->assertTrue(strlen($line) <= CakeEmail::LINE_LENGTH_MUST);
}
}

/**
* testSendRenderWithVars method
*
Expand Down
8 changes: 8 additions & 0 deletions lib/Cake/Test/test_app/View/Emails/html/html.ctp
View file
@@ -0,0 +1,8 @@
<h1>HTML Ipsum Presents</h1><p><strong>Pellentesque habitant morbi tristique</strong> senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. <em>Aenean ultricies mi vitae est.</em> Mauris placerat eleifend leo. Quisque sit amet est et sapien ullamcorper pharetra. Vestibulum erat wisi, condimentum sed, <code>commodo vitae</code>, ornare sit amet, wisi. Aenean fermentum, elit eget tincidunt condimentum, eros ipsum rutrum orci, sagittis tempus lacus enim ac dui. <a href="#">Donec non enim</a> in turpis pulvinar facilisis. Ut felis.</p><h2>Header Level 2</h2><ol><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ol><blockquote><p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus magna. Cras in mi at felis aliquet congue. Ut a est eget ligula molestie gravida. Curabitur massa. Donec eleifend, libero at sagittis mollis, tellus est malesuada tellus, at luctus turpis elit sit amet quam. Vivamus pretium ornare est.</p></blockquote><h3>Header Level 3</h3><ul><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ul>
<pre><code>
#header h1 a {
display: block;
width: 300px;
height: 80px;
}</code></pre>
<p>Some more <b>Bold</b> test.</p>

0 comments on commit ff0aa70

Please sign in to comment.