[ss-2016-015] Fix value / title escaping in CheckboxSetField and Opti…

…onsetField

forms/CheckboxSetField.php

@@ -4,7 +4,7 @@
  *
  * ASSUMPTION -> IF you pass your source as an array, you pass values as an array too. Likewise objects are handled
  * the same.
- * 
+ *
  * Example:
  * <code>
  * new CheckboxSetField(
@@ -19,25 +19,25 @@
  *  $value = "1"
  * );
  * </code>
- * 
+ *
  * <b>Saving</b>
  * The checkbox set field will save its data in one of ways:
  * - If the field name matches a many-many join on the object being edited, that many-many join will be updated to
  *   link to the objects selected on the checkboxes.  In this case, the keys of your value map should be the IDs of
  *   the database records.
  * - If the field name matches a database field, a comma-separated list of values will be saved to that field.  The
  *   keys can be text or numbers.
- * 
+ *
  * @todo Document the different source data that can be used
  * with this form field - e.g ComponentSet, ArrayList,
  * array. Is it also appropriate to accept so many different
  * types of data when just using an array would be appropriate?
- * 
+ *
  * @package forms
  * @subpackage fields-basic
  */
 class CheckboxSetField extends OptionsetField {
-	
+
 	/**
 	 * @var array
 	 */
@@ -82,7 +82,7 @@ public function getOptions() {
 				}
 			}
 		}
-		
+
 		// Source is not an array
 		if(!is_array($source) && !is_a($source, 'SQLMap')) {
 			if(is_array($values)) {
@@ -126,19 +126,22 @@ public function getOptions() {
 		if(is_array($source)) {
 			unset($source['']);
 		}
-		
+
 		$options = array();
-		
+
 		if ($source == null) {
 			$source = array();
 		}
 
 		foreach($source as $value => $item) {
-			if($item instanceof DataObject) {
+			// Ensure $title is cast for template
+			if ($item instanceof DataObject) {
 				$value = $item->ID;
-				$title = $item->Title;
-			} else {
+				$title = $item->obj('Title');
+			} elseif ($item instanceof DBField) {
 				$title = $item;
+			} else {
+				$title = DBField::create_field('Text', $item);
 			}
 
 			$itemID = $this->ID() . '_' . preg_replace('/[^a-zA-Z0-9]/', '', $value);
@@ -168,21 +171,21 @@ public function getOptions() {
 	 * Default selections, regardless of the {@link setValue()} settings.
 	 * Note: Items marked as disabled through {@link setDisabledItems()} can still be
 	 * selected by default through this method.
-	 * 
+	 *
 	 * @param Array $items Collection of array keys, as defined in the $source array
 	 */
 	public function setDefaultItems($items) {
 		$this->defaultItems = $items;
 		return $this;
 	}
-	
+
 	/**
 	 * @return Array
 	 */
 	public function getDefaultItems() {
 		return $this->defaultItems;
 	}
-	
+
 	/**
 	 * Load a value into this CheckboxSetField
 	 */
@@ -198,7 +201,7 @@ public function setValue($value, $obj = null) {
 
 		return $this;
 	}
-	
+
 	/**
 	 * Save the current value of this CheckboxSetField into a DataObject.
 	 * If the field it is saving to is a has_many or many_many relationship,
@@ -228,11 +231,11 @@ public function saveInto(DataObjectInterface $record) {
 			}
 		}
 	}
-	
+
 	/**
-	 * Return the CheckboxSetField value as a string 
+	 * Return the CheckboxSetField value as a string
 	 * selected item keys.
-	 * 
+	 *
 	 * @return string
 	 */
 	public function dataValue() {
@@ -243,30 +246,30 @@ public function dataValue() {
 					$filtered[] = str_replace(",", "{comma}", $item);
 				}
 			}
-			
+
 			return implode(',', $filtered);
 		}
-		
+
 		return '';
 	}
-	
+
 	public function performDisabledTransformation() {
 		$clone = clone $this;
 		$clone->setDisabled(true);
-		
+
 		return $clone;
 	}
-	
+
 	/**
 	 * Transforms the source data for this CheckboxSetField
 	 * into a comma separated list of values.
-	 * 
+	 *
 	 * @return ReadonlyField
 	 */
 	public function performReadonlyTransformation() {
 		$values = '';
 		$data = array();
-		
+
 		$items = $this->value;
 		if($this->source) {
 			foreach($this->source as $source) {
@@ -275,21 +278,21 @@ public function performReadonlyTransformation() {
 				}
 			}
 		}
-		
+
 		if($items) {
 			// Items is a DO Set
 			if($items instanceof SS_List) {
 				foreach($items as $item) {
 					$data[] = $item->Title;
 				}
 				if($data) $values = implode(', ', $data);
-				
+
 			// Items is an array or single piece of string (including comma seperated string)
 			} else {
 				if(!is_array($items)) {
 					$items = preg_split('/ *, */', trim($items));
 				}
-				
+
 				foreach($items as $item) {
 					if(is_array($item)) {
 						$data[] = $item['Title'];
@@ -301,23 +304,23 @@ public function performReadonlyTransformation() {
 						$data[] = $item;
 					}
 				}
-				
+
 				$values = implode(', ', $data);
 			}
 		}
-		
+
 		$field = $this->castedCopy('ReadonlyField');
 		$field->setValue($values);
-		
+
 		return $field;
 	}
 
 	public function Type() {
 		return 'optionset checkboxset';
 	}
-	
+
 	public function ExtraOptions() {
 		return FormField::ExtraOptions();
 	}
-	
+
 }

forms/OptionsetField.php

@@ -1,13 +1,13 @@
 <?php
 /**
  * Set of radio buttons designed to emulate a dropdown.
- * 
- * This field allows you to ensure that a form element is submitted is not optional and is part of a fixed set of 
- * data. This field uses the input type of radio. It's a direct subclass of {@link DropdownField}, 
+ *
+ * This field allows you to ensure that a form element is submitted is not optional and is part of a fixed set of
+ * data. This field uses the input type of radio. It's a direct subclass of {@link DropdownField},
  * so the constructor and arguments are in the same format.
- * 
+ *
  * <b>Usage</b>
- * 
+ *
  * <code>
  * new OptionsetField(
  *    $name = "Foobar",
@@ -22,15 +22,15 @@
  *    $value = "1"
  * );
  * </code>
- * 
- * You can use the helper functions on data object set to create the source array. eg: 
- * 
+ *
+ * You can use the helper functions on data object set to create the source array. eg:
+ *
  * <code>
  * //Database request for the object
  * $map = FooBar::get()->map();
  *  // returns an SS_Map object containing an array of ID => Title
  *
- * // Instantiate the OptionsetField 
+ * // Instantiate the OptionsetField
  * $FieldList = new FieldList(
  *   new OptionsetField(
  *    $name = "Foobar",
@@ -42,16 +42,16 @@
  *
  * // Pass the fields to the form constructor. etc
  * </code>
- * 
+ *
  * @see CheckboxSetField for multiple selections through checkboxes instead.
  * @see DropdownField for a simple <select> field with a single element.
  * @see TreeDropdownField for a rich and customizeable UI that can visualize a tree of selectable elements
- * 
+ *
  * @package forms
  * @subpackage fields-basic
  */
 class OptionsetField extends DropdownField {
-	
+
 	/**
 	 * @var Array
 	 */
@@ -61,14 +61,19 @@ public function Field($properties = array()) {
 		$source = $this->getSource();
 		$odd = 0;
 		$options = array();
-		
+
 		if($source) {
 			foreach($source as $value => $title) {
+				// Ensure $title is safely cast
+				if ( !($title instanceof DBField) ) {
+					$title = DBField::create_field('Text', $title);
+				}
+
 				$itemID = $this->ID() . '_' . preg_replace('/[^a-zA-Z0-9]/', '', $value);
 				$odd = ($odd + 1) % 2;
 				$extraClass = $odd ? 'odd' : 'even';
 				$extraClass .= ' val' . preg_replace('/[^a-zA-Z0-9\-\_]/', '_', $value);
-				
+
 				$options[] = new ArrayData(array(
 					'ID' => $itemID,
 					'Class' => $extraClass,
@@ -95,28 +100,28 @@ public function performReadonlyTransformation() {
 		$field = $this->castedCopy('LookupField');
 		$field->setSource($this->getSource());
 		$field->setReadonly(true);
-		
+
 		return $field;
 	}
-	
+
 	/**
 	 * Mark certain elements as disabled,
 	 * regardless of the {@link setDisabled()} settings.
-	 * 
+	 *
 	 * @param array $items Collection of array keys, as defined in the $source array
 	 */
 	public function setDisabledItems($items) {
 		$this->disabledItems = $items;
 		return $this;
 	}
-	
+
 	/**
 	 * @return Array
 	 */
 	public function getDisabledItems() {
 		return $this->disabledItems;
 	}
-	
+
 	public function ExtraOptions() {
 		return new ArrayList();
 	}

templates/forms/CheckboxSetField.ss

@@ -2,9 +2,9 @@
 	<% if $Options.Count %>
 		<% loop $Options %>
 			<li class="$Class">
-				<input id="$ID" class="checkbox" name="$Name" type="checkbox" value="$Value"<% if $isChecked %> checked="checked"<% end_if %><% if $isDisabled %> disabled="disabled"<% end_if %> />
+				<input id="$ID" class="checkbox" name="$Name" type="checkbox" value="$Value.ATT"<% if $isChecked %> checked="checked"<% end_if %><% if $isDisabled %> disabled="disabled"<% end_if %> />
 				<label for="$ID">$Title</label>
-			</li> 
+			</li>
 		<% end_loop %>
 	<% else %>
 		<li>No options available</li>

templates/forms/OptionsetField.ss

@@ -1,7 +1,7 @@
 <ul id="$ID" class="$extraClass">
 	<% loop $Options %>
 		<li class="$Class">
-			<input id="$ID" class="radio" name="$Name" type="radio" value="$Value"<% if $isChecked %> checked<% end_if %><% if $isDisabled %> disabled<% end_if %> />
+			<input id="$ID" class="radio" name="$Name" type="radio" value="$Value.ATT"<% if $isChecked %> checked<% end_if %><% if $isDisabled %> disabled<% end_if %> />
 			<label for="$ID">$Title</label>
 		</li>
 	<% end_loop %>