From 26d70d6fca06bb33cb11fa8a083ec7d61c15c3c7 Mon Sep 17 00:00:00 2001
From: Sean Harvey
Date: Fri, 14 Sep 2012 17:31:12 +1200
Subject: [PATCH] BUG HtmlEditorField doesn't save HTML fragments in HTMLValue
correctly
The issue was raised in #7628, where an anchor tag was being changed from
to 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.
---
model/HTMLValue.php | 23 +++++++++++++---------
tests/forms/HtmlEditorFieldTest.php | 30 +++++++++++++++--------------
tests/integration/HTMLValueTest.php | 12 +++++-------
3 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/model/HTMLValue.php b/model/HTMLValue.php
index e6d2cf805..83f68b979 100644
--- a/model/HTMLValue.php
+++ b/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 @@ class SS_HTMLValue extends ViewableData {
* @return string
*/
public function getContent() {
- // strip the body tags from the output (which are automatically added by DOMDocument)
- return preg_replace (
- array (
- '/^\s*]*>/i',
- '/<\/body[^>]*>\s*$/i'
- ),
- null,
- $this->getDocument()->saveXML($this->getDocument()->documentElement->lastChild)
+ // strip any surrounding tags before the and after the 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(
+ '/^/i',
+ '/(.*)/i',
+ '/<\/body>(.*)/i',
+ ),
+ '',
+ $this->getDocument()->saveHTML()
+ )
);
}
diff --git a/tests/forms/HtmlEditorFieldTest.php b/tests/forms/HtmlEditorFieldTest.php
index bd9e95a7b..cb11563ea 100644
--- a/tests/forms/HtmlEditorFieldTest.php
+++ b/tests/forms/HtmlEditorFieldTest.php
@@ -30,7 +30,7 @@ class HtmlEditorFieldTest extends FunctionalTest {
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 @@ class HtmlEditorFieldTest extends FunctionalTest {
public function testImageInsertion() {
$obj = new HtmlEditorFieldTest_Object();
- $editor = new HtmlEditorField('Content');
+ $editor = new HtmlEditorField('Content');
$editor->setValue('');
$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('');
$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("First Paragraph
Second Paragraph
");
+ $editor->setValue('First Paragraph
Second Paragraph
');
$editor->saveInto($obj);
- $this->assertEquals("First Paragraph
Second Paragraph
", $obj->Content);
+ $this->assertEquals('First Paragraph
Second Paragraph
', $obj->Content);
}
public function testSavingLinksWithoutHref() {
@@ -72,7 +74,7 @@ class HtmlEditorFieldTest extends FunctionalTest {
$editor->saveInto($obj);
$this->assertEquals (
- '
', $obj->Content, 'Saving a link without a href attribute works'
+ '
', $obj->Content, 'Saving a link without a href attribute works'
);
}
diff --git a/tests/integration/HTMLValueTest.php b/tests/integration/HTMLValueTest.php
index 4b505c907..600325adf 100644
--- a/tests/integration/HTMLValueTest.php
+++ b/tests/integration/HTMLValueTest.php
@@ -9,8 +9,8 @@ class SS_HTMLValueTest extends SapphireTest {
$value = new SS_HTMLValue();
$invalid = array (
'Enclosed Value
' => 'Enclosed Value
',
- '' => '',
- '' => '',
+ '' => '',
+ '' => '',
'/bodu>/body>' => '/bodu>/body>'
);
@@ -22,7 +22,7 @@ class SS_HTMLValueTest extends SapphireTest {
public function testInvalidHTMLTagNames() {
$value = new SS_HTMLValue();
- $invalid = array (
+ $invalid = array(
'',
'