FIX Ensure comments are escaped in RSS feeds.

FIX Also fix up preview to only output the comment content rather than the whole template.

FIX Hide preview after posting comment.

API Move AllowHtml to field to prevent issues with altering Html configuration after comments have been posted.

FIX If moderation is turned on for commenting, still render comments in preview mode.

code/controllers/CommentingController.php

@@ -117,7 +117,13 @@ class CommentingController extends Controller {
 		$comments = new PaginatedList($comments, $request);
 		$comments->setPageLength(Commenting::get_config_value(null, 'comments_per_page'));
 
-		return new RSSFeed($comments, $link, $title, $link, 'Title', 'Comment', 'AuthorName');
+		return new RSSFeed(
+			$comments, 
+			$link, 
+			$title, 
+			$link, 
+			'Title', 'EscapedComment', 'AuthorName'
+		);
 	}
 
 	/**
@@ -242,23 +248,25 @@ class CommentingController extends Controller {
 	public function CommentsForm() {
 		$usePreview = Commenting::get_config_value($this->getBaseClass(), 'use_preview');
 		$member = Member::currentUser();
+
 		$fields = new FieldList(
-			TextField::create("Name", _t('CommentInterface.YOURNAME', 'Your name'))
+			$dataFields = new CompositeField(
-				->setCustomValidationMessage(_t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name'))
+				TextField::create("Name", _t('CommentInterface.YOURNAME', 'Your name'))
-				->setAttribute('data-message-required', _t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name')),
+					->setCustomValidationMessage(_t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name'))
+					->setAttribute('data-message-required', _t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name')),
 
-			EmailField::create("Email", _t('CommentingController.EMAILADDRESS', "Your email address (will not be published)"))
+				EmailField::create("Email", _t('CommentingController.EMAILADDRESS', "Your email address (will not be published)"))
-				->setCustomValidationMessage(_t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address'))
+					->setCustomValidationMessage(_t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address'))
-				->setAttribute('data-message-required', _t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address'))
+					->setAttribute('data-message-required', _t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address'))
-				->setAttribute('data-message-email', _t('CommentInterface.EMAILADDRESS_MESSAGE_EMAIL', 'Please enter a valid email address')),
+					->setAttribute('data-message-email', _t('CommentInterface.EMAILADDRESS_MESSAGE_EMAIL', 'Please enter a valid email address')),
 
-			TextField::create("URL", _t('CommentingController.WEBSITEURL', "Your website URL"))
+				TextField::create("URL", _t('CommentingController.WEBSITEURL', "Your website URL"))
-				->setAttribute('data-message-url', _t('CommentInterface.COMMENT_MESSAGE_URL', 'Please enter a valid URL')),
+					->setAttribute('data-message-url', _t('CommentInterface.COMMENT_MESSAGE_URL', 'Please enter a valid URL')),
-
-			TextareaField::create("Comment", _t('CommentingController.COMMENTS', "Comments"))
-				->setCustomValidationMessage(_t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment'))
-				->setAttribute('data-message-required', _t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment')),
 
+				TextareaField::create("Comment", _t('CommentingController.COMMENTS', "Comments"))
+					->setCustomValidationMessage(_t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment'))
+					->setAttribute('data-message-required', _t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment'))
+			),
 			HiddenField::create("ParentID"),
 			HiddenField::create("ReturnURL"),
 			HiddenField::create("BaseClass")
@@ -274,6 +282,7 @@ class CommentingController extends Controller {
 			);
 		}
 	
+		$dataFields->addExtraClass('data-fields');
 
 		// save actions
 		$actions = new FieldList(
@@ -360,7 +369,7 @@ class CommentingController extends Controller {
 	public function doPostComment($data, $form) {
 		$class = (isset($data['BaseClass'])) ? $data['BaseClass'] : $this->getBaseClass();
 		$usePreview = Commenting::get_config_value($class, 'use_preview');
-		$isPreview = ($usePreview && isset($data['preview']) && $data['preview']);
+		$isPreview = ($usePreview && isset($data['IsPreview']) && $data['IsPreview']);
 		
 		// if no class then we cannot work out what controller or model they
 		// are on so throw an error
@@ -398,23 +407,24 @@ class CommentingController extends Controller {
 		$comment = new Comment();
 		$form->saveInto($comment);
 
+		$comment->AllowHtml = Commenting::get_config_value($class, 'html_allowed');
 		$comment->Moderated = ($moderated) ? false : true;
 
 		// Save into DB, or call pre-save hooks to give accurate preview
 		if($isPreview) {
-			$comment->onBeforeWrite();	
+			$comment->extend('onBeforeWrite', $dummy);
 		} else {
 			$comment->write();	
-		}
 
-		// extend hook to allow extensions. Also see onBeforePostComment
+			// extend hook to allow extensions. Also see onBeforePostComment
-		$this->extend('onAfterPostComment', $comment);	
+			$this->extend('onAfterPostComment', $comment);	
+		}
 		
 		// clear the users comment since it passed validation
 		Cookie::set('CommentsForm_Comment', false);
 		
 		if(Director::is_ajax()) {
-			if(!$comment->Moderated) {
+			if(!$comment->Moderated && !$isPreview) {
 				return $comment->renderWith('CommentsInterface_pendingcomment');
 			} else {
 				return $comment->renderWith('CommentsInterface_singlecomment');
@@ -431,6 +441,7 @@ class CommentingController extends Controller {
 
 	public function doPreviewComment($data, $form) {
 		$data['IsPreview'] = 1;
+
 		return $this->doPostComment($data, $form);
 	}
 }

code/dataobjects/Comment.php

@@ -9,13 +9,14 @@ class Comment extends DataObject {
 	
 	public static $db = array(
 		"Name"			=> "Varchar(200)",
-		"Comment"		=> "Text", // can contain sanitized HTML with 'html_allowed=true' config
+		"Comment"		=> "Text",
 		"Email"			=> "Varchar(200)",
 		"URL"			=> "Varchar(255)",
 		"BaseClass"		=> "Varchar(200)",
 		"Moderated"		=> "Boolean",
 		"IsSpam"		=> "Boolean",
-		"ParentID"		=> "Int"
+		"ParentID"		=> "Int",
+		'AllowHtml'		=> "Boolean"
 	);
 
 	public static $has_one = array(
@@ -59,7 +60,7 @@ class Comment extends DataObject {
 		parent::onBeforeWrite();
 
 		// Sanitize HTML, because its expected to be passed to the template unescaped later
-		if($this->getAllowHtml()) {
+		if($this->AllowHtml) {
 			$this->Comment = $this->purifyHtml($this->Comment);
 		}
 	}
@@ -148,7 +149,7 @@ class Comment extends DataObject {
 	 *
 	 * @return string
 	 */
-	public function getParentTitle(){
+	public function getParentTitle() {
 		$parent = $this->getParent();
 
 		return ($parent && $parent->Title) ? $parent->Title : $parent->ClassName . " #" . $parent->ID;
@@ -161,12 +162,42 @@ class Comment extends DataObject {
 	 */
 	public function getParentClassName() {
 		$default = 'SiteTree';
+		
 		if(!$this->BaseClass) {
 			return $default;
 		}
+
 		return $this->BaseClass;
 	}
 	
+	/**
+	 * Return the content for this comment escaped depending on the Html state.
+	 *
+	 * @return HTMLText
+	 */
+	public function getEscapedComment() {
+		$comment = $this->dbObject('Comment');
+
+		if ($comment->exists()) {
+			if ($this->AllowHtml) {
+				return DBField::create_field('HTMLText', nl2br($comment->RAW()));
+			} else {
+				return DBField::create_field('HTMLText', sprintf("<p>%s</p>", nl2br($comment->XML())));
+			}
+		}
+
+		return $comment;
+	}
+
+	/**
+	 * Return whether this comment is a preview (has not been written to the db)
+	 *
+	 * @return boolean
+	 */
+	public function isPreview() {
+		return ($this->ID < 1);
+	}
+
 	/**
 	 * @todo needs to compare to the new {@link Commenting} configuration API
 	 *
@@ -331,20 +362,14 @@ class Comment extends DataObject {
 	public function getCMSFields() {
 		$fields = parent::getCMSFields();
 		$parent = $this->getParent()->ID;
-		$parentIDField = new HiddenField('ParentID', 'Parent', $parent);
-		$authorIDField = new HiddenField('AuthorID');
-		$baseClassField = new HiddenField('BaseClass');
-		$fields->replaceField('ParentID', $parentIDField);
-		$fields->replaceField('AuthorID', $authorIDField);
-		$fields->replaceField('BaseClass', $baseClassField);
-		return $fields;
-	}
 
-	public function getAllowHtml() {
+		$hidden = array('ParentID', 'AuthorID', 'BaseClass', 'AllowHtml');
-		return (
+
-			Commenting::has_commenting($this->BaseClass)
+		foreach($hidden as $private) {
-			&& Commenting::get_config_value($this->BaseClass, 'html_allowed')
+			$fields->removeByName($private);
-		);	
+		}
+
+		return $fields;
 	}
 
 	/**
@@ -361,7 +386,7 @@ class Comment extends DataObject {
 	 */
 	public function getHtmlPurifierService() {
 		$config = HTMLPurifier_Config::createDefault();
-		$config->set('HTML.AllowedElements', 
+		$config->set('HTML.AllowedElements',
 			Commenting::get_config_value($this->BaseClass, 'html_allowed_elements')
 		);
 		$config->set('AutoFormat.AutoParagraph', true);

javascript/CommentsInterface.js

@@ -22,22 +22,20 @@
 		 * Validate
 		 */
 		form.validate({
-		invalidHandler : function(form, validator){
+			invalidHandler : function(form, validator) {
 				$('html, body').animate({
-			scrollTop: $(validator.errorList[0].element).offset().top - 30
+					scrollTop: $(validator.errorList[0].element).offset().top - 30
-			}, 200);
+				}, 200);
 			},
-
 			showErrors: function(errorMap, errorList) {
-			this.defaultShowErrors();
+				this.defaultShowErrors();
-			// hack to add the extra classes we need to the validation message elements
+				// hack to add the extra classes we need to the validation message elements
-			form.find('span.error').addClass('message required');
+				form.find('span.error').addClass('message required');
-		},
+			},
 
 			errorElement: "span",
 			errorClass: "error",
 			ignore: '.hidden',
-
 			rules: {
 				Name : {
 					required : true
@@ -80,6 +78,8 @@
 			if(!form.validate().valid()){
 				return false;
 			}
+			
+			previewEl.removeClass('loading').hide();
 
 			// submit the form
 			$(this).ajaxSubmit(function(response) {
@@ -114,15 +114,18 @@
 		$(':submit[name=action_doPreviewComment]', form).click(function(e) {
 			e.preventDefault();
 
-			if(!form.validate().valid()) return false;
+			if(!form.validate().valid()) {
+				return false;
+			}
 
 			previewEl.show().addClass('loading').find('.middleColumn').html(' ');
+
 			form.ajaxSubmit({
 				success: function(response) {
 					var responseEl = $(response);
 					if(responseEl.is('form')) {
 						// Validation failed, renders form instead of single comment
-						form.replaceWith(responseEl);
+						form.find(".data-fields").replaceWith(responseEl.find(".data-fields"));
 					} else {
 						// Default behaviour
 						previewEl.removeClass('loading').find('.middleColumn').html(responseEl);
@@ -136,7 +139,7 @@
 		 * Hide outdated preview on form changes
 		 */
 		$(':input', form).on('change keydown', function() {
-			previewEl.hide();
+			previewEl.removeClass('loading').hide();
 		});
 		
 		/**

templates/CommentsInterface_singlecomment.ss

@@ -1,32 +1,30 @@
-<div class="comment" id="$Permalink">
+<div class="comment" id="<% if isPreview %>comment-preview<% else %>$Permalink<% end_if %>">
-	<% if AllowHtml %>
+	$EscapedComment
-		$Comment.RAW
-	<% else %>
-		<p>$Comment.XML</p>
-	<% end_if %>
 </div>
 
-<p class="info">
+<% if not isPreview %>
-	<% if $URL %>
+	<p class="info">
-		<% _t('PBY','Posted by') %> <a href="$URL.URL" rel="nofollow">$AuthorName.XML</a>, $Created.Nice ($Created.Ago)
+		<% if $URL %>
-	<% else %>
+			<% _t('PBY','Posted by') %> <a href="$URL.URL" rel="nofollow">$AuthorName.XML</a>, $Created.Nice ($Created.Ago)
-		<% _t('PBY','Posted by') %> $AuthorName.XML, $Created.Nice ($Created.Ago)
+		<% else %>
-	<% end_if %>
+			<% _t('PBY','Posted by') %> $AuthorName.XML, $Created.Nice ($Created.Ago)
-</p>
+		<% end_if %>
+	</p>
 
-<% if $ApproveLink || $SpamLink || $HamLink || $DeleteLink %>
+	<% if $ApproveLink || $SpamLink || $HamLink || $DeleteLink %>
-	<ul class="action-links">
+		<ul class="action-links">
-		<% if ApproveLink %>
+			<% if ApproveLink %>
-			<li><a href="$ApproveLink.ATT" class="approve"><% _t('APPROVE', 'approve this comment') %></a></li>
+				<li><a href="$ApproveLink.ATT" class="approve"><% _t('APPROVE', 'approve this comment') %></a></li>
-		<% end_if %>
+			<% end_if %>
-		<% if SpamLink %>
+			<% if SpamLink %>
-			<li><a href="$SpamLink.ATT" class="spam"><% _t('ISSPAM','this comment is spam') %></a></li>
+				<li><a href="$SpamLink.ATT" class="spam"><% _t('ISSPAM','this comment is spam') %></a></li>
-		<% end_if %>
+			<% end_if %>
-		<% if HamLink %>
+			<% if HamLink %>
-			<li><a href="$HamLink.ATT" class="ham"><% _t('ISNTSPAM','this comment is not spam') %></a></li>
+				<li><a href="$HamLink.ATT" class="ham"><% _t('ISNTSPAM','this comment is not spam') %></a></li>
-		<% end_if %>
+			<% end_if %>
-		<% if DeleteLink %>
+			<% if DeleteLink %>
-			<li class="last"><a href="$DeleteLink.ATT" class="delete"><% _t('REMCOM','remove this comment') %></a></li>
+				<li class="last"><a href="$DeleteLink.ATT" class="delete"><% _t('REMCOM','remove this comment') %></a></li>
-		<% end_if %>
+			<% end_if %>
-	</ul>
+		</ul>
+	<% end_if %>
 <% end_if %>