fuel / email

Fuel PHP Framework - Fuel v1.x Email library
60 stars 38 forks source link

Auto generated Alt-Body has no line breaks #14

Closed felixkoch closed 12 years ago

felixkoch commented 12 years ago

Hey,

I think there are two approches to get linebreaks from html.

  1. Put a line break for each p, br ...
  2. Use the line breaks in the html code.

For me the 2nd approch gives the better results. So I changed driver.php/generate_alt(), please see comments:

    protected static function generate_alt($html, $wordwrap, $newline)
{
    // $html = preg_replace('/\s{2,}/', ' ', $html); This line deletes all linebreaks, not good
    $html = trim(strip_tags(preg_replace('/<(head|title|style|script)[^>]*>.*?<\/\\1>/s', '', $html)));
    $lines = explode($newline, $html);
    $result = array();
    $first_newline = true;
    foreach ($lines as $line)
    {
        $line = trim($line);
        if ( ! empty($line))
        {
            $result[] = $line;
        }
    }

    $html = join($newline, $result);
            $html = html_entity_decode($html,  ENT_COMPAT, 'UTF-8'); // This isn't HTML anymore

    return wordwrap($html, $wordwrap, $newline, false); // False is more consistent with static::wrap_text() (URLs will not be cut)
}

And in driver.php/send() I had to change one more line, because it eats the new line breaks :

    // Don't wrap the text when using quoted-printable
    if ($wrapping and ! $qp_mode)
    {
        $this->body = static::wrap_text($this->body, $wrapping, $newline, $is_html);
        $this->alt_body = static::wrap_text($this->alt_body, $wrapping, $newline, false); // to false, alt-body  is definitely not HTML 
    }

Regards Felix

floorish commented 12 years ago

@FrenkyNet any reason why you didn't add this edit? :

$this->alt_body = static::wrap_text($this->alt_body, $wrapping, $newline, false);

Since the alt_body definitely isn't html, so that last parameter should be false. For me, generated and manually added alt body still has no line breaks due to that call. With false as last parameter of wrap_text, it works.

frankdejonge commented 12 years ago

@floorish: https://github.com/fuel/email/commit/5b96ecccaee16b3c8d1a655e3f1da5570e493999 thanks!