Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
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...
commit 26d70d6fca06bb33cb11fa8a083ec7d61c15c3c7 1 parent 7a8c35f
@halkyon halkyon authored
View
23 model/HTMLValue.php
@@ -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()
+ )
);
}
View
30 tests/forms/HtmlEditorFieldTest.php
@@ -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'
);
}
View
12 tests/integration/HTMLValueTest.php
@@ -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'
);
Please sign in to comment.
Something went wrong with that request. Please try again.