Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This file includes only changes we consider noteworthy for users, admins and plu
- Preserve the original message date on import of EML messages (#5559, #10251)
- OAuth: Validate JWT token signature (#10210)
- Use `X-Content-Type-Options:nosniff` for attachment previews and downloads (#10308)
- Fix extra `<div>` tags accumulating in the HTML body on every "Edit as New" cycle (#9919)

## Release 1.7.3

Expand Down
10 changes: 9 additions & 1 deletion program/actions/mail/compose.php
Original file line number Diff line number Diff line change
Expand Up @@ -998,11 +998,19 @@ public static function prepare_html_body($body, $wash_params = [])
'add_comments' => false,
];

if (self::$COMPOSE['mode'] == rcmail_sendmail::MODE_DRAFT) {
if (self::$COMPOSE['mode'] == rcmail_sendmail::MODE_DRAFT
|| self::$COMPOSE['mode'] == rcmail_sendmail::MODE_EDIT
) {
// convert TinyMCE's empty-line sequence (#1490463)
$body = preg_replace('/<p>\xC2\xA0<\/p>/', '<p><br /></p>', $body);
// remove <body> tags (not their content)
// Note: do not wrap the body in a container element here, otherwise
// an extra <div> would accumulate on every edit/send cycle (#9919).
// The body callback must be skipped too, otherwise it still wraps the
// (possibly style-carrying) <body> tag in a new <div> regardless of
// ignore_elements, since a registered callback takes precedence.
$wash_params['ignore_elements'] = ['body'];
$wash_params['skip_washer_body_callback'] = true;
} else {
$wash_params['container_id'] = $container_id;
}
Expand Down
4 changes: 3 additions & 1 deletion program/actions/mail/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,9 @@ public static function wash_html($html, $p, $cid_replaces = [])
self::$wash_html_body_attrs = [];

if (!empty($p['inline_html'])) {
$washer->add_callback('body', 'rcmail_action_mail_index::washtml_callback');
if (empty($p['skip_washer_body_callback'])) {
$washer->add_callback('body', 'rcmail_action_mail_index::washtml_callback');
}

if ($wash_opts['body_class']) {
self::$wash_html_body_attrs['class'] = $wash_opts['body_class'];
Expand Down
90 changes: 90 additions & 0 deletions tests/Actions/Mail/ComposeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

use Roundcube\Tests\ActionTestCase;

use function Roundcube\Tests\invokeMethod;
use function Roundcube\Tests\setProperty;

/**
* Test class to test rcmail_action_mail_compose
*/
Expand Down Expand Up @@ -38,4 +41,91 @@ public function test_quote_text()

$this->assertSame($expected, $result);
}

/**
* Invoke prepare_html_body() with a given compose mode and HTML body
*/
private function invoke_prepare_html_body($mode, $body)
{
$object = new \rcmail_action_mail_compose();

setProperty($object, 'COMPOSE', ['mode' => $mode], \rcmail_action_mail_compose::class);
setProperty($object, 'MESSAGE', (object) ['is_safe' => true], \rcmail_action_mail_compose::class);
setProperty($object, 'CID_MAP', [], \rcmail_action_mail_compose::class);

return invokeMethod($object, 'prepare_html_body', [$body], \rcmail_action_mail_compose::class);
}

/**
* Test that "Edit as New" (MODE_EDIT) does not wrap the body in a container
* element. Wrapping accumulates extra <div> tags on every edit/send cycle (#9919).
*/
public function test_prepare_html_body_edit_mode_has_no_container()
{
$result = $this->invoke_prepare_html_body(\rcmail_sendmail::MODE_EDIT, '<p>Hello world</p>');

$this->assertStringNotContainsString('editbody', $result);
$this->assertStringNotContainsString('<div', $result);
$this->assertStringContainsString('Hello world', $result);
}

/**
* Test that reply mode still wraps the (quoted) body in a container element,
* so the #9919 fix does not regress style isolation for replies.
*/
public function test_prepare_html_body_reply_mode_has_container()
{
$result = $this->invoke_prepare_html_body(\rcmail_sendmail::MODE_REPLY, '<p>Hello world</p>');

$this->assertStringContainsString('replybody', $result);
$this->assertStringContainsString('Hello world', $result);
}

/**
* Test that "Edit as New" (MODE_EDIT) does not wrap the body in a <div> even
* when the stored message's <body> tag carries a style attribute (as added by
* send.php's default_font/default_font_size wrapping on every send). Without
* this, the body callback still forwards the style attribute into a new <div>
* on every edit/send cycle, so the accumulation from #9919 continues, just
* without the "editbody" id (#9919).
*/
public function test_prepare_html_body_edit_mode_strips_body_style()
{
$body = '<html><head></head><body style="font-size: 10pt; font-family: Verdana,Geneva,sans-serif;">'
. "\r\n<p>Hello world</p></body></html>";

$result = $this->invoke_prepare_html_body(\rcmail_sendmail::MODE_EDIT, $body);

$this->assertStringNotContainsString('<div', $result);
$this->assertStringContainsString('Hello world', $result);
}

/**
* Test that reopening a saved draft (MODE_DRAFT) does not wrap the body in a
* <div> either, mirroring MODE_EDIT - a draft can be saved and reopened
* repeatedly just like Edit-as-New resends, so it is susceptible to the same
* accumulation (#9919).
*/
public function test_prepare_html_body_draft_mode_strips_body_style()
{
$body = '<html><head></head><body style="font-size: 10pt; font-family: Verdana,Geneva,sans-serif;">'
. "\r\n<p>Hello world</p></body></html>";

$result = $this->invoke_prepare_html_body(\rcmail_sendmail::MODE_DRAFT, $body);

$this->assertStringNotContainsString('<div', $result);
$this->assertStringContainsString('Hello world', $result);
}

/**
* Test that forward mode still wraps the body in a container element,
* so the #9919 fix does not regress style isolation for forwards.
*/
public function test_prepare_html_body_forward_mode_has_container()
{
$result = $this->invoke_prepare_html_body(\rcmail_sendmail::MODE_FORWARD, '<p>Hello world</p>');

$this->assertStringContainsString('forwardbody', $result);
$this->assertStringContainsString('Hello world', $result);
}
}
Loading