Permalink
Browse files

BUG HtmlEditorField doesn't save HTML fragments in HTMLValue correctly

The issue was raised in #7628, where an anchor tag was being changed from
<a name="anchor"></a> to <a name="anchor"/> by SS_HTMLValue, when
HtmlEditorField::saveInto() parses the HTML fragments.

This is because SS_HTMLValue uses DOMDocument::saveXML(), which is fine
for saving an XML document, but not suitable for HTML. This fix changes
that to use DOMDocument::saveHTML() instead.
Note that we can't use the parameter to saveHTML() for selecting a single
node only, as that's only supported in PHP 5.3.6+, SilverStripe 3.0 supports
PHP 5.3.2 as a minimum. The workaround for this shortcoming is to replace
unncessary output by DOMDocument with a regular expression.
  • Loading branch information...
1 parent 7a8c35f commit 26d70d6fca06bb33cb11fa8a083ec7d61c15c3c7 @halkyon halkyon committed Sep 14, 2012
Showing with 35 additions and 30 deletions.
  1. +14 −9 model/HTMLValue.php
  2. +16 −14 tests/forms/HtmlEditorFieldTest.php
  3. +5 −7 tests/integration/HTMLValueTest.php
View
@@ -19,7 +19,7 @@ class SS_HTMLValue extends ViewableData {
public function __construct($content = null) {
$this->document = new DOMDocument('1.0', 'UTF-8');
$this->document->scrictErrorChecking = false;
-
+
$this->setContent($content);
parent::__construct();
@@ -29,14 +29,19 @@ public function __construct($content = null) {
* @return string
*/
public function getContent() {
- // strip the body tags from the output (which are automatically added by DOMDocument)
- return preg_replace (
- array (
- '/^\s*<body[^>]*>/i',
- '/<\/body[^>]*>\s*$/i'
- ),
- null,
- $this->getDocument()->saveXML($this->getDocument()->documentElement->lastChild)
+ // strip any surrounding tags before the <body> and after the </body> which are automatically added by DOMDocument
+ // note that we can't use the argument to saveHTML() as it's only supported in PHP 5.3.6+, we support 5.3.2 as a minimum
+ // in addition to the above, trim any surrounding newlines from the output
+ return trim(
+ preg_replace(
+ array(
+ '/^<!DOCTYPE.+?>/i',
+ '/(.*)<body>/i',
+ '/<\/body>(.*)/i',
+ ),
+ '',
+ $this->getDocument()->saveHTML()
+ )
);
}
@@ -30,7 +30,7 @@ public function testBasicSaving() {
public function testNullSaving() {
$obj = new HtmlEditorFieldTest_Object();
- $editor = new HtmlEditorField('Content');
+ $editor = new HtmlEditorField('Content');
$editor->setValue(null);
$editor->saveInto($obj);
@@ -39,29 +39,31 @@ public function testNullSaving() {
public function testImageInsertion() {
$obj = new HtmlEditorFieldTest_Object();
- $editor = new HtmlEditorField('Content');
+ $editor = new HtmlEditorField('Content');
$editor->setValue('<img src="assets/example.jpg" />');
$editor->saveInto($obj);
-
- $xml = new SimpleXMLElement($obj->Content);
- $this->assertNotNull($xml['alt'], 'Alt tags are added by default.');
- $this->assertNotNull($xml['title'], 'Title tags are added by default.');
-
+
+ $parser = new CSSContentParser($obj->Content);
+ $xml = $parser->getByXpath('//img');
+ $this->assertEquals('', $xml[0]['alt'], 'Alt tags are added by default.');
+ $this->assertEquals('', $xml[0]['title'], 'Title tags are added by default.');
+
$editor->setValue('<img src="assets/example.jpg" alt="foo" title="bar" />');
$editor->saveInto($obj);
-
- $xml = new SimpleXMLElement($obj->Content);
- $this->assertNotNull('foo', $xml['alt'], 'Alt tags are preserved.');
- $this->assertNotNull('bar', $xml['title'], 'Title tags are preserved.');
+
+ $parser = new CSSContentParser($obj->Content);
+ $xml = $parser->getByXpath('//img');
+ $this->assertEquals('foo', $xml[0]['alt'], 'Alt tags are preserved.');
+ $this->assertEquals('bar', $xml[0]['title'], 'Title tags are preserved.');
}
public function testMultiLineSaving() {
$obj = $this->objFromFixture('HtmlEditorFieldTest_Object', 'home');
$editor = new HtmlEditorField('Content');
- $editor->setValue("<p>First Paragraph</p><p>Second Paragraph</p>");
+ $editor->setValue('<p>First Paragraph</p><p>Second Paragraph</p>');
$editor->saveInto($obj);
- $this->assertEquals("<p>First Paragraph</p><p>Second Paragraph</p>", $obj->Content);
+ $this->assertEquals('<p>First Paragraph</p><p>Second Paragraph</p>', $obj->Content);
}
public function testSavingLinksWithoutHref() {
@@ -72,7 +74,7 @@ public function testSavingLinksWithoutHref() {
$editor->saveInto($obj);
$this->assertEquals (
- '<p><a name="example-anchor"/></p>', $obj->Content, 'Saving a link without a href attribute works'
+ '<p><a name="example-anchor"></a></p>', $obj->Content, 'Saving a link without a href attribute works'
);
}
@@ -9,8 +9,8 @@ public function testInvalidHTMLSaving() {
$value = new SS_HTMLValue();
$invalid = array (
'<p>Enclosed Value</p></p>' => '<p>Enclosed Value</p>',
- '<p><div class="example"></div></p>' => '<p/><div class="example"/>',
- '<html><html><body><falsetag "attribute=""attribute""">' => '<falsetag/>',
+ '<p><div class="example"></div></p>' => '<p></p><div class="example"></div>',
+ '<html><html><body><falsetag "attribute=""attribute""">' => '<falsetag></falsetag>',
'<body<body<body>/bodu>/body>' => '/bodu&gt;/body&gt;'
);
@@ -22,7 +22,7 @@ public function testInvalidHTMLSaving() {
public function testInvalidHTMLTagNames() {
$value = new SS_HTMLValue();
- $invalid = array (
+ $invalid = array(
'<p><div><a href="test-link"></p></div>',
'<html><div><a href="test-link"></a></a></html_>',
'""\'\'\'"""\'""<<<>/</<htmlbody><a href="test-link"<<>'
@@ -40,11 +40,9 @@ public function testInvalidHTMLTagNames() {
public function testMixedNewlines() {
$value = new SS_HTMLValue();
- $eol = "\n";
- $platformEOL = PHP_EOL; // native EOL for platform. Windows is \r\n (CR-LF). UNIX is LF
- $value->setContent("<p>paragraph</p>{$platformEOL}<ul><li>1</li>\r\n</ul>");
+ $value->setContent("<p>paragraph</p>\n<ul><li>1</li>\r\n</ul>");
$this->assertEquals(
- "<p>paragraph</p>{$eol}<ul><li>1</li>{$eol}</ul>",
+ "<p>paragraph</p>\n<ul><li>1</li>\n</ul>",
$value->getContent(),
'Newlines get converted'
);

0 comments on commit 26d70d6

Please sign in to comment.