Merge branch 'idvalidattr' of git://github.com/wilr/sapphire into wilr-idvalidattr

admin/code/ModelAdmin.php

@@ -1,29 +1,12 @@
 <?php
 /**
- * Generates a three-pane UI for editing model classes,
+ * Generates a three-pane UI for editing model classes, with an
- * with an automatically generated search panel, tabular results
+ * automatically generated search panel, tabular results and edit forms.
- * and edit forms.
+ *
- * Relies on data such as {@link DataObject::$db} and {@DataObject::getCMSFields()}
+ * Relies on data such as {@link DataObject::$db} and {@link DataObject::getCMSFields()}
  * to scaffold interfaces "out of the box", while at the same time providing
  * flexibility to customize the default output.
- * 
- * Add a route
- * <code>
- * Director::config()->rules = array(array('admin/mymodel/$Class/$Action/$ID' => 'MyModelAdmin'));
- * </code>
  *
- * @todo saving logic (should mostly use Form->saveInto() and iterate over relations)
- * @todo ajax form loading and saving
- * @todo ajax result display
- * @todo relation formfield scaffolding (one tab per relation) - relations don't have DBField sublclasses, we do
- * 	we define the scaffold defaults. can be ComplexTableField instances for a start. 
- * @todo has_many/many_many relation autocomplete field (HasManyComplexTableField doesn't work well with larger
- *       datasets)
- * 
- * Long term TODOs:
- * @todo Hook into RESTful interface on DataObjects (yet to be developed)
- * @todo Permission control via datamodel and Form class
- * 
  * @uses SearchContext
  * 
  * @package framework

admin/css/screen.css

@@ -462,18 +462,18 @@ body.cms { overflow: hidden; }
 .cms-add-form ul.SelectionGroup { padding-left: 28px; }
 .cms-add-form .parent-mode { padding: 8px; overflow: auto; }
 
-#PageType ul { padding-left: 20px; }
+#Form_AddForm_PageType_Holder ul { padding-left: 20px; }
-#PageType ul li { float: none; width: 100%; padding: 9px 0 9px 15px; overflow: hidden; border-bottom-width: 2px; border-bottom: 2px groove rgba(255, 255, 255, 0.8); -webkit-border-image: url(../images/textures/bg_fieldset_elements_border.png) 2 stretch stretch; border-image: url(../images/textures/bg_fieldset_elements_border.png) 2 stretch stretch; }
+#Form_AddForm_PageType_Holder ul li { float: none; width: 100%; padding: 9px 0 9px 15px; overflow: hidden; border-bottom-width: 2px; border-bottom: 2px groove rgba(255, 255, 255, 0.8); -webkit-border-image: url(../images/textures/bg_fieldset_elements_border.png) 2 stretch stretch; border-image: url(../images/textures/bg_fieldset_elements_border.png) 2 stretch stretch; }
-#PageType ul li:last-child { border-bottom: none; }
+#Form_AddForm_PageType_Holder ul li:last-child { border-bottom: none; }
-#PageType ul li:hover, #PageType ul li.selected { background-color: rgba(255, 255, 102, 0.3); }
+#Form_AddForm_PageType_Holder ul li:hover, #Form_AddForm_PageType_Holder ul li.selected { background-color: rgba(255, 255, 102, 0.3); }
-#PageType ul li.disabled { color: #aaaaaa; filter: progid:DXImageTransform.Microsoft.Alpha(Opacity=50); opacity: 0.5; }
+#Form_AddForm_PageType_Holder ul li.disabled { color: #aaaaaa; filter: progid:DXImageTransform.Microsoft.Alpha(Opacity=50); opacity: 0.5; }
-#PageType ul li.disabled:hover { background: none; }
+#Form_AddForm_PageType_Holder ul li.disabled:hover { background: none; }
-#PageType ul li input { margin: inherit; }
+#Form_AddForm_PageType_Holder ul li input { margin: inherit; }
-#PageType ul li label { padding-left: 0; padding-bottom: 0; }
+#Form_AddForm_PageType_Holder ul li label { padding-left: 0; padding-bottom: 0; }
-#PageType ul li input, #PageType ul li label, #PageType ul li .page-icon, #PageType ul li .title { float: left; line-height: 1.3em; }
+#Form_AddForm_PageType_Holder ul li input, #Form_AddForm_PageType_Holder ul li label, #Form_AddForm_PageType_Holder ul li .page-icon, #Form_AddForm_PageType_Holder ul li .title { float: left; line-height: 1.3em; }
-#PageType ul li .page-icon { margin: 0 4px; }
+#Form_AddForm_PageType_Holder ul li .page-icon { margin: 0 4px; }
-#PageType ul li .title { width: 120px; font-weight: bold; padding-right: 10px; }
+#Form_AddForm_PageType_Holder ul li .title { width: 120px; font-weight: bold; padding-right: 10px; }
-#PageType ul li .description { font-style: italic; display: inline; clear: none; margin: 0; }
+#Form_AddForm_PageType_Holder ul li .description { font-style: italic; display: inline; clear: none; margin: 0; }
 
 /** -------------------------------------------- Content toolbar -------------------------------------------- */
 .cms-content-toolbar { min-height: 29px; display: block; margin: 0 0 8px 0; border-bottom: 1px solid #d0d3d5; -webkit-box-shadow: 0 1px 0 rgba(248, 248, 248, 0.9); -moz-box-shadow: 0 1px 0 rgba(248, 248, 248, 0.9); -o-box-shadow: 0 1px 0 rgba(248, 248, 248, 0.9); box-shadow: 0 1px 0 rgba(248, 248, 248, 0.9); *zoom: 1; /* smaller treedropdown */ }
@@ -1017,8 +1017,8 @@ visible. Added and removed with js in TabSet.js  */ /***************************
 .cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel.first { left: 0; width: 203px; }
 .cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel .ui-icon { padding-right: 0; }
 .cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel .tab-nav-link, .cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel .ss-ui-button { font-size: 12px; }
-.cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel #PageType ul { padding: 0; }
+.cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel #Form_AddForm_PageType_Holder ul { padding: 0; }
-.cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel #PageType ul li { padding: 4px 5px; }
+.cms .ss-ui-action-tabset.multi .ss-ui-action-tab.ui-tabs-panel #Form_AddForm_PageType_Holder ul li { padding: 4px 5px; }
 .cms .ss-ui-action-tabset.tabset-open ul.ui-tabs-nav, .cms .ss-ui-action-tabset.tabset-open ul.ui-tabs-nav li.first { -moz-border-radius-bottomleft: 0; -webkit-border-bottom-left-radius: 0; border-bottom-left-radius: 0; }
 .cms .ss-ui-action-tabset.tabset-open-last ul.ui-tabs-nav li.last { -moz-border-radius-bottomright: 0; -webkit-border-bottom-right-radius: 0; border-bottom-right-radius: 0; }
 .cms .ss-ui-action-tabset .batch-check, .cms .ss-ui-action-tabset .ui-icon { display: inline-block; float: left; margin-left: -2px; padding-right: 6px; }

admin/javascript/SecurityAdmin.js

@@ -37,7 +37,7 @@
 		 * As they're disabled, any changes won't be submitted (which is intended behaviour),
 		 * checking all boxes is purely presentational.
 		 */
-		$('#Permissions .checkbox[value=ADMIN]').entwine({
+		$('.permissioncheckboxset .checkbox[value=ADMIN]').entwine({
 			onmatch: function() {
 				this.toggleCheckboxes();
 
@@ -56,7 +56,8 @@
 			 * Function: toggleCheckboxes
 			 */
 			toggleCheckboxes: function() {
-				var self = this, checkboxes = this.parents('.field:eq(0)').find('.checkbox').not(this);
+				var self = this,
+					checkboxes = this.parents('.field:eq(0)').find('.checkbox').not(this);
 				
 				if(this.is(':checked')) {
 					checkboxes.each(function() {

admin/scss/_actionTabs.scss

@@ -124,8 +124,10 @@ $border: 1px solid darken(#D9D9D9, 15%);
 				.tab-nav-link, .ss-ui-button {
 					font-size: 12px;
 				}	
-				#PageType ul{
+
-					padding:0;
+				#Form_AddForm_PageType_Holder ul {
+					padding: 0;
+
 					li{
 						padding:4px 5px;
 					}

admin/scss/_style.scss

@@ -531,7 +531,7 @@ body.cms {
 	}	
 }
 
-#PageType {
+#Form_AddForm_PageType_Holder {
 	ul {
 		padding-left: 20px;
 		li {

core/Convert.php

@@ -43,22 +43,49 @@ class Convert {
 	}
 
 	/**
-	 * Convert a value to be suitable for an HTML attribute.
+	 * Convert a value to be suitable for an HTML ID attribute. Replaces non
-	 * 
+	 * supported characters with a space.
-	 * This is useful for converting human readable values into
-	 * a value suitable for an ID or NAME attribute.
 	 * 
 	 * @see http://www.w3.org/TR/REC-html40/types.html#type-cdata
-	 * @uses Convert::raw2att()
+	 *
 	 * @param array|string $val String to escape, or array of strings
+	 *
 	 * @return array|string
 	 */
 	public static function raw2htmlname($val) {
 		if(is_array($val)) {
-			foreach($val as $k => $v) $val[$k] = self::raw2htmlname($v);
+			foreach($val as $k => $v) {
+				$val[$k] = self::raw2htmlname($v);
+			}
+
 			return $val;
 		} else {
-			return preg_replace('/[^a-zA-Z0-9\-_:.]+/','', $val);
+			return self::raw2att($val);
+		}
+	}
+
+	/**
+	 * Convert a value to be suitable for an HTML ID attribute. Replaces non
+	 * supported characters with an underscore.
+	 *
+	 * @see http://www.w3.org/TR/REC-html40/types.html#type-cdata
+	 *
+	 * @param array|string $val String to escape, or array of strings
+	 *
+	 * @return array|string
+	 */
+	public static function raw2htmlid($val) {
+		if(is_array($val)) {
+			foreach($val as $k => $v) {
+				$val[$k] = self::raw2htmlid($v);
+			}
+
+			return $val;
+		} else {
+			return trim(preg_replace(
+				'/_+/', '_', preg_replace('/[^a-zA-Z0-9\-_:.]+/','_', $val)),
+				'_'
+			);
 		}
 	}
 	

css/AssetUploadField.css

@@ -17,8 +17,6 @@ Used in side panels and action tabs
 
 .backlink { padding-left: 12px; }
 
-#Form_EditorToolbarMediaForm .ui-tabs-panel { padding-left: 0px; }
-
 body.cms.ss-uploadfield-edit-iframe, .composite.ss-assetuploadfield .details fieldset { padding: 16px; overflow: auto; background: #E2E2E2; }
 body.cms.ss-uploadfield-edit-iframe span.readonly, .composite.ss-assetuploadfield .details fieldset span.readonly { font-style: italic; color: #777777; text-shadow: 0px 1px 0px #fff; }
 body.cms.ss-uploadfield-edit-iframe .fieldholder-small label, .composite.ss-assetuploadfield .details fieldset .fieldholder-small label { margin-left: 0; }

css/ComplexTableField_popup.css

@@ -1,35 +0,0 @@
-html { overflow-y: auto !important; }
-
-body { height: 100%; }
-
-#ComplexTableField_Popup_DetailForm input.loading { background: white url(../images/network-save.gif) left center no-repeat; padding-left: 16px; }
-
-.PageControls { padding: 5px; width: 100%; }
-.PageControls * { vertical-align: middle; }
-.PageControls .Left { width: 33%; }
-.PageControls .Count { width: 33%; text-align: center; }
-.PageControls .Right { width: 33%; text-align: right; }
-
-.ComplexTableField_Popup td.hidden { display: none; }
-.ComplexTableField_Popup th.HiddenField { display: none; }
-.ComplexTableField_Popup span.right { float: right; clear: none; }
-.ComplexTableField_Popup span.left { float: left; clear: none; }
-.ComplexTableField_Popup form p.checkbox input { margin: 0pt 1px; }
-.ComplexTableField_Popup form ul.optionset { margin: 0; padding: 0; }
-.ComplexTableField_Popup form ul.optionset li { margin: 4px 0; }
-.ComplexTableField_Popup form div.Actions input { font-size: 11px; margin-top: 10px; }
-
-/* Pagination */
-#ComplexTableField_Pagination, #ComplexTableField_Pagination * { vertical-align: middle; }
-
-#ComplexTableField_Pagination { margin-top: 10px; margin-left: auto; margin-right: auto; font-size: 11px; }
-#ComplexTableField_Pagination a { /*font-size: 1.2em;*/ font-size: 13px; font-weight: bold; text-decoration: none; width: 1px; height: 1px; margin: 1px; }
-#ComplexTableField_Pagination a:hover { background: none; }
-#ComplexTableField_Pagination span { display: inline; font-weight: bold; font-size: 15px; color: #f00; }
-#ComplexTableField_Pagination div { display: inline; }
-
-#ComplexTableField_Pagination_Previous { padding-right: 10px; }
-
-#ComplexTableField_Pagination_Next { padding-left: 10px; }
-
-#ComplexTableField_Pagination_Next img, #ComplexTableField_Pagination_Previous img { margin: 0 3px 2px; }

docs/en/changelogs/rc/3.2.0.md

@@ -14,6 +14,7 @@ Otherwise, you'll need to include the module yourself
  * API: Removed URL routing by controller name
  * Security: The multiple authenticator login page should now be styled manually - i.e. without the default jQuery UI layout. A new template, Security_MultiAuthenticatorLogin.ss is available.
  * Security: This controller's templates can be customised by overriding the `getTemplate` function.
+ * API: Form and FormField ID attributes rewritten.
 
 ## Details
 
@@ -63,3 +64,78 @@ you can reinstate the old behaviour through a director rule:
 	Director:
   		rules:
     		'$Controller//$Action/$ID/$OtherID':  '*'
+
+### API: Default Form and FormField ID attributes rewritten.
+
+Previously the automatic generation of ID attributes throughout the Form API
+could generate invalid ID values such as Password[ConfirmedPassword] as well
+as duplicate ID values between forms on the same page. For example, if you
+created a field called `Email` on more than one form on the page, the resulting
+HTML would have multiple instances of `#Email`. ID should be a unique
+identifier for a single element within the document.
+
+This rewrite has several angles, each of which is described below. If you rely
+on ID values in your CSS files, Javascript code or application unit tests *you
+will need to update your code*.
+
+#### Conversion of invalid form ID values
+
+ID attributes on Form and Form Fields will now follow the
+[HTML specification](http://www.w3.org/TR/REC-html40/types.html#type-cdata).
+Generating ID attributes is now handled by the new `FormTemplateHelper` class.
+
+Please test each of your existing site forms to ensure that they work
+correctly in particular, javascript and css styles which rely on specific ID
+values.
+
+#### Invalid ID attributes stripped
+
+ID attributes will now be run through `Convert::raw2htmlid`. Invalid characters
+are replaced with a single underscore character. Duplicate, leading and trailing
+underscores are removed. Custom ID attributes (set through `setHTMLID`) will not
+be altered.
+
+	Before:
+	<form id="MyForm[Form]"
+		<div id="MyForm[Form][ID]">
+
+	Now:
+	<form id="MyForm_Form">
+		<div id="MyForm_Form_ID">
+
+#### Namespaced FormField ID's
+
+Form Field ID values will now be namespaced with the parent form ID.
+
+	Before:
+	<div id="Email">
+
+	Now:
+	<div id="MyForm_Email">
+
+#### FormField wrapper containers suffixed with `_Holder`
+
+Previously both the container div and FormField tag shared the same ID in
+certain cases. Now, the wrapper div in the default `FormField` template will be
+suffixed with `_Holder`.
+
+	Before:
+	<div id="Email">
+		<input id="Email" />
+
+	After:
+	<div id="MyForm_Email_Holder"
+		<input id="MyForm_Email" />
+
+#### Reverting to the old specification
+
+If upgrading existing forms is not feasible, developers can opt out of the new
+specifications by using the `FormTemplateHelper_Pre32` class rules instead of
+the default ones.
+
+	:::yaml
+	# mysite/config/_config.yml
+
+	Injector:
+		FormTemplateHelper:
+			class: FormTemplateHelper_Pre32

docs/en/reference/complextablefield.md

@@ -1,135 +0,0 @@
-# Complex Table Field
-
-## Introduction
-
-<div class="warning" markdown="1">
-	This field is deprecated in favour of the new [GridField](/reference/grid-field) API.
-</div>
-
-Shows a group of DataObjects as a (readonly) tabular list (similiar to `[api:TableListField]`.)
-
-You can specify limits and filters for the resultset by customizing query-settings (mostly the ID-field on the other
-side of a one-to-many-relationship).
-
-See `[api:TableListField]` for more documentation on the base-class
-
-## Source Input
-
-See `[api:TableListField]`.
-
-## Setting Parent/Child-Relations
-
-`[api:ComplexTableField]` tries to determine the parent-relation automatically by looking at the $has_one property on the listed
-child, or the record loaded into the surrounding form (see getParentClass() and getParentIdName()). You can force a
-specific parent relation:
-
-	:::php
-	$myCTF->setParentClass('ProductGroup');
-
-
-## Customizing Popup
-
-By default, getCMSFields() is called on the listed DataObject.
-You can override this behaviour in various ways:
-
-	:::php
-	// option 1: implicit (left out of the constructor), chooses based on Object::useCustomClass or specific instance
-	$myCTF = new ComplexTableField(
-	  $this,
-	  'MyName',
-	  'Product',
-	  array('Price','Code')
-	)
-	
-	// option 2: constructor
-	$myCTF = new ComplexTableField(
-	  $this,
-	  'MyName',
-	  'Product',
-	  array('Price','Code'),
-	  new FieldList(
-	    new TextField('Price')
-	  )
-	)
-	
-	// option 3: constructor function
-	$myCTF = new ComplexTableField(
-	  $this,
-	  'MyName',
-	  'Product',
-	  array('Price','Code'),
-	  'getCustomCMSFields'
-	)
-
-
-## Customizing Display & Functionality
-
-If you don't want several functions to appear (e.g. no add-link), there's several ways:
-
-*  Use `ComplexTableField->setPermissions(array("show","edit"))` to limit the functionality without touching the template
-(more secure). Possible values are "show","edit", "delete" and "add".  
-
-*  Subclass `[api:ComplexTableField]` and override the rendering-mechanism
-*  Use `ComplexTableField->setTemplate()` and `ComplexTableField->setTemplatePopup()` to provide custom templates
-
-### Customising fields and Requirements in the popup
-
-There are several ways to customise the fields in the popup. Often you would want to display more information in the
-popup as there is more real-estate for you to play with. 
-
-`[api:ComplexTableField]` gives you several options to do this. You can either
-
-*  Pass a FieldList in the constructor.
-*  Pass a String in the constructor. 
-
-The first will simply add the fieldlist to the form, and populate it with the source class. 
-The second will call the String as a method on the source class (Which should return a FieldList) of fields for the
-Popup. 
-
-You can also customise Javascript which is loaded for the Lightbox. As Requirements::clear() is called when the popup is
-instantiated, `[api:ComplexTableField]` will look for a function to gather any specific requirements that you might need on your
-source class. (e.g. Inline Javascript or styling).
-
-For this, create a function called "getRequirementsForPopup". 
-
-## Getting it working on the front end (not the CMS)
-
-Sometimes you'll want to have a nice table on the front end, so you can move away from relying on the CMS for maintaing
-parts of your site.
-
-You'll have to do something like this in your form:
-
-	:::php
-	$tableField = new ComplexTableField(
-	   $controller,
-	   'Works',
-	   'Work',
-	   array(
-	      'MyField' => 'My awesome field name'
-	   ),
-	   'getPopupFields'
-	);
-	
-	$tableField->setParentClass(false);
-			
-	$fields = new FieldList(
-	   new HiddenField('ID', ''),
-	   $tableField
-	);
-
-
-You have to hack in an ID on the form, as the CMS forms have this, and front end forms usually do not.
-
-It's not a perfect solution, but it works relatively well to get a simple `[api:ComplexTableField]` up and running on the front
-end.
-
-To come: Make it a lot more flexible so tables can be easily used on the front end. It also needs to be flexible enough
-to use a popup as well, out of the box.
-
-## Subclassing
-
-Most of the time, you need to override the following methods:
-
-*  ComplexTableField->sourceItems() - querying
-*  ComplexTableField->DetailForm() - form output
-*  ComplexTableField_Popup->saveComplexTableField() - saving

docs/en/reference/index.md

@@ -3,8 +3,7 @@
 Reference articles complement our auto-generated [API docs](http://api.silverstripe.org) in providing deeper introduction into a specific API. 
 
 * [BBCode](bbcode): Extensible shortcode syntax
-* [CMS Architecture](cms-architecture): A quick run down to get you started with creating your own data management interface
+* [CMS Architecture](cms-architecture): A quick run down to get you started with creating your own data management interface.
-* [ComplexTableField](complextablefield): Manage records and their relations inside the CMS
 * [GridField](grid-field): The GridField is a flexible form field for creating tables of data.
 * [Database Structure](database-structure): Conventions and best practices for database tables and fields
 * [DataExtension](dataextension): A "mixin" system allowing to extend core classes

docs/en/reference/tablefield.md

@@ -63,12 +63,6 @@ Due to the nested nature of this fields dataset, you can't set any required colu
 Note: You still have to attach some form of `[api:Validator]` to the form to trigger any validation on this field.
 
 
-### Nested Table Fields
-
-When you have `[api:TableField]` inside a `[api:ComplexTableField]`, the parent ID may not be known in your
-getCMSFields() method.  In these cases, you can set a value to '$RecordID' in your `[api:TableField]` extra data, and this
-will be populated with the newly created record id upon save.
-
 ## Known Issues
 
 *  A `[api:TableField]` doesn't reload any submitted form-data if the saving is interrupted by a failed validation. After

docs/en/topics/javascript.md

@@ -289,14 +289,12 @@ request](http://docs.jquery.com/Frequently_Asked_Questions#Why_do_my_events_stop
 ### Assume Element Collections
 
 jQuery is based around collections of DOM elements, the library functions typically handle multiple elements (where it
-makes sense). Encapsulate your code by nesting your jQuery commands inside a `jQuery().each()` call.
+makes sense). Encapsulate your code by nesting your jQuery commands inside a `jQuery().each()` call. Example:
-
-Example: ComplexTableField implements a paginated table with a pop-up for displaying 
 
 	:::js
-	$('div.ComplexTableField').each(function() {
+	$('.MyCustomField').each(function() {
-	  // This is the over code for the tr elements inside a ComplexTableField.
+	  // This is the over code for the elements inside a MyCustomField.
-	  $(this).find('tr').hover(
+	  $(this).hover(
 	    // ...
 	  );
 	});

forms/Form.php

@@ -149,6 +149,21 @@ class Form extends RequestHandler {
 	 */
 	protected $attributes = array();
 
+	/**
+	 * @var FormTemplateHelper
+	 */
+	private $templateHelper = null;
+
+	/**
+	 * @ignore
+	 */
+	private $htmlID = null;
+
+	/**
+	 * @ignore
+	 */
+	private $formActionPath = false;
+
 	/**
 	 * Create a new form, with the given fields an action buttons.
 	 * 
@@ -639,13 +654,14 @@ class Form extends RequestHandler {
 
 	public function getAttributes() {
 		$attrs = array(
-			'id' => $this->FormName(),
+			'id' => $this->getTemplateHelper()->generateFormID($this),
 			'action' => $this->FormAction(),
 			'method' => $this->FormMethod(),
 			'enctype' => $this->getEncType(),
 			'target' => $this->target,
 			'class' => $this->extraClass(),
 		);
+
 		if($this->validator && $this->validator->getErrors()) {
 			if(!isset($attrs['class'])) $attrs['class'] = '';
 			$attrs['class'] .= ' validationerror';
@@ -668,6 +684,7 @@ class Form extends RequestHandler {
 
 		if(!$attrs || is_string($attrs)) $attrs = $this->getAttributes();
 
+
 		// Figure out if we can cache this form
 		// - forms with validation shouldn't be cached, cos their error messages won't be shown
 		// - forms with security tokens shouldn't be cached because security tokens expire
@@ -708,13 +725,43 @@ class Form extends RequestHandler {
 	}
 
 	/**
-	* Set the target of this form to any value - useful for opening the form contents in a new window or refreshing
+	 * Set the {@link FormTemplateHelper}
-	* another frame
+	 *
-	* 
+	 * @param string|FormTemplateHelper
-	* @param target The value of the target
+	 */
-	*/
+	public function setTemplateHelper($helper) {
+		$this->templateHelper = $helper;
+	}
+
+	/**
+	 * Return a {@link FormTemplateHelper} for this form. If one has not been
+	 * set, return the default helper.
+	 *
+	 * @return FormTemplateHelper
+	 */
+	public function getTemplateHelper() {
+		if($this->templateHelper) {
+			if(is_string($this->templateHelper)) {
+				return Injector::inst()->get($this->templateHelper);
+			}
+
+			return $this->templateHelper;
+		}
+
+		return Injector::inst()->get('FormTemplateHelper');
+	}
+
+	/**
+	 * Set the target of this form to any value - useful for opening the form
+	 * contents in a new window or refreshing another frame.
+	 *
+	 * @param target $target The value of the target
+	 *
+	 * @return FormField
+	 */
 	public function setTarget($target) {
 		$this->target = $target;
+
 		return $this;
 	}
 	
@@ -864,50 +911,67 @@ class Form extends RequestHandler {
 		}
 	}
 	
-	/** @ignore */
-	private $formActionPath = false;
-	
 	/**
 	 * Set the form action attribute to a custom URL.
 	 * 
-	 * Note: For "normal" forms, you shouldn't need to use this method.  It is recommended only for situations where
+	 * Note: For "normal" forms, you shouldn't need to use this method.  It is
-	 * you have two relatively distinct parts of the system trying to communicate via a form post.
+	 * recommended only for situations where you have two relatively distinct
+	 * parts of the system trying to communicate via a form post.
+	 *
+	 * @param string
+	 *
+	 * @return Form
 	 */
 	public function setFormAction($path) {
 		$this->formActionPath = $path;
+
 		return $this;
 	}
 
 	/**
-	 * @ignore
+	 * Returns the name of the form.
-	 */
+	 *
-	private $htmlID = null;
+	 * @return string
-
-	/**
-	 * Returns the name of the form
 	 */
 	public function FormName() {
-		if($this->htmlID) return $this->htmlID;
+		return $this->getTemplateHelper()->generateFormID($this);
-		else return $this->class . '_' . str_replace(array('.', '/'), '', $this->name);
 	}
 
 	/**
-	 * Set the HTML ID attribute of the form
+	 * Set the HTML ID attribute of the form.
+	 *
+	 * @param string $id
+	 *
+	 * @return FormField
 	 */
 	public function setHTMLID($id) {
 		$this->htmlID = $id;
+
+		return $this;
+	}
+
+	/**
+	 * @return string
+	 */
+	public function getHTMLID() {
+		return $this->htmlID;
 	}
 	
 	/**
 	 * Returns this form's controller.
-	 * This is used in the templates.
+	 *
+	 * @return Controller
+	 * @deprecated 4.0
 	 */
 	public function Controller() {
+		Deprecation::notice('4.0', 'Use getController() rather than Controller() to access controller');
+
 		return $this->getController();
 	}
 
 	/**
 	 * Get the controller.
+	 *
 	 * @return Controller
 	 */
 	public function getController() {
@@ -916,16 +980,19 @@ class Form extends RequestHandler {
 
 	/**
 	 * Set the controller.
+	 *
 	 * @param Controller $controller
 	 * @return Form
 	 */
 	public function setController($controller) {
 		$this->controller = $controller;
+
 		return $this;
 	}
 
 	/**
 	 * Get the name of the form.
+	 *
 	 * @return string
 	 */
 	public function getName() {
@@ -934,32 +1001,37 @@ class Form extends RequestHandler {
 
 	/**
 	 * Set the name of the form.
+	 *
 	 * @param string $name
 	 * @return Form
 	 */
 	public function setName($name) {
 		$this->name = $name;
+
 		return $this;
 	}
 
 	/**
-	 * Returns an object where there is a method with the same name as each data field on the form.
+	 * Returns an object where there is a method with the same name as each data 
+	 * field on the form.
+	 *
 	 * That method will return the field itself.
-	 * It means that you can execute $firstNameField = $form->FieldMap()->FirstName(), which can be handy
+	 *
+	 * It means that you can execute $firstName = $form->FieldMap()->FirstName()
 	 */
 	public function FieldMap() {
 		return new Form_FieldMap($this);
 	}
 
 	/**
-	 * The next functions store and modify the forms
+	 * The next functions store and modify the forms message attributes. 
-	 * message attributes. messages are stored in session under
+	 * messages are stored in session under $_SESSION[formname][message];
-	 * $_SESSION[formname][message];
 	 * 
 	 * @return string
 	 */
 	public function Message() {
 		$this->getMessageFromSession();
+
 		return $this->message;
 	}
 	
@@ -968,16 +1040,17 @@ class Form extends RequestHandler {
 	 */
 	public function MessageType() {
 		$this->getMessageFromSession();
+
 		return $this->messageType;
 	}
 
 	protected function getMessageFromSession() {
 		if($this->message || $this->messageType) {
 			return $this->message;
-		}else{
-			$this->message = Session::get("FormInfo.{$this->FormName()}.formError.message");
-			$this->messageType = Session::get("FormInfo.{$this->FormName()}.formError.type");
 		}
+		
+		$this->message = Session::get("FormInfo.{$this->FormName()}.formError.message");
+		$this->messageType = Session::get("FormInfo.{$this->FormName()}.formError.type");
 	}
 
 	/**
@@ -1014,6 +1087,7 @@ class Form extends RequestHandler {
 		Session::clear("FormInfo.{$this->FormName()}.formError");
 		Session::clear("FormInfo.{$this->FormName()}.data");
 	}
+
 	public function resetValidation() {
 		Session::clear("FormInfo.{$this->FormName()}.errors");
 		Session::clear("FormInfo.{$this->FormName()}.data");
@@ -1041,26 +1115,32 @@ class Form extends RequestHandler {
 
 	/**
 	 * Processing that occurs before a form is executed.
+	 *
 	 * This includes form validation, if it fails, we redirect back
 	 * to the form with appropriate error messages.
+	 *
 	 * Triggered through {@link httpSubmission()}.
+	 *
 	 * Note that CSRF protection takes place in {@link httpSubmission()},
 	 * if it fails the form data will never reach this method.
 	 * 
 	 * @return boolean
 	 */
-	public function validate(){
+	public function validate() {
-		if($this->validator){
+		if($this->validator) {
 			$errors = $this->validator->validate();
 
-			if($errors){
+			if($errors) {
 				// Load errors into session and post back
 				$data = $this->getData();
+
 				Session::set("FormInfo.{$this->FormName()}.errors", $errors); 
 				Session::set("FormInfo.{$this->FormName()}.data", $data);
+				
 				return false;
 			}
 		}
+
 		return true;
 	}
 
@@ -1070,6 +1150,7 @@ class Form extends RequestHandler {
 
 	/**
 	 * Load data from the given DataObject or array.
+	 *
 	 * It will call $object->MyField to get the value of MyField.
 	 * If you passed an array, it will call $object[MyField].
 	 * Doesn't save into dataless FormFields ({@link DatalessField}),
@@ -1079,15 +1160,19 @@ class Form extends RequestHandler {
 	 * its value will not be saved to the field, retaining
 	 * potential existing values.
 	 * 
-	 * Passed data should not be escaped, and is saved to the FormField instances unescaped.
+	 * Passed data should not be escaped, and is saved to the FormField 
-	 * Escaping happens automatically on saving the data through {@link saveInto()}.
+	 * instances unescaped.
+	 *
+	 * Escaping happens automatically on saving the data through 
+	 * {@link saveInto()}.
 	 * 
 	 * @uses FieldList->dataFields()
 	 * @uses FormField->setValue()
 	 * 
 	 * @param array|DataObject $data
 	 * @param int $mergeStrategy
-	 *  For every field, {@link $data} is interogated whether it contains a relevant property/key, and
+	 *  For every field, {@link $data} is interogated whether it contains a 
+	 * relevant property/key, and
 	 *  what that property/key's value is.
 	 *
 	 *  By default, if {@link $data} does contain a property/key, the fields value is always replaced by {@link $data}'s
@@ -1218,11 +1303,11 @@ class Form extends RequestHandler {
 	
 	/**
 	 * Get the submitted data from this form through
-	 * {@link FieldList->dataFields()}, which filters out
+	 * {@link FieldList->dataFields()}, which filters out any form-specific data 
-	 * any form-specific data like form-actions.
+	 * like form-actions.
-	 * Calls {@link FormField->dataValue()} on each field,
+	 * 
-	 * which returns a value suitable for insertion into a DataObject
+	 * Calls {@link FormField->dataValue()} on each field, which returns a value 
-	 * property.
+	 * suitable for insertion into a DataObject property.
 	 * 
 	 * @return array
 	 */
@@ -1230,20 +1315,23 @@ class Form extends RequestHandler {
 		$dataFields = $this->fields->dataFields();
 		$data = array();
 		
-		if($dataFields){
+		if($dataFields) {
 			foreach($dataFields as $field) {
 				if($field->getName()) {
 					$data[$field->getName()] = $field->dataValue();
 				}
 			}
 		}
+
 		return $data;
 	}
 
 	/**
 	 * Call the given method on the given field.
-	 * This is used by Ajax-savvy form fields.  By putting '&action=callfieldmethod' to the end
+	 *
-	 * of the form action, they can access server-side data.
+	 * This is used by Ajax-savvy form fields.  By putting '&action=callfieldmethod' 
+	 * to the end of the form action, they can access server-side data.
+	 *
 	 * @param fieldName The name of the field.  Can be overridden by $_REQUEST[fieldName]
 	 * @param methodName The name of the field.  Can be overridden by $_REQUEST[methodName]
 	 */
@@ -1276,6 +1364,8 @@ class Form extends RequestHandler {
 	 * 
 	 * This is returned when you access a form as $FormObject rather
 	 * than <% with FormObject %>
+	 *
+	 * @return HTML
 	 */
 	public function forTemplate() {
 		$return = $this->renderWith(array_merge(
@@ -1291,7 +1381,11 @@ class Form extends RequestHandler {
 
 	/**
 	 * Return a rendered version of this form, suitable for ajax post-back.
-	 * It triggers slightly different behaviour, such as disabling the rewriting of # links
+	 *
+	 * It triggers slightly different behaviour, such as disabling the rewriting 
+	 * of # links.
+	 *
+	 * @return HTML
 	 */
 	public function forAjaxTemplate() {
 		$view = new SSViewer(array(
@@ -1304,8 +1398,12 @@ class Form extends RequestHandler {
 
 	/**
 	 * Returns an HTML rendition of this form, without the <form> tag itself.
-	 * Attaches 3 extra hidden files, _form_action, _form_name, _form_method, and _form_enctype.  These are
+	 *
-	 * the attributes of the form.  These fields can be used to send the form to Ajax.
+	 * Attaches 3 extra hidden files, _form_action, _form_name, _form_method, 
+	 * and _form_enctype.  These are the attributes of the form.  These fields 
+	 * can be used to send the form to Ajax.
+	 *
+	 * @return HTML
 	 */
 	public function formHtmlContent() {
 		$this->IncludeFormTag = false;
@@ -1322,77 +1420,115 @@ class Form extends RequestHandler {
 	}
 
 	/**
-	 * Render this form using the given template, and return the result as a string
+	 * Render this form using the given template, and return the result as a 
-	 * You can pass either an SSViewer or a template name
+	 * string.
+	 *
+	 * You can pass either an SSViewer or a template name.
+	 *
+	 * @param SSViewer|string $template
+	 *
+	 * @return HTML
 	 */
 	public function renderWithoutActionButton($template) {
 		$custom = $this->customise(array(
 			"Actions" => "",
 		));
 
-		if(is_string($template)) $template = new SSViewer($template);
+		if(is_string($template)) {
+			$template = new SSViewer($template);
+		}
+
 		return $template->process($custom);
 	}
 
 
 	/**
-	 * Sets the button that was clicked.  This should only be called by the Controller.
+	 * Sets the button that was clicked.  This should only be called by the 
-	 * @param funcName The name of the action method that will be called.
+	 * {@link Controller}
+	 *
+	 * @param string $funcName The name of the action method that will be called
+	 *
+	 * @return Form
 	 */
 	public function setButtonClicked($funcName) {
 		$this->buttonClickedFunc = $funcName;
+
 		return $this;
 	}
 
+	/**
+	 * @return FormAction
+	 */
 	public function buttonClicked() {
 		foreach($this->actions as $action) {
-			if($this->buttonClickedFunc == $action->actionName()) return $action;
+			if($this->buttonClickedFunc == $action->actionName()) {
+				return $action;
+			}
 		}
 	}
 
 	/**
-	 * Return the default button that should be clicked when another one isn't available
+	 * Return the default button that should be clicked when another one isn't 
+	 * available.
+	 *
+	 * @return FormAction
 	 */
 	public function defaultAction() {
-		if($this->hasDefaultAction && $this->actions)
+		if($this->hasDefaultAction && $this->actions) {
 			return $this->actions->First();
+		}
 	}
 
 	/**
 	 * Disable the default button.
-	 * Ordinarily, when a form is processed and no action_XXX button is available, then the first button in the
+	 *
-	 * actions list will be pressed.  However, if this is "delete", for example, this isn't such a good idea.
+	 * Ordinarily, when a form is processed and no action_XXX button is 
+	 * available, then the first button in the actions list will be pressed. 
+	 * However, if this is "delete", for example, this isn't such a good idea.
+	 *
+	 * @return Form
 	 */
 	public function disableDefaultAction() {
 		$this->hasDefaultAction = false;
+
 		return $this;
 	}
 	
 	/**
-	 * Disable the requirement of a security token on this form instance. This security protects
+	 * Disable the requirement of a security token on this form instance. This 
-	 * against CSRF attacks, but you should disable this if you don't want to tie 
+	 * security protects against CSRF attacks, but you should disable this if 
-	 * a form to a session - eg a search form.
+	 * you don't want to tie a form to a session - eg a search form.
 	 * 
-	 * Check for token state with {@link getSecurityToken()} and {@link SecurityToken->isEnabled()}.
+	 * Check for token state with {@link getSecurityToken()} and 
+	 * {@link SecurityToken->isEnabled()}.
+	 *
+	 * @return Form
 	 */
 	public function disableSecurityToken() {
 		$this->securityToken = new NullSecurityToken();
+
 		return $this;
 	}
 	
 	/**
 	 * Enable {@link SecurityToken} protection for this form instance.
 	 * 
-	 * Check for token state with {@link getSecurityToken()} and {@link SecurityToken->isEnabled()}.
+	 * Check for token state with {@link getSecurityToken()} and 
+	 * {@link SecurityToken->isEnabled()}.
+	 *
+	 * @return Form
 	 */
 	public function enableSecurityToken() {
 		$this->securityToken = new SecurityToken();
+
 		return $this;
 	}
 	
 	/**
 	 * Returns the security token for this form (if any exists).
+	 *
 	 * Doesn't check for {@link securityTokenEnabled()}.
+	 *
 	 * Use {@link SecurityToken::inst()} to get a global token.
 	 * 
 	 * @return SecurityToken|null
@@ -1402,26 +1538,32 @@ class Form extends RequestHandler {
 	}
 		
 	/**
-	 * Returns the name of a field, if that's the only field that the current controller is interested in.
+	 * Returns the name of a field, if that's the only field that the current 
+	 * controller is interested in.
+	 *
 	 * It checks for a call to the callfieldmethod action.
-	 * This is useful for optimising your forms
 	 * 
 	 * @return string
 	 */
 	public static function single_field_required() {
-		if(self::current_action() == 'callfieldmethod') return $_REQUEST['fieldName'];
+		if(self::current_action() == 'callfieldmethod') {
+			return $_REQUEST['fieldName'];
+		}
 	}
 
 	/**
 	 * Return the current form action being called, if available.
-	 * This is useful for optimising your forms
+	 *
+	 * @return string
 	 */
 	public static function current_action() {
 		return self::$current_action;
 	}
 
 	/**
-	 * Set the current form action.  Should only be called by Controller.
+	 * Set the current form action. Should only be called by {@link Controller}.
+	 *
+	 * @param string $action
 	 */
 	public static function set_current_action($action) {
 		self::$current_action = $action;
@@ -1442,6 +1584,8 @@ class Form extends RequestHandler {
 	 *
 	 * @param string $class A string containing a classname or several class
 	 *				names delimited by a single space.
+	 *
+	 * @return Form
 	 */
 	public function addExtraClass($class) {
 		$classes = explode(' ', $class);
@@ -1464,6 +1608,7 @@ class Form extends RequestHandler {
 	public function removeExtraClass($class) {
 		$classes = explode(' ', $class);
 		$this->extraClasses = array_diff($this->extraClasses, $classes);
+
 		return $this;
 	}
 	
@@ -1494,9 +1639,6 @@ class Form extends RequestHandler {
 		$data['action_' . $action] = true;
 
 		return Director::test($this->FormAction(), $data, Controller::curr()->getSession());
-		
-		//$response = $this->controller->run($data);
-		//return $response;
 	}
 	
 	/**
@@ -1515,6 +1657,7 @@ class Form extends RequestHandler {
  * @subpackage core
  */
 class Form_FieldMap extends ViewableData {
+
 	protected $form;
 	
 	public function __construct($form) {
@@ -1523,7 +1666,10 @@ class Form_FieldMap extends ViewableData {
 	}
 	
 	/**
-	 * Ensure that all potential method calls get passed to __call(), therefore to dataFieldByName
+	 * Ensure that all potential method calls get passed to __call(), therefore 
+	 * to dataFieldByName.
+	 *
+	 * @param string
 	 */
 	public function hasMethod($method) {
 		return true;

forms/FormField.php

@@ -1,18 +1,21 @@
 <?php
+
 /**
  * Represents a field in a form. 
  *  
- * A FieldList contains a number of FormField objects which make up the whole of a form.
+ * A FieldList contains a number of FormField objects which make up the whole
- * In addition to single fields, FormField objects can be "composite", for example, the {@link TabSet}
+ * of a form. In addition to single fields, FormField objects can be
- * field.  Composite fields let us define complex forms without having to resort to custom HTML.
+ * "composite", for example, the {@link TabSet} field. Composite fields let us
+ * define complex forms without having to resort to custom HTML.
  * 
  * <b>Subclassing</b>
  * 
- * Define a {@link dataValue()} method that returns a value suitable for inserting into a single database field. 
+ * Define a {@link dataValue()} method that returns a value suitable for
- * For example, you might tidy up the format of a date or currency field.
+ * inserting into a single database field. For example, you might tidy up the
- * Define {@link saveInto()} to totally customise saving. 
+ * format of a date or currency field. Define {@link saveInto()} to totally
- * For example, data might be saved to the filesystem instead of the data record, 
+ * customise saving. For example, data might be saved to the filesystem instead
- * or saved to a component of the data record instead of the data record itself.
+ * of the data record, or saved to a component of the data record instead of
+ * the data record itself.
  * 
  * @package forms
  * @subpackage core
@@ -112,6 +115,7 @@ class FormField extends RequestHandler {
 		} else {
 			$label = $fieldName;
 		}
+
 		$label = preg_replace("/([a-z]+)([A-Z])/","$1 $2", $label);
 		
 		return $label;
@@ -119,9 +123,16 @@ class FormField extends RequestHandler {
 
 	/**
 	 * Construct and return HTML tag.
+	 *
+	 * @param string $tag
+	 * @param array $attributes
+	 * @param mixed $content
+	 *
+	 * @return string
 	 */
 	public static function create_tag($tag, $attributes, $content = null) {
 		$preparedAttributes = '';
+
 		foreach($attributes as $k => $v) {
 			// Note: as indicated by the $k == value item here; the decisions over what to include in the attributes
 			// can sometimes get finicky
@@ -130,15 +141,20 @@ class FormField extends RequestHandler {
 			}
 		}
 
-		if($content || $tag != 'input') return "<$tag$preparedAttributes>$content</$tag>";
+		if($content || $tag != 'input') {
-		else return "<$tag$preparedAttributes />";
+			return "<$tag$preparedAttributes>$content</$tag>";
+		}
+		else {
+			return "<$tag$preparedAttributes />";
+		}
 	}
 
 	/**
 	 * Create a new field.
-	 * @param name The internal field name, passed to forms.
+	 *
-	 * @param title The field label.
+	 * @param string $name The internal field name, passed to forms.
-	 * @param value The value of the field.
+	 * @param string $title The field label.
+	 * @param mixed $value The value of the field.
 	 */
 	public function __construct($name, $title = null, $value = null) {
 		$this->name = $name;
@@ -150,7 +166,11 @@ class FormField extends RequestHandler {
 	}
 	
 	/**
-	 * Return a Link to this field
+	 * Return a link to this field.
+	 *
+	 * @param string $action
+	 *
+	 * @return string
 	 */
 	public function Link($action = null) {
 		return Controller::join_links($this->form->FormAction(), 'field/' . $this->name, $action);
@@ -158,17 +178,44 @@ class FormField extends RequestHandler {
 	
 	/**
 	 * Returns the HTML ID of the field - used in the template by label tags.
+	 *
 	 * The ID is generated as FormName_FieldName.  All Field functions should ensure
 	 * that this ID is included in the field.
+	 *
+	 * @return string
 	 */
 	public function ID() {
-		$name = preg_replace('/(^-)|(-$)/', '', preg_replace('/[^A-Za-z0-9_-]+/', '-', $this->name));
+		return $this->getTemplateHelper()->generateFieldID($this);
-		if($this->form) return $this->form->FormName() . '_' . $name;
-		else return $name;
 	}
 	
 	/**
-	 * Returns the field name - used by templates.
+	 * Returns the HTML ID for the form field holder element.
+	 *
+	 * @return string
+	 */
+	public function HolderID() {
+		return $this->getTemplateHelper()->generateFieldHolderID($this);
+	}
+
+	/**
+	 * Returns the current {@link FormTemplateHelper} on either the parent
+	 * Form or the global helper set through the {@link Injector} layout.
+	 *
+	 * To customize a single {@link FormField}, use {@link setTemplate} and
+	 * provide a custom template name.
+	 *
+	 * @return FormTemplateHelper
+	 */
+	public function getTemplateHelper() {
+		if($this->form) {
+			return $this->form->getTemplateHelper();
+		}
+
+		return Injector::inst()->get('FormTemplateHelper');
+	}
+
+	/**
+	 * Returns the raw field name.
 	 * 
 	 * @return string
 	 */
@@ -178,6 +225,7 @@ class FormField extends RequestHandler {
 
 	/** 
 	 * Returns the field message, used by form validation.
+	 *
 	 * Use {@link setError()} to set this property.
 	 * 
 	 * @return string
@@ -188,9 +236,9 @@ class FormField extends RequestHandler {
 	
 	/** 
 	 * Returns the field message type, used by form validation.
-	 * Arbitrary value which is mostly used for CSS classes
+	 *
-	 * in the rendered HTML, e.g. "required".
+	 * Arbitrary value which is mostly used for CSS classes in the rendered HTML,
-	 * Use {@link setError()} to set this property.
+	 * e.g. "required". Use {@link setError()} to set this property.
 	 * 
 	 * @return string
 	 */
@@ -206,7 +254,8 @@ class FormField extends RequestHandler {
 	}
 	
 	/**
-	 * Method to save this form field into the given data object.
+	 * Method to save this form field into the given {@link DataObject}.
+	 *
 	 * By default, makes use of $this->dataValue()
 	 * 
 	 * @param DataObjectInterface $record DataObject to save data into
@@ -218,7 +267,10 @@ class FormField extends RequestHandler {
 	}
 	
 	/**
-	 * Returns the field value suitable for insertion into the data object
+	 * Returns the field value suitable for insertion into the
+	 * {@link DataObject}.
+	 *
+	 * @return mixed
 	 */
 	public function dataValue() {
 		return $this->value;
@@ -226,11 +278,18 @@ class FormField extends RequestHandler {
 	
 	/**
 	 * Returns the field label - used by templates.
+	 *
+	 * @return string
 	 */
 	public function Title() {
 		return $this->title;
 	}
 	
+	/**
+	 * @param string $val
+	 *
+	 * @return FormField
+	 */
 	public function setTitle($val) {
 		$this->title = $val;
 		return $this;
@@ -324,7 +383,7 @@ class FormField extends RequestHandler {
 	 * - 'name': {@link setName}
 	 * 
 	 * CAUTION Doesn't work on most fields which are composed of more than one HTML form field:
-	 * AjaxUniqueTextField, CheckboxSetField, ComplexTableField, CompositeField, ConfirmedPasswordField,
+	 * AjaxUniqueTextField, CheckboxSetField, CompositeField, ConfirmedPasswordField,
 	 * CountryDropdownField, CreditCardField, CurrencyField, DateField, DatetimeField, FieldGroup, GridField,
 	 * HtmlEditorField, ImageField, ImageFormAction, InlineFormAction, ListBoxField, etc.
 	 * 
@@ -344,6 +403,7 @@ class FormField extends RequestHandler {
 	 */
 	public function getAttribute($name) {
 		$attrs = $this->getAttributes();
+
 		return @$attrs[$name];
 	}
 	
@@ -366,12 +426,15 @@ class FormField extends RequestHandler {
 	/**
 	 * @param Array Custom attributes to process. Falls back to {@link getAttributes()}.
 	 * If at least one argument is passed as a string, all arguments act as excludes by name.
+	 *
 	 * @return string HTML attributes, ready for insertion into an HTML tag
 	 */
 	public function getAttributesHTML($attrs = null) {
 		$exclude = (is_string($attrs)) ? func_get_args() : null;
 
-		if(!$attrs || is_string($attrs)) $attrs = $this->getAttributes();
+		if(!$attrs || is_string($attrs)) {
+			$attrs = $this->getAttributes();
+		}
 
 		// Remove empty
 		$attrs = array_filter((array)$attrs, function($v) {
@@ -379,10 +442,13 @@ class FormField extends RequestHandler {
 		}); 
 
 		// Remove excluded
-		if($exclude) $attrs = array_diff_key($attrs, array_flip($exclude));
+		if($exclude) {
+			$attrs = array_diff_key($attrs, array_flip($exclude));
+		}
 
-		// Create markkup
+		// Create markup
 		$parts = array();
+
 		foreach($attrs as $name => $value) {
 			$parts[] = ($value === true) ? "{$name}=\"{$name}\"" : "{$name}=\"" . Convert::raw2att($value) . "\"";
 		}
@@ -391,13 +457,20 @@ class FormField extends RequestHandler {
 	}
 
 	/**
-	 * Returns a version of a title suitable for insertion into an HTML attribute
+	 * Returns a version of a title suitable for insertion into an HTML
+	 * attribute.
+	 *
+	 * @return string
 	 */
 	public function attrTitle() {
 		return Convert::raw2att($this->title);
 	}
+
 	/**
-	 * Returns a version of a title suitable for insertion into an HTML attribute
+	 * Returns a version of a title suitable for insertion into an HTML
+	 * attribute.
+	 *
+	 * @return string
 	 */
 	public function attrValue() {
 		return Convert::raw2att($this->value);
@@ -405,30 +478,43 @@ class FormField extends RequestHandler {
 	
 	/**
 	 * Set the field value.
-	 * 
+	 *
 	 * @param mixed $value
-	 * @return FormField Self reference
+	 *
+	 * @return FormField.
 	 */
 	public function setValue($value) {
 		$this->value = $value;
+
 		return $this;
 	}
 	
 	/**
 	 * Set the field name
+	 *
+	 * @param string $name
+	 *
+	 * @return FormField
 	 */
 	public function setName($name) {
 		$this->name = $name;
+
 		return $this;
 	}
 	
 	/**
 	 * Set the container form.
-	 * This is called whenever you create a new form and put fields inside it, so that you don't
+	 *
-	 * have to worry about linking the two.
+	 * This is called whenever you create a new form and put fields inside it,
+	 * so that you don't have to worry about linking the two.
+	 *
+	 * @param Form
+	 *
+	 * @return FormField
 	 */
 	public function setForm($form) {
 		$this->form = $form; 
+
 		return $this;
 	}
 	
@@ -442,20 +528,30 @@ class FormField extends RequestHandler {
 	}
 	
 	/**
-	 * Return TRUE if security token protection is enabled on the parent {@link Form}.
+	 * Return TRUE if security token protection is enabled on the parent
+	 * {@link Form}.
 	 *
 	 * @return bool
 	 */
 	public function securityTokenEnabled() {
 		$form = $this->getForm();
-		if(!$form) return false;
+
+		if(!$form) {
+			return false;
+		}
 		
 		return $form->getSecurityToken()->isEnabled();
 	}
 	
 	/**
-	 * Sets the error message to be displayed on the form field
+	 * Sets the error message to be displayed on the {@link FormField}.
-	 * Set by php validation of the form
+	 *
+	 * Set by php validation of the form.
+	 *
+	 * @param string $message
+	 * @param string $messageType
+	 *
+	 * @return FormField
 	 */
 	public function setError($message, $messageType) {
 		$this->message = $message; 
@@ -467,9 +563,11 @@ class FormField extends RequestHandler {
 	/**
 	 * Set the custom error message to show instead of the default
 	 * format of Please Fill In XXX. Different from setError() as
-	 * that appends it to the standard error messaging
+	 * that appends it to the standard error messaging.
 	 * 
-	 * @param string Message for the error
+	 * @param string $msg Message for the error
+	 *
+	 * @return FormField
 	 */
 	public function setCustomValidationMessage($msg) {
 		$this->customValidationMessage = $msg;
@@ -482,7 +580,6 @@ class FormField extends RequestHandler {
 	 * message has not been defined then just return blank. The default
 	 * error is defined on {@link Validator}.
 	 *
-	 * @todo Should the default error message be stored here instead
 	 * @return string
 	 */
 	public function getCustomValidationMessage() {
@@ -491,10 +588,13 @@ class FormField extends RequestHandler {
 
 	/**
 	 * Set name of template (without path or extension).
-	 * Caution: Not consistently implemented in all subclasses,
+	 *
-	 * please check the {@link Field()} method on the subclass for support.
+	 * Caution: Not consistently implemented in all subclasses, please check
+	 * the {@link Field()} method on the subclass for support.
 	 * 
-	 * @param string
+	 * @param string $template
+	 *
+	 * @return FormField
 	 */
 	public function setTemplate($template) {
 		$this->template = $template;
@@ -523,7 +623,9 @@ class FormField extends RequestHandler {
 	 * Caution: Not consistently implemented in all subclasses,
 	 * please check the {@link Field()} method on the subclass for support.
 	 * 
-	 * @param string
+	 * @param string $template
+	 *
+	 * @return FormField
 	 */
 	public function setFieldHolderTemplate($template) {
 		$this->fieldHolderTemplate = $template;

forms/FormTemplateHelper.php

@@ -0,0 +1,135 @@
+<?php
+
+/**
+ * A helper class for managing {@link Form} and {@link FormField} HTML template
+ * output.
+ *
+ * This primarily exists to maintain backwards compatibility between Form and
+ * FormField template changes since developers may rely on specific HTML output
+ * in their applications. Any core changes to templates (such as changing ID's)
+ * may have the potential to silently prevent websites from working.
+ *
+ * To provide a form with a custom FormTemplateHelper use the following snippet:
+ *
+ * <code>
+ * $form->setTemplateHelper('ClassName');
+ * </code>
+ *
+ * Globally, the FormTemplateHelper can be set via the {@link Injector} API.
+ *
+ * For backwards compatibility, with < 3.2 use the {@link FormTemplateHelper_Pre32}
+ * class which will preserve the old style form field attributes.
+ *
+ * <code>
+ *	Injector:
+ *	  FormTemplateHelper:
+ *	    class: FormTemplateHelper_Pre32
+ * </code>
+ *
+ * @package framework
+ * @subpackage forms
+ */
+class FormTemplateHelper {
+
+	/**
+	 * @param Form $form
+	 *
+	 * @return string
+	 */
+	public function generateFormID($form) {
+		if($id = $form->getHTMLID()) {
+			return Convert::raw2htmlid($id);
+		}
+
+		return Convert::raw2htmlid(
+			get_class($form) . '_' . str_replace(array('.', '/'), '', $form->getName())
+		);
+	}
+
+	/**
+	 * @param FormField $field
+	 *
+	 * @return string
+	 */
+	public function generateFieldHolderID($field) {
+		return $this->generateFieldID($field) . '_Holder';
+	}
+
+	/**
+	 * Generate the field ID value
+	 *
+	 * @param FormField
+	 *
+	 * @return string
+	 */
+	public function generateFieldID($field) {
+		if($form = $field->getForm()) {
+			return sprintf("%s_%s",
+				$this->generateFormID($form),
+				Convert::raw2htmlid($field->getName())
+			);
+		}
+
+		return Convert::raw2htmlid($field->getName());
+	}
+
+}
+
+/**
+ * Note that this will cause duplicate and invalid ID attributes.
+ *
+ * @deprecated 4.0
+ *
+ * @package framework
+ * @subpackage forms
+ */
+class FormTemplateHelper_Pre32 extends FormTemplateHelper {
+
+	/**
+	 * @param Form
+	 *
+	 * @return string
+	 */
+	public function generateFormID($form) {
+		if($id = $form->getHTMLID()) {
+			return $id;
+		}
+
+		return sprintf("%s_%s", 
+			$form->class,
+			str_replace(array('.', '/'), '', $form->getName())
+		);
+	}
+
+	/**
+	 * @param FormField
+	 *
+	 * @return string
+	 */
+	public function generateFieldHolderID($field) {
+		return $field->getName();
+	}
+
+	/**
+	 * @param FormField
+	 *
+	 * @return string
+	 */
+	public function generateFieldID($field) {
+		$name = preg_replace(
+			'/(^-)|(-$)/', '', 
+			preg_replace('/[^A-Za-z0-9_-]+/', '-', $field->getName())
+		);
+
+		if($form = $field->getForm()) {
+			$form = sprintf("%s_%s",
+				get_class($form),
+				str_replace(array('.', '/'), '', $form->getName())
+			);
+
+			return $form . '_' . $name;
+		}
+
+		return $name;
+	}
+}

javascript/HtmlEditorField.js

@@ -515,35 +515,51 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE;
 			redraw: function() {
 				this._super();
 
-				var linkType = this.find(':input[name=LinkType]:checked').val(), list = ['internal', 'external', 'file', 'email'];
+				var linkType = this.find(':input[name=LinkType]:checked').val(),
+					list = ['internal', 'external', 'file', 'email'];
 
 				this.addAnchorSelector();
 
 				// Toggle field visibility depending on the link type.
 				this.find('div.content .field').hide();
-				this.find('.field#LinkType').show();
+				this.find('.field[id$="LinkType_Holder"]').show();
-				this.find('.field#' + linkType).show();
+				this.find('.field[id$="' + linkType +'_Holder"]').show();
-				if(linkType == 'internal' || linkType == 'anchor') this.find('.field#Anchor').show();
+
-				if(linkType !== 'email') this.find('.field#TargetBlank').show();
+				if(linkType == 'internal' || linkType == 'anchor') {
+					this.find('.field[id$="Anchor_Holder"]').show();
+				}
+
+				if(linkType !== 'email') {
+					this.find('.field[id$="TargetBlank_Holder"]').show();
+				}
+
 				if(linkType == 'anchor') {
-					this.find('.field#AnchorSelector').show();
+					this.find('.field[id$="AnchorSelector_Holder"]').show();
-					this.find('.field#AnchorRefresh').show();
+					this.find('.field[id$="AnchorRefresh_Holder"]').show();
 				}
 			},
 			/**
 			 * @return Object Keys: 'href', 'target', 'title'
 			 */
 			getLinkAttributes: function() {
-				var href, target = null, anchor = this.find(':input[name=Anchor]').val();
+				var href,
+					target = null,
+					anchor = this.find(':input[name=Anchor]').val();
 				
 				// Determine target
-				if(this.find(':input[name=TargetBlank]').is(':checked')) target = '_blank';
+				if(this.find(':input[name=TargetBlank]').is(':checked')) {
-				
+					target = '_blank';
+				}
+
 				// All other attributes
 				switch(this.find(':input[name=LinkType]:checked').val()) {
 					case 'internal':
 						href = '[sitetree_link,id=' + this.find(':input[name=internal]').val() + ']';
-						if(anchor) href += '#' + anchor;
+
+						if(anchor) {
+							href += '#' + anchor;
+						}
+
 						break;
 
 					case 'anchor':
@@ -578,12 +594,14 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE;
 				this.modifySelection(function(ed){
 					ed.insertLink(this.getLinkAttributes());
 				});
+
 				this.updateFromEditor();
 			},
 			removeLink: function() {
 				this.modifySelection(function(ed){
 					ed.removeLink();
 				});
+
 				this.close();
 			},
 			addAnchorSelector: function() {
@@ -1229,7 +1247,9 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE;
 				this.setOrigVal(parseInt(this.val(), 10));
 
 				// Default to a managable size for the HTML view. Can be overwritten by user after initialization
-				if(this.attr('name') == 'Width') this.closest('.ss-htmleditorfield-file').updateDimensions('Width', 600);
+				if(this.attr('name') == 'Width') {
+					this.closest('.ss-htmleditorfield-file').updateDimensions('Width', 600);
+				}
 
 			},
 			onunmatch: function() {
@@ -1307,7 +1327,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE;
 		});
 
 
-		$('form.htmleditorfield-mediaform #ParentID .TreeDropdownField').entwine({
+		$('form.htmleditorfield-mediaform .field[id$="ParentID_Holder"] .TreeDropdownField').entwine({
 			onadd: function() {
 				this._super();
 

javascript/UploadField.js

@@ -434,8 +434,11 @@
 			},
 			toggleEditForm: function() {
 				var itemInfo = this.prev('.ss-uploadfield-item-info'), status = itemInfo.find('.ss-uploadfield-item-status');
-				var iframe = this.find('iframe').contents(), saved=iframe.find('#Form_EditForm_error');
+
-				var text="";
+				var iframe = this.find('iframe').contents(),
+					saved = iframe.find('#Form_EditForm_error');
+
+				var text = "";
 
 				if(this.height() === 0) {
 					text = ss.i18n._t('UploadField.Editing', "Editing ...");

javascript/UploadField_select.js

@@ -1,7 +1,7 @@
 (function($) {
 	$.entwine('ss', function($) {
 		// Install the directory selection handler
-		$('form.uploadfield-form #ParentID .TreeDropdownField').entwine({
+		$('form.uploadfield-form .TreeDropdownField').entwine({
 			onmatch: function() {
 				this._super();
 

scss/AssetUploadField.scss

@@ -7,7 +7,7 @@
 @import "_elementMixins";
 
 // Temporary. To be hidden and replaced with javascript tooltip
-.ss-uploadfield-view-allowed-extensions{
+.ss-uploadfield-view-allowed-extensions {
 	padding-top:5px;
 	clear:both;
 	max-width:750px;
@@ -19,20 +19,10 @@
 	}
 }
 
-#AssetUploadField {
-	border-bottom: 0;
-	@include box-shadow(none);
-}
 .backlink {
 	padding-left: 12px;
 }
 
-#Form_EditorToolbarMediaForm {
-	.ui-tabs-panel {
-		padding-left: 0px;
-	}
-}
-
 body.cms.ss-uploadfield-edit-iframe, .composite.ss-assetuploadfield .details fieldset {
 	padding: $grid-x*2;
 	overflow: auto;
@@ -52,6 +42,9 @@ body.cms.ss-uploadfield-edit-iframe, .composite.ss-assetuploadfield .details fie
 }
 
 .ss-assetuploadfield {
+	border-bottom: 0;
+	@include box-shadow(none);
+
 	h3 {
 		border-bottom: 1px solid $color-shadow-light;
 		@include box-shadow(0 1px 0 lighten($color-shadow-light, 95%));

scss/ComplexTableField_popup.scss

@@ -1,115 +0,0 @@
-html {
-	overflow-y: auto !important;
-}
-body {
-	height: 100%;
-}
-
-#ComplexTableField_Popup_DetailForm input.loading {
-	background: #fff url(../images/network-save.gif) left center no-repeat;
-	padding-left: 16px;
-}
-
-.PageControls {
-	padding: 5px;
-	width: 100%;
-
-	* {
-		vertical-align: middle;
-	}
-
-	.Left {
-		width: 33%;
-	}
-
-	.Count {
-		width: 33%;
-		text-align: center;
-	}
-
-	.Right {
-		width: 33%;
-		text-align: right;
-	}
-}
-
-.ComplexTableField_Popup {
-	td.hidden {
-		display: none;
-	}
-	th.HiddenField{
-		display: none;
-	}
-
-	span.right{
-		float: right;
-		clear: none;
-	}
-	span.left{
-		float: left;
-		clear: none;
-	}
-	form p.checkbox input {
-		margin:0pt 1px;
-	}
-	form ul.optionset {
-		margin: 0;
-		padding: 0;
-	}
-	form ul.optionset li {
-		margin: 4px 0;
-	}
-
-	form div.Actions input {
-		font-size: 11px;
-		margin-top: 10px;
-	}
-}
-
-/* Pagination */ 
-#ComplexTableField_Pagination,
-#ComplexTableField_Pagination * {
-	vertical-align: middle;
-}
-
-#ComplexTableField_Pagination {
-	margin-top: 10px;
-	margin-left: auto;
-	margin-right: auto;
-	font-size: 11px;
-
-	a {
-		/*font-size: 1.2em;*/
-		font-size: 13px;
-		font-weight: bold;
-		text-decoration: none;
-		width: 1px;
-		height: 1px;
-		margin: 1px;
-	}
-
-	a:hover {
-		background: none;
-	}
-
-	span {
-		display: inline;
-		font-weight: bold;
-		font-size: 15px;
-		color: #f00;
-	}
-
-	div {
-		display: inline;
-	}
-}
-
-#ComplexTableField_Pagination_Previous {
-	padding-right: 10px;
-}
-#ComplexTableField_Pagination_Next {
-	padding-left: 10px;
-}
-#ComplexTableField_Pagination_Next img,#ComplexTableField_Pagination_Previous img {
-	margin: 0 3px 2px;
-}

templates/forms/FormField_holder.ss

@@ -1,4 +1,4 @@
-<div id="$Name" class="field<% if extraClass %> $extraClass<% end_if %>">
+<div id="$HolderID" class="field<% if extraClass %> $extraClass<% end_if %>">
 	<% if Title %><label class="left" for="$ID">$Title</label><% end_if %>
 	<div class="middleColumn">
 		$Field

tests/core/ConvertTest.php

@@ -1,11 +1,15 @@
 <?php
+
 /**
  * Test various functions on the {@link Convert} class.
+ *
  * @package framework
  * @subpackage tests
  */
 class ConvertTest extends SapphireTest {
 
+	protected $usesDatabase = false;
+
 	/**
 	 * Tests {@link Convert::raw2att()}
 	 */
@@ -81,9 +85,18 @@ class ConvertTest extends SapphireTest {
 			'Newlines are retained. They should not be replaced with <br /> as it is not XML valid');
 	}
 	
-	public function testRaw2HtmlName() {
+	/**
+	 * Tests {@link Convert::raw2htmlid()}
+	 */
+	public function testRaw2HtmlID() {
 		$val1 = 'test test 123';
-		$this->assertEquals('testtest123', Convert::raw2htmlname($val1));
+		$this->assertEquals('test_test_123', Convert::raw2htmlid($val1));
+
+		$val1 = 'test[test][123]';
+		$this->assertEquals('test_test_123', Convert::raw2htmlid($val1));
+
+		$val1 = '[test[[test]][123]]';
+		$this->assertEquals('test_test_123', Convert::raw2htmlid($val1));
 	}
 	
 	/**

tests/forms/FieldListTest.php

@@ -126,24 +126,28 @@ class FieldListTest extends SapphireTest {
 		/* We have no fields in the tab now */
 		$this->assertEquals(0, $tab->Fields()->Count());
 	}
-	
+
-	/**
-	 * Test removing a field from a set by it's name.
-	 */
 	public function testRemoveFieldByName() {
 		$fields = new FieldList();
-		
-		/* First of all, we add a field into our FieldList object */
 		$fields->push(new TextField('Name', 'Your name'));
 		
-		/* We have 1 field in our set now */
 		$this->assertEquals(1, $fields->Count());
-		
-		/* Then, we call up removeByName() to take it out again */
 		$fields->removeByName('Name');
-		
-		/* We have 0 fields in our set now, as we've just removed the one we added */
 		$this->assertEquals(0, $fields->Count());
+
+		$fields->push(new TextField('Name[Field]', 'Your name'));
+		$this->assertEquals(1, $fields->Count());
+		$fields->removeByName('Name[Field]');
+		$this->assertEquals(0, $fields->Count());
+	}
+
+	public function testDataFieldByName() {
+		$fields = new FieldList();
+		$fields->push($basic = new TextField('Name', 'Your name'));
+		$fields->push($brack = new TextField('Name[Field]', 'Your name'));
+
+		$this->assertEquals($basic, $fields->dataFieldByName('Name'));
+		$this->assertEquals($brack, $fields->dataFieldByName('Name[Field]'));
 	}
 	
 	/**
@@ -177,14 +181,19 @@ class FieldListTest extends SapphireTest {
 		/* A field gets added to the set */
 		$fields->addFieldToTab('Root', new TextField('Country'));
 
-		/* We have the same object as the one we pushed */
 		$this->assertSame($fields->dataFieldByName('Country'), $tab->fieldByName('Country'));
 		
-		/* The field called Country is replaced by the field called Email */
 		$fields->replaceField('Country', new EmailField('Email'));
-		
+		$this->assertEquals(1, $tab->Fields()->Count());
-		/* We have 1 field inside our tab */
+
-		$this->assertEquals(1, $tab->Fields()->Count());		
+		$fields = new FieldList();
+		$fields->push(new TextField('Name', 'Your name'));
+		$brack = new TextField('Name[Field]', 'Your name');
+
+		$fields->replaceField('Name', $brack);
+		$this->assertEquals(1, $fields->Count());
+
+		$this->assertEquals('Name[Field]', $fields->first()->getName());
 	}
 	
 	public function testRenameField() {

tests/forms/FormTest.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @package framework
  * @subpackage tests
@@ -12,7 +13,7 @@ class FormTest extends FunctionalTest {
 		'FormTest_Team',
 	);
 
-	function setUp() {
+	public function setUp() {
 		parent::setUp();
 
 		Config::inst()->update('Director', 'rules', array(
@@ -237,21 +238,21 @@ class FormTest extends FunctionalTest {
 				// leaving out "Required" field
 			)
 		);
+
 		$this->assertPartialMatchBySelector(
-			'#Email span.message',
+			'#Form_Form_Email_Holder span.message',
 			array(
 				'Please enter an email address'
 			),
 			'Formfield validation shows note on field if invalid'
 		);
 		$this->assertPartialMatchBySelector(
-			'#SomeRequiredField span.required',
+			'#Form_Form_SomeRequiredField_Holder span.required',
 			array(
 				'"Some Required Field" is required'
 			),
 			'Required fields show a notification on field when left blank'
 		);
-		
 	}
 	
 	public function testSessionSuccessMessage() {
@@ -433,6 +434,10 @@ class FormTest extends FunctionalTest {
 	
 }
 
+/**
+ * @package framework
+ * @subpackage tests
+ */
 class FormTest_Player extends DataObject implements TestOnly {
 	private static $db = array(
 		'Name' => 'Varchar',
@@ -454,6 +459,10 @@ class FormTest_Player extends DataObject implements TestOnly {
 	
 }
 
+/**
+ * @package framework
+ * @subpackage tests
+ */
 class FormTest_Team extends DataObject implements TestOnly {
 	private static $db = array(
 		'Name' => 'Varchar',
@@ -465,6 +474,10 @@ class FormTest_Team extends DataObject implements TestOnly {
 	);
 }
 
+/**
+ * @package framework
+ * @subpackage tests
+ */
 class FormTest_Controller extends Controller implements TestOnly {
 	private static $url_handlers = array(
 		'$Action//$ID/$OtherID' => "handleAction",
@@ -510,6 +523,10 @@ class FormTest_Controller extends Controller implements TestOnly {
 
 }
 
+/**
+ * @package framework
+ * @subpackage tests
+ */
 class FormTest_ControllerWithSecurityToken extends Controller implements TestOnly {
 	private static $url_handlers = array(
 		'$Action//$ID/$OtherID' => "handleAction",

tests/model/MoneyTest.php

@@ -280,15 +280,24 @@ class MoneyTest extends SapphireTest {
 
 }
 
+/**
+ * @package framework
+ * @subpackage tests
+ */
 class MoneyTest_DataObject extends DataObject implements TestOnly {
 	private static $db = array(
 		'MyMoney' => 'Money', 
 		//'MyOtherMoney' => 'Money', 
 	);
-
 }
+
+/**
+ * @package framework
+ * @subpackage tests
+ */
 class MoneyTest_SubClass extends MoneyTest_DataObject implements TestOnly {
-	static $db = array(
+
+	private static $db = array(
 		'MyOtherMoney' => 'Money', 
 	);