Merge remote-tracking branch 'origin/3.1' into 3.2

Conflicts:
	dev/SapphireTest.php
	docs/en/02_Developer_Guides/01_Templates/01_Syntax.md
	forms/DatetimeField.php
	forms/NullableField.php
	forms/NumericField.php
	forms/gridfield/GridField.php
	tests/control/DirectorTest.php
	tests/model/DataObjectSchemaGenerationTest.php
	tests/model/MySQLDatabaseTest.php

admin/css/screen.css

@@ -557,11 +557,11 @@ body.cms { overflow: hidden; }
 .cms-content-batchactions { float: left; position: relative; display: block; }
 .cms-content-batchactions .view-mode-batchactions-wrapper { height: 18px; float: left; padding: 4px 6px; border: 1px solid #aaa; margin-bottom: 8px; margin-right: -1px; background-color: #D9D9D9; background-image: url('data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4gPHN2ZyB2ZXJzaW9uPSIxLjEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyI+PGRlZnM+PGxpbmVhckdyYWRpZW50IGlkPSJncmFkIiBncmFkaWVudFVuaXRzPSJvYmplY3RCb3VuZGluZ0JveCIgeDE9IjAuNSIgeTE9IjAuMCIgeDI9IjAuNSIgeTI9IjEuMCI+PHN0b3Agb2Zmc2V0PSIwJSIgc3RvcC1jb2xvcj0iI2ZmZmZmZiIvPjxzdG9wIG9mZnNldD0iMTAwJSIgc3RvcC1jb2xvcj0iI2Q5ZDlkOSIvPjwvbGluZWFyR3JhZGllbnQ+PC9kZWZzPjxyZWN0IHg9IjAiIHk9IjAiIHdpZHRoPSIxMDAlIiBoZWlnaHQ9IjEwMCUiIGZpbGw9InVybCgjZ3JhZCkiIC8+PC9zdmc+IA=='); background-size: 100%; background-image: -webkit-gradient(linear, 50% 0%, 50% 100%, color-stop(0%, #ffffff), color-stop(100%, #d9d9d9)); background-image: -moz-linear-gradient(top, #ffffff, #d9d9d9); background-image: -webkit-linear-gradient(top, #ffffff, #d9d9d9); background-image: linear-gradient(to bottom, #ffffff, #d9d9d9); border-top-left-radius: 4px; border-bottom-left-radius: 4px; }
 .cms-content-batchactions .view-mode-batchactions-wrapper input { vertical-align: middle; }
-.cms-content-batchactions .view-mode-batchactions-wrapper label { vertical-align: middle; display: none; }
+.cms-content-batchactions .view-mode-batchactions-wrapper .view-mode-batchactions-label { vertical-align: middle; display: none; }
 .cms-content-batchactions .view-mode-batchactions-wrapper fieldset, .cms-content-batchactions .view-mode-batchactions-wrapper .Actions { display: inline-block; }
 .cms-content-batchactions .view-mode-batchactions-wrapper #view-mode-batchactions { margin-top: 2px; }
 .cms-content-batchactions.inactive .view-mode-batchactions-wrapper { border-radius: 4px; }
-.cms-content-batchactions.inactive .view-mode-batchactions-wrapper label { display: inline; }
+.cms-content-batchactions.inactive .view-mode-batchactions-wrapper .view-mode-batchactions-label { display: inline; }
 .cms-content-batchactions form > * { display: block; float: left; }
 .cms-content-batchactions form.cms-batch-actions { float: left; }
 .cms-content-batchactions.inactive form { display: none; }

admin/scss/_style.scss

@@ -907,7 +907,7 @@ body.cms {
 			vertical-align: middle;
 		}
 
-		label {
+		.view-mode-batchactions-label {
 			vertical-align: middle;
 			display: none;
 		}
@@ -923,7 +923,7 @@ body.cms {
 		&.inactive .view-mode-batchactions-wrapper {
 			border-radius: 4px;
 
-			label {
+			.view-mode-batchactions-label {
 				display: inline;
 			}
 		}

dev/SapphireTest.php

@@ -157,9 +157,20 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 		return static::$fixture_file;
 	}
 
+	/**
+	 * @var array $fixtures Array of {@link YamlFixture} instances
+	 * @deprecated 3.1 Use $fixtureFactory instad
+	 */
+	protected $fixtures = array();
+
 	protected $model;
 
 	public function setUp() {
+
+		//nest config and injector for each test so they are effectively sandboxed per test
+		Config::nest();
+		Injector::nest();
+
 		// We cannot run the tests on this abstract class.
 		if(get_class($this) == "SapphireTest") $this->skipTest = true;
 
@@ -284,6 +295,10 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 * for tearing down the state again.
 	 */
 	public function setUpOnce() {
+
+		//nest config and injector for each suite so they are effectively sandboxed
+		Config::nest();
+		Injector::nest();
 		$isAltered = false;
 
 		if(!Director::isDev()) {
@@ -332,24 +347,12 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 * tearDown method that's called once per test class rather once per test method.
 	 */
 	public function tearDownOnce() {
-		// If we have made changes to the extensions present, then migrate the database schema.
+		//unnest injector / config now that the test suite is over
-		if($this->extensionsToReapply || $this->extensionsToRemove) {
+		// this will reset all the extensions on the object too (see setUpOnce)
-			// Remove extensions added for testing
+		Injector::unnest();
-			foreach($this->extensionsToRemove as $class => $extensions) {
+		Config::unnest();
-				foreach($extensions as $extension) {
-					$class::remove_extension($extension);
-				}
-			}
 
-			// Reapply ones removed
+		if(!empty($this->extensionsToReapply) || !empty($this->extensionsToRemove) || !empty($this->extraDataObjects)) {
-			foreach($this->extensionsToReapply as $class => $extensions) {
-				foreach($extensions as $extension) {
-					$class::add_extension($extension);
-				}
-			}
-		}
-
-		if($this->extensionsToReapply || $this->extensionsToRemove || $this->extraDataObjects) {
 			$this->resetDBSchema();
 		}
 	}
@@ -499,6 +502,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 			$controller->response->setStatusCode(200);
 			$controller->response->removeHeader('Location');
 		}
+		//unnest injector / config now that tests are over
+		Injector::unnest();
+		Config::unnest();
 	}
 
 	public static function assertContains(
@@ -763,7 +769,6 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 
 		$this->assertNotContains($needleSQL, $haystackSQL, $message, $ignoreCase, $checkForObjectIdentity);
 	}
-	
 	/**
 	 * Helper function for the DOS matchers
 	 */

docs/en/02_Developer_Guides/06_Testing/00_Unit_Testing.md

@@ -182,18 +182,10 @@ end of each test.
 				$page->publish('Stage', 'Live');
 			}
 
-			// reset configuration for the test.
+			// set custom configuration for the test.
-			Config::nest();
 			Config::inst()->update('Foo', 'bar', 'Hello!');
 		}
 
-		public function tearDown() {
-			// restores the config variables
-			Config::unnest();
-
-			parent::tearDown();
-		}
-
 		public function testMyMethod() {
 			// ..
 		}
@@ -224,6 +216,32 @@ individual test case.
 		}
 	}
 	
+### Config and Injector Nesting
+
+A powerful feature of both [`Config`](/developer_guides/configuration/configuration/) and [`Injector`](/developer_guides/extending/injector/) is the ability to "nest" them so that you can make changes that can easily be discarded without having to manage previous values.
+
+The testing suite makes use of this to "sandbox" each of the unit tests as well as each suite to prevent leakage between tests.
+
+If you need to make changes to `Config` (or `Injector) for each test (or the whole suite) you can safely update `Config` (or `Injector`) settings in the `setUp` or `tearDown` functions.
+
+It's important to remember that the `parent::setUp();` functions will need to be called first to ensure the nesting feature works as expected.
+
+	:::php
+	function setUpOnce() {
+		parent::setUpOnce();
+		//this will remain for the whole suite and be removed for any other tests
+		Config::inst()->update('ClassName', 'var_name', 'var_value');
+	}
+	
+	function testFeatureDoesAsExpected() {
+		//this will be reset to 'var_value' at the end of this test function
+		Config::inst()->update('ClassName', 'var_name', 'new_var_value');
+	}
+	
+	function testAnotherFeatureDoesAsExpected() {
+		Config::inst()->get('ClassName', 'var_name'); // this will be 'var_value'
+	}
+
 ## Generating a Coverage Report
 
 PHPUnit can generate a code coverage report ([docs](http://www.phpunit.de/manual/current/en/code-coverage-analysis.html))

docs/en/04_Changelogs/index.md

@@ -1,7 +1,7 @@
 title: Changelogs
 introduction: Key information on new features and improvements in each version.
 
-Keep up to date with new releases subscribe to the [SilverStripe Release Announcements](https://groups.google.com/group/silverstripe-announce) group,
+Keep up to date with new releases by subscribing to the [SilverStripe Release Announcements](https://groups.google.com/group/silverstripe-announce) group,
 or read our [blog posts about releases](http://silverstripe.org/blog/tag/release).
 
 We also keep an overview of [security-related releases](http://silverstripe.org/security-releases/).

docs/en/05_Contributing/00_Issues_and_Bugs.md

@@ -62,4 +62,4 @@ read our guide on [how to write secure code](/developer_guides/security/secure_c
 
 * [silverstripe.org/forums](http://www.silverstripe.org/community/forums/): Forums on silverstripe.org
 * [silverstripe-dev](http://groups.google.com/group/silverstripe-dev/): Core development mailinglist
-* [silverstripe-documentation](http://groups.google.com/group/silverstripe-translators/): Translation team mailing list
+* [silverstripe-documentation](http://groups.google.com/group/silverstripe-documentation/): Documentation mailing list

docs/en/05_Contributing/03_Documentation.md

@@ -16,8 +16,7 @@ page you want to edit. Alternatively, locate the appropriate .md file in the
 
 
  * After editing the documentation, describe your changes in the "commit summary" and "extended description" fields below then press "Commit Changes".
- * After that you will see a form to submit a Pull Request: "[pull requests](http://help.github.com/pull-requests/)". You should be able to adjust the version your 
+ * After that you will see a form to submit a Pull Request: "[pull requests](http://help.github.com/pull-requests/)". You should be able to adjust the version your documentation changes apply to and then submit the form. Your changes will be sent to the core committers for approval.
- * documentation changes are for and then submit the form. Your changes will be sent to the core committers for approval.
 
 <div class="warning" markdown='1'>
 You should make your changes in the lowest branch they apply to. For instance, if you fix a spelling issue that you found in the 3.1 documentation, submit your fix to that branch in Github and it'll be copied to the master (3.2) version of the documentation automatically. *Don't submit multiple pull requests*.
@@ -57,7 +56,7 @@ for documenting open source software.
 * Use PHPDoc in source code: Leave low level technical documentation to code comments within PHP, in [PHPDoc](http://en.wikipedia.org/wiki/PHPDoc) format. 
 * API and developer guides are two forms of source code documentation that complement each other.
 * API documentation should provide context, ie, the "bigger picture", by referring to developer guides inside your PHPDoc.
-* Make your documentation easy to find: Documentation lives by interlinking content so please make sure your contribution doesn't become an inaccessible island. At the very least, put a link to your should on the index page in the same folder. A link to your page can also appear
+* Make your documentation easy to find: Documentation is useful only when it is interlinked so please make sure your contribution doesn't become an inaccessible island. At the very least, put a link to your index page in the same folder. A link to your page can also appear
 as "related content" on other resource (e.g. `/tutorials/site_search` might link to `/developer_guides/forms/introduction`).
 
 ## Writing style
@@ -85,7 +84,7 @@ sparingly.
 "Tip box": A tip box is great for adding, deepening or accenting information in the main text. They can be used for background knowledge, or to provide links to further information (ie, a "see also" link).
 </div>
 
-Code:
+Code for a Tip box:
 
 	<div class="hint" markdown='1'>
 	...
@@ -95,7 +94,7 @@ Code:
 "Notification box": A notification box is good for technical notifications relating to the main text. For example, notifying users about a deprecated feature.
 </div>
 
-Code:
+Code for a Notification box:
 
 	<div class="notice" markdown='1'>
 	...
@@ -105,7 +104,7 @@ Code:
 "Warning box": A warning box is useful for highlighting a severe bug or a technical issue requiring a user's attention. For example, suppose a rare edge case sometimes leads to a variable being overwritten incorrectly. A warning box can be used to alert the user to this case so they can write their own code to handle it.  
 </div>
 
-Code:
+Code for a Warning box:
 
 	<div class="warning" markdown='1'>
 	...

docs/en/05_Contributing/05_Translation_Process.md

@@ -1,12 +1,12 @@
-title: Implement Internationalization
+title: Implement Internationalisation
-summary: Implement SilverStripe's internationalization system in your own modules.
+summary: Implement SilverStripe's internationalisation system in your own modules.
 
-# Implementing Internationalization
+# Implementing Internationalisation
 
-To find out about how to assist with translating SilverStripe from a users point of view, see the 
+To find out about how to assist with translating SilverStripe from a user's point of view, see the 
 [Contributing Translations page](/contributing/translations).
 
-## Set up your own module for localization
+## Set up your own module for localisation
 
 ### Collecting translatable text
 
@@ -33,7 +33,7 @@ source_lang = en
 type = YML
 ```
 
-If you don't have existing translations, your project is ready to go - simply point translators to the URL, have them 
+If you don't have existing translations to import, your project is ready to go - simply point translators to the URL, have them 
 sign up, and they can create languages and translations as required.
 
 ### Import existing translations
@@ -57,7 +57,7 @@ assumes you're adhering to the following guidelines:
  - Run the `i18nTextCollectorTask` with the `merge=true` option to avoid deleting unused entities
    (which might still be relevant in older release branches)
 
-### Converting your language files from 2.4 PHP format
+### Converting your language files from 2.4 PHP format to YML
 
 The conversion from PHP format to YML is taken care of by a module called 
 [i18n_yml_converter](https://github.com/chillu/i18n_yml_converter).
@@ -116,7 +116,7 @@ First of all, you need to create those source files in JSON, and store them in `
 	source_lang = en
 	type = KEYVALUEJSON
 
-Now you can upload the source files via a normal `tx push`. Once translations come in, you need to convert the source 
+Then you can upload the source files via a normal `tx push`. Once translations come in, you need to convert the source 
 files back into the JS files SilverStripe can actually read. This requires an installation of our 
 [buildtools](https://github.com/silverstripe/silverstripe-buildtools).
 

filesystem/File.php

@@ -633,7 +633,7 @@ class File extends DataObject {
 
 		// If it's changed, check for duplicates
 		if($oldName && $oldName != $name) {
-			$base = pathinfo($name, PATHINFO_BASENAME);
+			$base = pathinfo($name, PATHINFO_FILENAME);
 			$ext = self::get_file_extension($name);
 			$suffix = 1;
 
@@ -645,7 +645,7 @@ class File extends DataObject {
 				))->first()
 			) {
 				$suffix++;
-				$name = "$base-$suffix$ext";
+				$name = "$base-$suffix.$ext";
 			}
 		}
 

forms/CheckboxSetField.php

@@ -96,8 +96,12 @@ class CheckboxSetField extends OptionsetField {
 						}
 					}
 				} elseif($values && is_string($values)) {
+					if(!empty($values)) {
 						$items = explode(',', $values);
 						$items = str_replace('{comma}', ',', $items);
+					} else {
+						$items = array();
+					}
 				}
 			}
 		} else {
@@ -109,8 +113,12 @@ class CheckboxSetField extends OptionsetField {
 					$items = array();
 				}
 				else {
+					if(!empty($values)) {
 						$items = explode(',', $values);
 						$items = str_replace('{comma}', ',', $items);
+					} else {
+						$items = array();
+					}
 				}
 			}
 		}

forms/DatetimeField.php

@@ -63,7 +63,7 @@ class DatetimeField extends FormField {
 			->addExtraClass('fieldgroup-field');
 		$this->timeField = TimeField::create($name . '[time]', false)
 			->addExtraClass('fieldgroup-field');
-		$this->timezoneField = new HiddenField($this->getName() . '[timezone]');
+		$this->timezoneField = new HiddenField($name . '[timezone]');
 
 		parent::__construct($name, $title, $value);
 	}

forms/NullableField.php

@@ -1,35 +1,43 @@
 <?php
+
 /**
- * NullableField is a field that wraps other fields when you want to allow the user to specify whether the value of
+ * NullableField is a field that wraps other fields when you want to allow the user to specify
- * the field is null or not.
+ * whether the value of the field is null or not.
+ *
+ * The classic case is to wrap a TextField so that the user can distinguish between an empty string
+ * and a null string.
  *
- * The classic case is to wrap a TextField so that the user can distinguish between an empty string and a null string.
  * $a = new NullableField(new TextField("Field1", "Field 1", "abc"));
  *
- * It displays the field that is wrapped followed by a checkbox that is used to specify if the value is null or not.
+ * It displays the field that is wrapped followed by a checkbox that is used to specify if the
- * It uses the Title of the wrapped field for its title.
+ * value is null or not. It uses the Title of the wrapped field for its title.
- * When a form is submitted the field tests the value of the "is null" checkbox and sets its value accordingly.
+ *
- * You can retrieve the value of the wrapped field from the NullableField as follows:
+ * When a form is submitted the field tests the value of the "is null" checkbox and sets its value
+ * accordingly. You can retrieve the value of the wrapped field from the NullableField as follows:
+ *
  * $field->Value() or $field->dataValue()
  *
- * You can specify the label to use for the "is null" checkbox.  If you want to use I8N for this label then specify it
+ * You can specify the label to use for the "is null" checkbox. If you want to use i18n for this
- * like this:
+ * label then specify it like this:
  *
- * $field->setIsNullLabel(_T(SOME_MODULE_ISNULL_LABEL, "Is Null");
+ * $field->setIsNullLabel(_T(SOME_MODULE_ISNULL_LABEL, "Is Null"));
  *
  * @author Pete Bacon Darwin
+ *
  * @package forms
  * @subpackage fields-basic
  */
 class NullableField extends FormField {
 	/**
 	 * The field that holds the value of this field
+	 *
 	 * @var FormField
 	 */
 	protected $valueField;
 
 	/**
 	 * The label to show next to the is null check box.
+	 *
 	 * @var string
 	 */
 	protected $isNullLabel;
@@ -37,39 +45,55 @@ class NullableField extends FormField {
 
 	/**
 	 * Create a new nullable field
-	 * @param $valueField
+	 *
-	 * @return NullableField
+	 * @param FormField $valueField
+	 * @param null|string $isNullLabel
 	 */
 	public function __construct(FormField $valueField, $isNullLabel = null) {
 		$this->valueField = $valueField;
-		$this->isNullLabel = $isNullLabel;
+
-		if ( is_null($this->isNullLabel) ) {
+		if(isset($isNullLabel)) {
-			// Set a default label if one is not provided.
+			$this->setIsNullLabel($isNullLabel);
+		} else {
 			$this->isNullLabel = _t('NullableField.IsNullLabel', 'Is Null');
 		}
-		parent::__construct($valueField->getName(), $valueField->Title(), $valueField->Value(),
+
-			$valueField->getForm(), $valueField->RightTitle());
+		parent::__construct(
-		$this->readonly = $valueField->isReadonly();
+			$valueField->getName(),
+			$valueField->Title(),
+			$valueField->Value()
+		);
+
+		$this->setForm($valueField->getForm());
+		$this->setRightTitle($valueField->RightTitle());
+		$this->setReadonly($valueField->isReadonly());
 	}
 
 	/**
 	 * Get the label used for the Is Null checkbox.
+	 *
 	 * @return string
 	 */
 	public function getIsNullLabel() {
 		return $this->isNullLabel;
 	}
+
 	/**
 	 * Set the label used for the Is Null checkbox.
+	 *
 	 * @param $isNulLabel string
+	 *
+	 * @return $this
 	 */
-	public function setIsNullLabel(string $isNulLabel){
+	public function setIsNullLabel($isNulLabel) {
 		$this->isNullLabel = $isNulLabel;
+
 		return $this;
 	}
 
 	/**
 	 * Get the id used for the Is Null check box.
+	 *
 	 * @return string
 	 */
 	public function getIsNullId() {
@@ -77,8 +101,9 @@ class NullableField extends FormField {
 	}
 
 	/**
-	 * (non-PHPdoc)
+	 * @param array $properties
-	 * @see framework/forms/FormField#Field()
+	 *
+	 * @return string
 	 */
 	public function Field($properties = array()) {
 		if($this->isReadonly()) {
@@ -86,45 +111,71 @@ class NullableField extends FormField {
 		} else {
 			$nullableCheckbox = new CheckboxField($this->getIsNullId());
 		}
+
 		$nullableCheckbox->setValue(is_null($this->dataValue()));
 
-		return $this->valueField->Field() . ' ' . $nullableCheckbox->Field()
+		return sprintf(
-			. '&nbsp;<span>' . $this->getIsNullLabel().'</span>';
+			'%s %s&nbsp;<span>%s</span>',
+			$this->valueField->Field(),
+			$nullableCheckbox->Field(),
+			$this->getIsNullLabel()
+		);
 	}
 
 	/**
 	 * Value is sometimes an array, and sometimes a single value, so we need to handle both cases
+	 *
+	 * @param mixed $value
+	 * @param null|array $data
+	 *
+	 * @return $this
 	 */
 	public function setValue($value, $data = null) {
-		if ( is_array($data) && array_key_exists($this->getIsNullId(), $data) && $data[$this->getIsNullId()] ) {
+		$id = $this->getIsNullId();
+
+		if(is_array($data) && array_key_exists($id, $data) && $data[$id]) {
 			$value = null;
 		}
+
 		$this->valueField->setValue($value);
+
 		parent::setValue($value);
 
 		return $this;
 	}
 
 	/**
-	 * (non-PHPdoc)
+	 * @param string $name
-	 * @see forms/FormField#setName($name)
+	 *
+	 * @return $this
 	 */
 	public function setName($name) {
-		// We need to pass through the name change to the underlying value field.
 		$this->valueField->setName($name);
+
 		parent::setName($name);
 
 		return $this;
 	}
 
 	/**
-	 * (non-PHPdoc)
+	 * @return string
-	 * @see framework/forms/FormField#debug()
 	 */
 	public function debug() {
-		$result = "$this->class ($this->name: $this->title : <font style='color:red;'>$this->message</font>) = ";
+		$result = sprintf(
-		$result .= (is_null($this->value)) ? "<<null>>" : $this->value;
+			'%s (%s: $s : <span style="color: red">%s</span>) = ',
-		return result;
+			$this->class,
+			$this->name,
+			$this->title,
+			$this->message
+		);
+
+		if($this->value === null) {
+			$result .= "<<null>>";
+		} else {
+			$result .= (string) $this->value;
+		}
+
+		return $result;
 	}
 
 }

forms/NumericField.php

@@ -9,56 +9,75 @@
  * @subpackage fields-formattedinput
  */
 class NumericField extends TextField {
-
 	/**
-	 * Override locale for this field
+	 * Override locale for this field.
 	 *
 	 * @var string
 	 */
 	protected $locale = null;
 
+	/**
+	 * @param mixed $value
+	 * @param array $data
+	 *
+	 * @return $this
+	 *
+	 * @throws Zend_Locale_Exception
+	 */
 	public function setValue($value, $data = array()) {
 		require_once "Zend/Locale/Format.php";
 
 		// If passing in a non-string number, or a value
-		// directly from a dataobject then localise this number
+		// directly from a DataObject then localise this number
-		if ((is_numeric($value) && !is_string($value)) || 
+
-			($value && $data instanceof DataObject)
+		if(is_int($value) || is_float($value) || $data instanceof DataObject) {
-		){
 			$locale = new Zend_Locale($this->getLocale());
-			$this->value = Zend_Locale_Format::toNumber($value, array('locale' => $locale));
+
+			$this->value = Zend_Locale_Format::toNumber(
+				$value,
+				array('locale' => $locale)
+			);
 		} else {
-			// If an invalid number, store it anyway, but validate() will fail
 			$this->value = $this->clean($value);
 		}
+
 		return $this;
 	}
 
 	/**
-	 * In some cases and locales, validation expects non-breaking spaces
+	 * In some cases and locales, validation expects non-breaking spaces.
+	 *
+	 * Returns the value, with all spaces replaced with non-breaking spaces.
 	 *
 	 * @param string $input
-	 * @return string The input value, with all spaces replaced with non-breaking spaces
+	 *
+	 * @return string
 	 */
 	protected function clean($input) {
-		$nbsp = html_entity_decode(' ', null, 'UTF-8');
+		$replacement = html_entity_decode(' ', null, 'UTF-8');
-		return str_replace(' ', $nbsp, trim($input));
+
+		return str_replace(' ', $replacement, trim($input));
 	}
 
 	/**
-	 * Determine if the current value is a valid number in the current locale
+	 * Determine if the current value is a valid number in the current locale.
 	 *
 	 * @return bool
 	 */
 	protected function isNumeric() {
 		require_once "Zend/Locale/Format.php";
+
 		$locale = new Zend_Locale($this->getLocale());
+
 		return Zend_Locale_Format::isNumber(
 			$this->clean($this->value),
 			array('locale' => $locale)
 		);
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
 	public function Type() {
 		return 'numeric text';
 	}
@@ -81,74 +100,111 @@ class NumericField extends TextField {
 			return true;
 		}
 
-		if($this->isNumeric()) return true;
+		if($this->isNumeric()) {
+			return true;
+		}
 
 		$validator->validationError(
 			$this->name,
 			_t(
-				'NumericField.VALIDATION', "'{value}' is not a number, only numbers can be accepted for this field",
+				'NumericField.VALIDATION',
+				"'{value}' is not a number, only numbers can be accepted for this field",
 				array('value' => $this->value)
 			),
 			"validation"
 		);
+
 		return false;
 	}
 
 	/**
-	 * Extracts the number value from the localised string value
+	 * Extracts the number value from the localised string value.
 	 *
-	 * @return string number value
+	 * @return string
 	 */
 	public function dataValue() {
 		require_once "Zend/Locale/Format.php";
-		if(!$this->isNumeric()) return 0;
+
+		if(!$this->isNumeric()) {
+			return 0;
+		}
+
 		$locale = new Zend_Locale($this->getLocale());
+
 		$number = Zend_Locale_Format::getNumber(
 			$this->clean($this->value),
 			array('locale' => $locale)
 		);
+
 		return $number;
 	}
 
 	/**
-	 * Returns a readonly version of this field
+	 * Creates a read-only version of the field.
+	 *
+	 * @return NumericField_Readonly
 	 */
 	public function performReadonlyTransformation() {
-		$field = new NumericField_Readonly($this->name, $this->title, $this->value);
+		$field = new NumericField_Readonly(
+			$this->name,
+			$this->title,
+			$this->value
+		);
+
 		$field->setForm($this->form);
+
 		return $field;
 	}
 
 	/**
-	 * Gets the current locale this field is set to
+	 * Gets the current locale this field is set to.
 	 *
 	 * @return string
 	 */
 	public function getLocale() {
-		return $this->locale ?: i18n::get_locale();
+		if($this->locale) {
+			return $this->locale;
+		}
+
+		return i18n::get_locale();
 	}
 
 	/**
-	 * Override the locale for this field
+	 * Override the locale for this field.
 	 *
 	 * @param string $locale
+	 *
 	 * @return $this
 	 */
 	public function setLocale($locale) {
 		$this->locale = $locale;
+
 		return $this;
 	}
-
 }
 
+/**
+ * Readonly version of a numeric field.
+ *
+ * @package forms
+ * @subpackage fields-basic
+ */
 class NumericField_Readonly extends ReadonlyField {
-
+	/**
+	 * @return static
+	 */
 	public function performReadonlyTransformation() {
 		return clone $this;
 	}
 
+	/**
+	 * @return string
+	 */
 	public function Value() {
-		return Convert::raw2xml($this->value ? "$this->value" : "0");
+		if($this->value) {
+			return Convert::raw2xml((string) $this->value);
 		}
 
+		return '0';
+	}
 }

forms/gridfield/GridField.php

@@ -3,12 +3,11 @@
 /**
  * Displays a {@link SS_List} in a grid format.
  *
- * GridField is a field that takes an SS_List and displays it in an table with rows
+ * GridField is a field that takes an SS_List and displays it in an table with rows and columns.
- * and columns. It reminds of the old TableFields but works with SS_List types
+ * It reminds of the old TableFields but works with SS_List types and only loads the necessary
- * and only loads the necessary rows from the list.
+ * rows from the list.
  *
- * The minimum configuration is to pass in name and title of the field and a
+ * The minimum configuration is to pass in name and title of the field and a SS_List.
- * SS_List.
  *
  * <code>
  * $gridField = new GridField('ExampleGrid', 'Example grid', new DataList('Page'));
@@ -21,45 +20,44 @@
  * @property GridState_Data $State The gridstate of this object
  */
 class GridField extends FormField {
-
 	/**
-	 *
 	 * @var array
 	 */
 	private static $allowed_actions = array(
 		'index',
-		'gridFieldAlterAction'
+		'gridFieldAlterAction',
 	);
 
 	/**
-	 * The datasource
+	 * Data source.
 	 *
 	 * @var SS_List
 	 */
 	protected $list = null;
 
 	/**
-	 * The classname of the DataObject that the GridField will display. Defaults to the value of $this->list->dataClass
+	 * Class name of the DataObject that the GridField will display.
+	 *
+	 * Defaults to the value of $this->list->dataClass.
 	 *
 	 * @var string
 	 */
 	protected $modelClassName = '';
 
 	/**
-	 * the current state of the GridField
+	 * Current state of the GridField.
 	 *
 	 * @var GridState
 	 */
 	protected $state = null;
 
 	/**
-	 *
 	 * @var GridFieldConfig
 	 */
 	protected $config = null;
 
 	/**
-	 * The components list
+	 * Components list.
 	 *
 	 * @var array
 	 */
@@ -67,14 +65,15 @@ class GridField extends FormField {
 
 	/**
 	 * Internal dispatcher for column handlers.
-	 * Keys are column names and values are GridField_ColumnProvider objects
+	 *
+	 * Keys are column names and values are GridField_ColumnProvider objects.
 	 *
 	 * @var array
 	 */
 	protected $columnDispatch = null;
 
 	/**
-	 * Map of callbacks for custom data fields
+	 * Map of callbacks for custom data fields.
 	 *
 	 * @var array
 	 */
@@ -86,8 +85,6 @@ class GridField extends FormField {
 	protected $name = '';
 
 	/**
-	 * Creates a new GridField field
-	 *
 	 * @param string $name
 	 * @param string $title
 	 * @param SS_List $dataList
@@ -95,13 +92,18 @@ class GridField extends FormField {
 	 */
 	public function __construct($name, $title = null, SS_List $dataList = null, GridFieldConfig $config = null) {
 		parent::__construct($name, $title, null);
+
 		$this->name = $name;
 
 		if($dataList) {
 			$this->setList($dataList);
 		}
 
-		$this->setConfig($config ?: GridFieldConfig_Base::create());
+		if(!$config) {
+			$config = GridFieldConfig_Base::create();
+		}
+
+		$this->setConfig($config);
 
 		$this->config->addComponent(new GridState_Component());
 		$this->state = new GridState($this);
@@ -109,44 +111,58 @@ class GridField extends FormField {
 		$this->addExtraClass('ss-gridfield');
 	}
 
+	/**
+	 * @param SS_HTTPRequest $request
+	 *
+	 * @return string
+	 */
 	public function index($request) {
 		return $this->gridFieldAlterAction(array(), $this->getForm(), $request);
 	}
 
 	/**
 	 * Set the modelClass (data object) that this field will get it column headers from.
-	 * If no $displayFields has been set, the displayfields will be fetched from
+	 *
-	 * this modelclass $summary_fields
+	 * If no $displayFields has been set, the display fields will be $summary_fields.
+	 *
+	 * @see GridFieldDataColumns::getDisplayFields()
 	 *
 	 * @param string $modelClassName
 	 *
-	 * @see GridFieldDataColumns::getDisplayFields()
+	 * @return $this
 	 */
 	public function setModelClass($modelClassName) {
 		$this->modelClassName = $modelClassName;
+
 		return $this;
 	}
 
 	/**
 	 * Returns a data class that is a DataObject type that this GridField should look like.
 	 *
-	 * @throws Exception
 	 * @return string
+	 *
+	 * @throws LogicException
 	 */
 	public function getModelClass() {
-		if($this->modelClassName) return $this->modelClassName;
+		if($this->modelClassName) {
-		if($this->list && method_exists($this->list, 'dataClass')) {
+			return $this->modelClassName;
-			$class = $this->list->dataClass();
-			if($class) return $class;
 		}
 
-		throw new LogicException('GridField doesn\'t have a modelClassName,'
+		if($this->list && method_exists($this->list, 'dataClass')) {
-			. ' so it doesn\'t know the columns of this grid.');
+			$class = $this->list->dataClass();
+
+			if($class) {
+				return $class;
+			}
+		}
+
+		throw new LogicException(
+			'GridField doesn\'t have a modelClassName, so it doesn\'t know the columns of this grid.'
+		);
 	}
 
 	/**
-	 * Get the GridFieldConfig
-	 *
 	 * @return GridFieldConfig
 	 */
 	public function getConfig() {
@@ -156,61 +172,69 @@ class GridField extends FormField {
 	/**
 	 * @param GridFieldConfig $config
 	 *
-	 * @return GridField
+	 * @return $this
 	 */
 	public function setConfig(GridFieldConfig $config) {
 		$this->config = $config;
+
 		return $this;
 	}
 
+	/**
+	 * @return ArrayList
+	 */
 	public function getComponents() {
 		return $this->config->getComponents();
 	}
 
 	/**
-	 * Cast a arbitrary value with the help of a castingDefintion
+	 * Cast an arbitrary value with the help of a $castingDefinition.
-	 *
-	 * @param $value
-	 * @param $castingDefinition
 	 *
 	 * @todo refactor this into GridFieldComponent
+	 *
+	 * @param mixed $value
+	 * @param string|array $castingDefinition
+	 *
+	 * @return mixed
 	 */
 	public function getCastedValue($value, $castingDefinition) {
+		$castingParams = array();
+
 		if(is_array($castingDefinition)) {
 			$castingParams = $castingDefinition;
 			array_shift($castingParams);
 			$castingDefinition = array_shift($castingDefinition);
-		} else {
-			$castingParams = array();
 		}
 
 		if(strpos($castingDefinition, '->') === false) {
 			$castingFieldType = $castingDefinition;
 			$castingField = DBField::create_field($castingFieldType, $value);
-			$value = call_user_func_array(array($castingField, 'XML'), $castingParams);
+
-		} else {
+			return call_user_func_array(array($castingField, 'XML'), $castingParams);
-			$fieldTypeParts = explode('->', $castingDefinition);
-			$castingFieldType = $fieldTypeParts[0];
-			$castingMethod = $fieldTypeParts[1];
-			$castingField = DBField::create_field($castingFieldType, $value);
-			$value = call_user_func_array(array($castingField, $castingMethod), $castingParams);
 		}
 
-		return $value;
+		list($castingFieldType, $castingMethod) = explode('->', $castingDefinition);
+
+		$castingField = DBField::create_field($castingFieldType, $value);
+
+		return call_user_func_array(array($castingField, $castingMethod), $castingParams);
 	}
 
 	/**
-	 * Set the datasource
+	 * Set the data source.
 	 *
 	 * @param SS_List $list
+	 *
+	 * @return $this
 	 */
 	public function setList(SS_List $list) {
 		$this->list = $list;
+
 		return $this;
 	}
 
 	/**
-	 * Get the datasource
+	 * Get the data source.
 	 *
 	 * @return SS_List
 	 */
@@ -219,26 +243,28 @@ class GridField extends FormField {
 	}
 
 	/**
-	 * Get the datasource after applying the {@link GridField_DataManipulator}s to it.
+	 * Get the data source after applying every {@link GridField_DataManipulator} to it.
 	 *
 	 * @return SS_List
 	 */
 	public function getManipulatedList() {
 		$list = $this->getList();
+
 		foreach($this->getComponents() as $item) {
 			if($item instanceof GridField_DataManipulator) {
 				$list = $item->getManipulatedData($this, $list);
 			}
 		}
+
 		return $list;
 	}
 
 	/**
-	 * Get the current GridState_Data or the GridState
+	 * Get the current GridState_Data or the GridState.
 	 *
-	 * @param bool $getData - flag for returning the GridState_Data or the GridState
+	 * @param bool $getData
 	 *
-	 * @return GridState_data|GridState
+	 * @return GridState_Data|GridState
 	 */
 	public function getState($getData = true) {
 		if($getData) {
@@ -249,7 +275,9 @@ class GridField extends FormField {
 	}
 
 	/**
-	 * Returns the whole gridfield rendered with all the attached components
+	 * Returns the whole gridfield rendered with all the attached components.
+	 *
+	 * @param array $properties
 	 *
 	 * @return string
 	 */
@@ -265,88 +293,115 @@ class GridField extends FormField {
 		Requirements::javascript(THIRDPARTY_DIR . '/jquery-entwine/dist/jquery.entwine-dist.js');
 		Requirements::javascript(FRAMEWORK_DIR . '/javascript/GridField.js');
 
-		// Get columns
 		$columns = $this->getColumns();
 
-		// Get data
 		$list = $this->getManipulatedList();
 
-		// Render headers, footers, etc
 		$content = array(
-			"before" => "",
+			'before' => '',
-			"after" => "",
+			'after' => '',
-			"header" => "",
+			'header' => '',
-			"footer" => "",
+			'footer' => '',
 		);
 
 		foreach($this->getComponents() as $item) {
 			if($item instanceof GridField_HTMLProvider) {
 				$fragments = $item->getHTMLFragments($this);
-				if($fragments) foreach($fragments as $k => $v) {
+
-					$k = strtolower($k);
+				if($fragments) {
-					if(!isset($content[$k])) $content[$k] = "";
+					foreach($fragments as $fragmentKey => $fragmentValue) {
-					$content[$k] .= $v . "\n";
+						$fragmentKey = strtolower($fragmentKey);
+
+						if(!isset($content[$fragmentKey])) {
+							$content[$fragmentKey] = '';
+						}
+
+						$content[$fragmentKey] .= $fragmentValue . "\n";
+					}
 				}
 			}
 		}
 
-		foreach($content as $k => $v) {
+		foreach($content as $contentKey => $contentValue) {
-			$content[$k] = trim($v);
+			$content[$contentKey] = trim($contentValue);
 		}
 
-		// Replace custom fragments and check which fragments are defined
+		// Replace custom fragments and check which fragments are defined. Circular dependencies
-		// Nested dependencies are handled by deferring the rendering of any content item that
+		// are detected by disallowing any item to be deferred more than 5 times.
-		// Circular dependencies are detected by disallowing any item to be deferred more than 5 times
+
-		// It's a fairly crude algorithm but it works
+		$fragmentDefined = array(
+			'header' => true,
+			'footer' => true,
+			'before' => true,
+			'after' => true,
+		);
 
-		$fragmentDefined = array('header' => true, 'footer' => true, 'before' => true, 'after' => true);
 		reset($content);
-		while(list($k, $v) = each($content)) {
+
-			if(preg_match_all('/\$DefineFragment\(([a-z0-9\-_]+)\)/i', $v, $matches)) {
+		while(list($contentKey, $contentValue) = each($content)) {
+			if(preg_match_all('/\$DefineFragment\(([a-z0-9\-_]+)\)/i', $contentValue, $matches)) {
 				foreach($matches[1] as $match) {
 					$fragmentName = strtolower($match);
 					$fragmentDefined[$fragmentName] = true;
-					$fragment = isset($content[$fragmentName]) ? $content[$fragmentName] : "";
 
-					// If the fragment still has a fragment definition in it, when we should defer this item until
+					$fragment = '';
-					// later.
+
+					if(isset($content[$fragmentName])) {
+						$fragment = $content[$fragmentName];
+					}
+
+					// If the fragment still has a fragment definition in it, when we should defer
+					// this item until later.
+
 					if(preg_match('/\$DefineFragment\(([a-z0-9\-_]+)\)/i', $fragment, $matches)) {
-						// If we've already deferred this fragment, then we have a circular dependency
+						if(isset($fragmentDeferred[$contentKey]) && $fragmentDeferred[$contentKey] > 5) {
-						if(isset($fragmentDeferred[$k]) && $fragmentDeferred[$k] > 5) {
+							throw new LogicException(sprintf(
-							throw new LogicException("GridField HTML fragment '$fragmentName' and '$matches[1]' " .
+								'GridField HTML fragment "%s" and "%s" appear to have a circular dependency.',
-								"appear to have a circular dependency.");
+								$fragmentName,
+								$matches[1]
+							));
 						}
 
-						// Otherwise we can push to the end of the content array
+						unset($content[$contentKey]);
-						unset($content[$k]);
+
-						$content[$k] = $v;
+						$content[$contentKey] = $contentValue;
-						if(!isset($fragmentDeferred[$k])) {
+
-							$fragmentDeferred[$k] = 1;
+						if(!isset($fragmentDeferred[$contentKey])) {
-						} else {
+							$fragmentDeferred[$contentKey] = 0;
-							$fragmentDeferred[$k]++;
 						}
+
+						$fragmentDeferred[$contentKey]++;
+
 						break;
 					} else {
-						$content[$k] = preg_replace('/\$DefineFragment\(' . $fragmentName . '\)/i', $fragment,
+						$content[$contentKey] = preg_replace(
-							$content[$k]);
+							sprintf('/\$DefineFragment\(%s\)/i', $fragmentName),
+							$fragment,
+							$content[$contentKey]
+						);
 					}
 				}
 			}
 		}
 
-		// Check for any undefined fragments, and if so throw an exception
+		// Check for any undefined fragments, and if so throw an exception.
-		// While we're at it, trim whitespace off the elements
+		// While we're at it, trim whitespace off the elements.
-		foreach($content as $k => $v) {
+
-			if(empty($fragmentDefined[$k])) {
+		foreach($content as $contentKey => $contentValue) {
-				throw new LogicException("GridField HTML fragment '$k' was given content,"
+			if(empty($fragmentDefined[$contentKey])) {
-					. " but not defined.  Perhaps there is a supporting GridField component you need to add?");
+				throw new LogicException(sprintf(
+					'GridField HTML fragment "%s" was given content, but not defined. Perhaps there is a supporting GridField component you need to add?',
+					$contentKey
+				));
 			}
 		}
 
 		$total = count($list);
+
 		if($total > 0) {
 			$rows = array();
-			foreach($list as $idx => $record) {
+
+			foreach($list as $index => $record) {
 				if($record->hasMethod('canView') && !$record->canView()) {
 					continue;
 				}
@@ -356,58 +411,80 @@ class GridField extends FormField {
 				foreach($this->getColumns() as $column) {
 					$colContent = $this->getColumnContent($record, $column);
 
-					// A return value of null means this columns should be skipped altogether.
+					// Null means this columns should be skipped altogether.
+
 					if($colContent === null) {
 						continue;
 					}
 
 					$colAttributes = $this->getColumnAttributes($record, $column);
 
-					$rowContent .= $this->newCell($total, $idx, $record, $colAttributes, $colContent);
+					$rowContent .= $this->newCell(
+						$total,
+						$index,
+						$record,
+						$colAttributes,
+						$colContent
+					);
 				}
 
-				$rowAttributes = $this->getRowAttributes($total, $idx, $record);
+				$rowAttributes = $this->getRowAttributes($total, $index, $record);
 
-				$rows[] = $this->newRow($total, $idx, $record, $rowAttributes, $rowContent);
+				$rows[] = $this->newRow($total, $index, $record, $rowAttributes, $rowContent);
 			}
 			$content['body'] = implode("\n", $rows);
 		}
 
-		// Display a message when the grid field is empty
+		// Display a message when the grid field is empty.
-		if(!(isset($content['body']) && $content['body'])) {
+
-			$content['body'] = FormField::create_tag(
+		if(empty($content['body'])) {
-				'tr',
+			$cell = FormField::create_tag(
-				array("class" => 'ss-gridfield-item ss-gridfield-no-items'),
-				FormField::create_tag(
 				'td',
-					array('colspan' => count($columns)),
+				array(
+					'colspan' => count($columns),
+				),
 				_t('GridField.NoItemsFound', 'No items found')
-				)
 			);
+
+			$row = FormField::create_tag(
+				'tr',
+				array(
+					'class' => 'ss-gridfield-item ss-gridfield-no-items',
+				),
+				$cell
+			);
+
+			$content['body'] = $row;
 		}
 
-		// Turn into the relevant parts of a table
+		$header = $this->getOptionalTableHeader($content);
-		$head = $content['header']
+		$body = $this->getOptionalTableBody($content);
-			? FormField::create_tag('thead', array(), $content['header'])
+		$footer = $this->getOptionalTableFooter($content);
-			: '';
-		$body = $content['body']
-			? FormField::create_tag('tbody', array('class' => 'ss-gridfield-items'), $content['body'])
-			: '';
-		$foot = $content['footer']
-			? FormField::create_tag('tfoot', array(), $content['footer'])
-			: '';
 
 		$this->addExtraClass('ss-gridfield field');
-		$attrs = array_diff_key(
+
+		$fieldsetAttributes = array_diff_key(
 			$this->getAttributes(),
-			array('value' => false, 'type' => false, 'name' => false)
+			array(
+				'value' => false,
+				'type' => false,
+				'name' => false,
+			)
 		);
-		$attrs['data-name'] = $this->getName();
+
-		$tableAttrs = array(
+		$fieldsetAttributes['data-name'] = $this->getName();
-			'id' => isset($this->id) ? $this->id : null,
+
+		$tableId = null;
+
+		if($this->id) {
+			$tableId = $this->id;
+		}
+
+		$tableAttributes = array(
+			'id' => $tableId,
 			'class' => 'ss-gridfield-table',
 			'cellpadding' => '0',
-			'cellspacing' => '0'
+			'cellspacing' => '0',
 		);
 
 		if($this->getDescription()) {
@@ -418,11 +495,16 @@ class GridField extends FormField {
 			);
 		}
 
-		return
+		$table = FormField::create_tag(
-			FormField::create_tag('fieldset', $attrs,
+			'table',
-				$content['before'] .
+			$tableAttributes,
-				FormField::create_tag('table', $tableAttrs, $head . "\n" . $foot . "\n" . $body) .
+			$header . "\n" . $footer . "\n" . $body
-				$content['after']
+		);
+
+		return FormField::create_tag(
+			'fieldset',
+			$fieldsetAttributes,
+			$content['before'] . $table . $content['after']
 		);
 	}
 
@@ -495,27 +577,44 @@ class GridField extends FormField {
 			$classes[] = 'last';
 		}
 
-		$classes[] = ($index % 2) ? 'even' : 'odd';
+		if($index % 2) {
+			$classes[] = 'even';
+		} else {
+			$classes[] = 'odd';
+		}
 
 		return $classes;
 	}
 
+	/**
+	 * @param array $properties
+	 *
+	 * @return string
+	 */
 	public function Field($properties = array()) {
 		return $this->FieldHolder($properties);
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
 	public function getAttributes() {
-		return array_merge(parent::getAttributes(), array('data-url' => $this->Link()));
+		return array_merge(
+			parent::getAttributes(),
+			array(
+				'data-url' => $this->Link(),
+			)
+		);
 	}
 
 	/**
-	 * Get the columns of this GridField, they are provided by attached GridField_ColumnProvider
+	 * Get the columns of this GridField, they are provided by attached GridField_ColumnProvider.
 	 *
 	 * @return array
 	 */
 	public function getColumns() {
-		// Get column list
 		$columns = array();
+
 		foreach($this->getComponents() as $item) {
 			if($item instanceof GridField_ColumnProvider) {
 				$item->augmentColumns($this, $columns);
@@ -526,28 +625,36 @@ class GridField extends FormField {
 	}
 
 	/**
-	 * Get the value from a column
+	 * Get the value from a column.
 	 *
 	 * @param DataObject $record
 	 * @param string $column
 	 *
 	 * @return string
+	 *
 	 * @throws InvalidArgumentException
 	 */
 	public function getColumnContent($record, $column) {
-		// Build the column dispatch
 		if(!$this->columnDispatch) {
 			$this->buildColumnDispatch();
 		}
 
 		if(!empty($this->columnDispatch[$column])) {
-			$content = "";
+			$content = '';
+
 			foreach($this->columnDispatch[$column] as $handler) {
+				/**
+				 * @var GridField_ColumnProvider $handler
+				 */
 				$content .= $handler->getColumnContent($this, $record, $column);
 			}
+
 			return $content;
 		} else {
-			throw new InvalidArgumentException("Bad column '$column'");
+			throw new InvalidArgumentException(sprintf(
+				'Bad column "%s"',
+				$column
+			));
 		}
 	}
 
@@ -567,111 +674,139 @@ class GridField extends FormField {
 
 	/**
 	 * Get the value of a named field  on the given record.
-	 * Use of this method ensures that any special rules around the data for this gridfield are followed.
+	 *
+	 * Use of this method ensures that any special rules around the data for this gridfield are
+	 * followed.
+	 *
+	 * @param DataObject $record
+	 * @param string $fieldName
+	 *
+	 * @return mixed
 	 */
 	public function getDataFieldValue($record, $fieldName) {
-		// Custom callbacks
 		if(isset($this->customDataFields[$fieldName])) {
 			$callback = $this->customDataFields[$fieldName];
+
 			return $callback($record);
 		}
 
-		// Default implementation
 		if($record->hasMethod('relField')) {
 			return $record->relField($fieldName);
-		} elseif($record->hasMethod($fieldName)) {
-			return $record->$fieldName();
-		} else {
-			return $record->$fieldName;
 		}
+
+		if($record->hasMethod($fieldName)) {
+			return $record->$fieldName();
+		}
+
+		return $record->$fieldName;
 	}
 
 	/**
-	 * Get extra columns attributes used as HTML attributes
+	 * Get extra columns attributes used as HTML attributes.
 	 *
 	 * @param DataObject $record
 	 * @param string $column
 	 *
 	 * @return array
+	 *
 	 * @throws LogicException
 	 * @throws InvalidArgumentException
 	 */
 	public function getColumnAttributes($record, $column) {
-		// Build the column dispatch
 		if(!$this->columnDispatch) {
 			$this->buildColumnDispatch();
 		}
 
 		if(!empty($this->columnDispatch[$column])) {
-			$attrs = array();
+			$attributes = array();
 
 			foreach($this->columnDispatch[$column] as $handler) {
-				$column_attrs = $handler->getColumnAttributes($this, $record, $column);
+				/**
+				 * @var GridField_ColumnProvider $handler
+				 */
+				$columnAttributes = $handler->getColumnAttributes($this, $record, $column);
 
-				if(is_array($column_attrs)) {
+				if(is_array($columnAttributes)) {
-					$attrs = array_merge($attrs, $column_attrs);
+					$attributes = array_merge($attributes, $columnAttributes);
-				} elseif($column_attrs) {
+					continue;
-					$methodSignature = get_class($handler) . "::getColumnAttributes()";
-					throw new LogicException("Non-array response from $methodSignature.");
-				}
 				}
 
-			return $attrs;
+				throw new LogicException(sprintf(
-		} else {
+					'Non-array response from %s::getColumnAttributes().',
-			throw new InvalidArgumentException("Bad column '$column'");
+					get_class($handler)
+				));
 			}
+
+			return $attributes;
+		}
+
+		throw new InvalidArgumentException(sprintf(
+			'Bad column "%s"',
+			$column
+		));
 	}
 
 	/**
-	 * Get metadata for a column, example array('Title'=>'Email address')
+	 * Get metadata for a column.
+	 *
+	 * @example "array('Title'=>'Email address')"
 	 *
 	 * @param string $column
 	 *
 	 * @return array
+	 *
 	 * @throws LogicException
 	 * @throws InvalidArgumentException
 	 */
 	public function getColumnMetadata($column) {
-		// Build the column dispatch
 		if(!$this->columnDispatch) {
 			$this->buildColumnDispatch();
 		}
 
 		if(!empty($this->columnDispatch[$column])) {
-			$metadata = array();
+			$metaData = array();
 
 			foreach($this->columnDispatch[$column] as $handler) {
-				$column_metadata = $handler->getColumnMetadata($this, $column);
+				/**
+				 * @var GridField_ColumnProvider $handler
+				 */
+				$columnMetaData = $handler->getColumnMetadata($this, $column);
 
-				if(is_array($column_metadata)) {
+				if(is_array($columnMetaData)) {
-					$metadata = array_merge($metadata, $column_metadata);
+					$metaData = array_merge($metaData, $columnMetaData);
-				} else {
+					continue;
-					$methodSignature = get_class($handler) . "::getColumnMetadata()";
-					throw new LogicException("Non-array response from $methodSignature.");
 				}
 
+				throw new LogicException(sprintf(
+					'Non-array response from %s::getColumnMetadata().',
+					get_class($handler)
+				));
 			}
 
-			return $metadata;
+			return $metaData;
 		}
-		throw new InvalidArgumentException("Bad column '$column'");
+
+		throw new InvalidArgumentException(sprintf(
+			'Bad column "%s"',
+			$column
+		));
 	}
 
 	/**
-	 * Return how many columns the grid will have
+	 * Return how many columns the grid will have.
 	 *
 	 * @return int
 	 */
 	public function getColumnCount() {
-		// Build the column dispatch
+		if(!$this->columnDispatch) {
-		if(!$this->columnDispatch) $this->buildColumnDispatch();
+			$this->buildColumnDispatch();
+		}
+
 		return count($this->columnDispatch);
 	}
 
 	/**
-	 * Build an columnDispatch that maps a GridField_ColumnProvider to a column
+	 * Build an columnDispatch that maps a GridField_ColumnProvider to a column for reference later.
-	 * for reference later
-	 *
 	 */
 	protected function buildColumnDispatch() {
 		$this->columnDispatch = array();
@@ -691,108 +826,137 @@ class GridField extends FormField {
 	 * This is the action that gets executed when a GridField_AlterAction gets clicked.
 	 *
 	 * @param array $data
+	 * @param Form $form
+	 * @param SS_HTTPRequest $request
 	 *
 	 * @return string
 	 */
 	public function gridFieldAlterAction($data, $form, SS_HTTPRequest $request) {
-		$html = '';
 		$data = $request->requestVars();
 		$name = $this->getName();
-		$fieldData = isset($data[$name]) ? $data[$name] : null;
 
-		// Update state from client
+		$fieldData = null;
+
+		if(isset($data[$name])) {
+			$fieldData = $data[$name];
+		}
+
 		$state = $this->getState(false);
 
 		if(isset($fieldData['GridState'])) {
 			$state->setValue($fieldData['GridState']);
 		}
 
-		// Try to execute alter action
+		foreach($data as $dataKey => $dataValue) {
-		foreach($data as $k => $v) {
+			if(preg_match('/^action_gridFieldAlterAction\?StateID=(.*)/', $dataKey, $matches)) {
-			if(preg_match('/^action_gridFieldAlterAction\?StateID=(.*)/', $k, $matches)) {
+				$stateChange = Session::get($matches[1]);
-				$id = $matches[1];
-				$stateChange = Session::get($id);
-
 				$actionName = $stateChange['actionName'];
 
-				$args = isset($stateChange['args']) ? $stateChange['args'] : array();
+				$arguments = array();
-				$html = $this->handleAlterAction($actionName, $args, $data);
+
-				// A field can optionally return its own HTML
+				if(isset($stateChange['args'])) {
-				if($html) return $html;
+					$arguments = $stateChange['args'];
+				};
+
+				$html = $this->handleAlterAction($actionName, $arguments, $data);
+
+				if($html) {
+					return $html;
+				}
 			}
 		}
 
-		switch($request->getHeader('X-Pjax')) {
+		if($request->getHeader('X-Pjax') === 'CurrentField') {
-			case 'CurrentField':
 			return $this->FieldHolder();
-				break;
-
-			case 'CurrentForm':
-				return $form->forTemplate();
-				break;
-
-			default:
-				return $form->forTemplate();
-				break;
 		}
+
+		return $form->forTemplate();
 	}
 
 	/**
-	 * Pass an action on the first GridField_ActionProvider that matches the $actionName
+	 * Pass an action on the first GridField_ActionProvider that matches the $actionName.
 	 *
 	 * @param string $actionName
-	 * @param mixed $args
+	 * @param mixed $arguments
-	 * @param array $data - send data from a form
+	 * @param array $data
 	 *
 	 * @return mixed
+	 *
 	 * @throws InvalidArgumentException
 	 */
-	public function handleAlterAction($actionName, $args, $data) {
+	public function handleAlterAction($actionName, $arguments, $data) {
 		$actionName = strtolower($actionName);
+
 		foreach($this->getComponents() as $component) {
-			if(!($component instanceof GridField_ActionProvider)) {
+			if($component instanceof GridField_ActionProvider) {
-				continue;
+				$actions = array_map('strtolower', (array) $component->getActions($this));
+
+				if(in_array($actionName, $actions)) {
+					return $component->handleAction($this, $actionName, $arguments, $data);
+				}
+			}
 		}
 
-			if(in_array($actionName, array_map('strtolower', (array) $component->getActions($this)))) {
+		throw new InvalidArgumentException(sprintf(
-				return $component->handleAction($this, $actionName, $args, $data);
+			'Can\'t handle action "%s"',
-			}
+			$actionName
-		}
+		));
-		throw new InvalidArgumentException("Can't handle action '$actionName'");
 	}
 
 	/**
-	 * Custom request handler that will check component handlers before proceeding to the default implementation.
+	 * Custom request handler that will check component handlers before proceeding to the default
+	 * implementation.
 	 *
-	 * @todo There is too much code copied from RequestHandler here.
+	 * @todo copy less code from RequestHandler.
+	 *
+	 * @param SS_HTTPRequest $request
+	 * @param DataModel $model
+	 *
+	 * @return array|RequestHandler|SS_HTTPResponse|string|void
+	 *
+	 * @throws SS_HTTPResponse_Exception
 	 */
 	public function handleRequest(SS_HTTPRequest $request, DataModel $model) {
 		if($this->brokenOnConstruct) {
-			user_error("parent::__construct() needs to be called on {$handlerClass}::__construct()", E_USER_WARNING);
+			user_error(
+				sprintf(
+					"parent::__construct() needs to be called on %s::__construct()",
+					__CLASS__
+				),
+				E_USER_WARNING
+			);
 		}
 
 		$this->setRequest($request);
 		$this->setDataModel($model);
 
 		$fieldData = $this->getRequest()->requestVar($this->getName());
-		if($fieldData && isset($fieldData['GridState'])) $this->getState(false)->setValue($fieldData['GridState']);
 
-		foreach($this->getComponents() as $component) {
+		if($fieldData && isset($fieldData['GridState'])) {
-			if(!($component instanceof GridField_URLHandler)) {
+			$this->getState(false)->setValue($fieldData['GridState']);
-				continue;
 		}
 
-			$urlHandlers = $component->getURLHandlers($this);
+		foreach($this->getComponents() as $component) {
-
+			if($component instanceof GridField_URLHandler && $urlHandlers = $component->getURLHandlers($this)) {
-			if($urlHandlers) foreach($urlHandlers as $rule => $action) {
+				foreach($urlHandlers as $rule => $action) {
 					if($params = $request->match($rule, true)) {
-					// Actions can reference URL parameters, eg, '$Action/$ID/$OtherID' => '$Action',
+						// Actions can reference URL parameters.
-					if($action[0] == '$') $action = $params[substr($action, 1)];
+						// e.g. '$Action/$ID/$OtherID' → '$Action'
+
+						if($action[0] == '$') {
+							$action = $params[substr($action, 1)];
+						}
+
 						if(!method_exists($component, 'checkAccessAction') || $component->checkAccessAction($action)) {
 							if(!$action) {
 								$action = "index";
-						} else if(!is_string($action)) {
+							}
-							throw new LogicException("Non-string method name: " . var_export($action, true));
+
+							if(!is_string($action)) {
+								throw new LogicException(sprintf(
+									'Non-string method name: %s',
+									var_export($action, true)
+								));
 							}
 
 							try {
@@ -805,26 +969,29 @@ class GridField extends FormField {
 								return $result;
 							}
 
-						if($this !== $result && !$request->isEmptyPattern($rule) && is_object($result)
+							if($this !== $result && !$request->isEmptyPattern($rule) && is_object($result) && $result instanceof RequestHandler) {
-							&& $result instanceof RequestHandler
-						) {
-
 								$returnValue = $result->handleRequest($request, $model);
 
 								if(is_array($returnValue)) {
-								throw new LogicException("GridField_URLHandler handlers can't return arrays");
+									throw new LogicException(
+										'GridField_URLHandler handlers can\'t return arrays'
+									);
 								}
 
 								return $returnValue;
+							}
 
-							// If we return some other data, and all the URL is parsed, then return that
+							if($request->allParsed()) {
-						} else if($request->allParsed()) {
 								return $result;
+							}
 
-							// But if we have more content on the URL and we don't know what to do with it, return an error
+							return $this->httpError(
-						} else {
+								404,
-							return $this->httpError(404,
+								sprintf(
-								"I can't handle sub-URLs of a " . get_class($result) . " object.");
+									'I can\'t handle sub-URLs of a %s object.',
+									get_class($result)
+								)
+							);
 						}
 					}
 				}
@@ -834,6 +1001,9 @@ class GridField extends FormField {
 		return parent::handleRequest($request, $model);
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
 	public function saveInto(DataObjectInterface $record) {
 		foreach($this->getComponents() as $component) {
 			if($component instanceof GridField_SaveHandler) {
@@ -842,18 +1012,61 @@ class GridField extends FormField {
 		}
 	}
 
+	/**
+	 * @param array $content
+	 *
+	 * @return string
+	 */
+	protected function getOptionalTableHeader(array $content) {
+		if($content['header']) {
+			return FormField::create_tag(
+				'thead', array(), $content['header']
+			);
 		}
 
+		return '';
+	}
 
 	/**
- * This class is the base class when you want to have an action that alters
+	 * @param array $content
- * the state of the {@link GridField}, rendered as a button element.
+	 *
+	 * @return string
+	 */
+	protected function getOptionalTableBody(array $content) {
+		if($content['body']) {
+			return FormField::create_tag(
+				'tbody', array('class' => 'ss-gridfield-items'), $content['body']
+			);
+		}
+
+		return '';
+	}
+
+	/**
+	 * @param $content
+	 *
+	 * @return string
+	 */
+	protected function getOptionalTableFooter($content) {
+		if($content['footer']) {
+			return FormField::create_tag(
+				'tfoot', array(), $content['footer']
+			);
+		}
+
+		return '';
+	}
+
+}
+
+/**
+ * This class is the base class when you want to have an action that alters the state of the
+ * {@link GridField}, rendered as a button element.
  *
  * @package forms
  * @subpackage fields-gridfield
  */
 class GridField_FormAction extends FormAction {
-
 	/**
 	 * @var GridField
 	 */
@@ -882,7 +1095,7 @@ class GridField_FormAction extends FormAction {
 	/**
 	 * @param GridField $gridField
 	 * @param string $name
-	 * @param string $label
+	 * @param string $title
 	 * @param string $actionName
 	 * @param array $args
 	 */
@@ -895,19 +1108,20 @@ class GridField_FormAction extends FormAction {
 	}
 
 	/**
-	 * urlencode encodes less characters in percent form than we need - we
+	 * Encode all non-word characters.
-	 * need everything that isn't a \w.
 	 *
-	 * @param string $val
+	 * @param string $value
+	 *
+	 * @return string
 	 */
-	public function nameEncode($val) {
+	public function nameEncode($value) {
-		return preg_replace_callback('/[^\w]/', array($this, '_nameEncode'), $val);
+		return (string) preg_replace_callback('/[^\w]/', array($this, '_nameEncode'), $value);
 	}
 
 	/**
-	 * The callback for nameEncode
+	 * @param array $match
 	 *
-	 * @param string $val
+	 * @return string
 	 */
 	public function _nameEncode($match) {
 		return '%' . dechex(ord($match[0]));
@@ -941,9 +1155,7 @@ class GridField_FormAction extends FormAction {
 	}
 
 	/**
-	 * Calculate the name of the gridfield relative to the Form
+	 * Calculate the name of the gridfield relative to the form.
-	 *
-	 * @param GridField $base
 	 *
 	 * @return string
 	 */

tests/control/DirectorTest.php

@@ -18,8 +18,6 @@ class DirectorTest extends SapphireTest {
 	public function setUp() {
 		parent::setUp();
 
-		// Required for testRequestFilterInDirectorTest
-		Injector::nest();
 
 		// Hold the original request URI once so it doesn't get overwritten
 		if(!self::$originalRequestURI) {
@@ -53,9 +51,6 @@ class DirectorTest extends SapphireTest {
 	public function tearDown() {
 		// TODO Remove director rule, currently API doesnt allow this
 
-		// Remove base URL override (setting to false reverts to default behaviour)
-		Config::inst()->update('Director', 'alternate_base_url', false);
-
 		$_GET = $this->originalGet;
 		$_SESSION = $this->originalSession;
 
@@ -68,7 +63,6 @@ class DirectorTest extends SapphireTest {
 			}
 		}
 
-		Injector::unnest();
 
 		parent::tearDown();
 	}

tests/core/ConfigTest.php

@@ -189,8 +189,7 @@ class ConfigTest extends SapphireTest {
 		// But it won't affect subclasses - this is *uninherited* static
 		$this->assertNotContains('test_2b',
 			Config::inst()->get('ConfigStaticTest_Third', 'first', Config::UNINHERITED));
-		$this->assertNotContains('test_2b',
+		$this->assertNull(Config::inst()->get('ConfigStaticTest_Fourth', 'first', Config::UNINHERITED));
-			Config::inst()->get('ConfigStaticTest_Fourth', 'first', Config::UNINHERITED));
 
 		// Subclasses that don't have the static explicitly defined should allow definition, also
 		// This also checks that set can be called after the first uninherited get()

tests/core/manifest/ConfigManifestTest.php

@@ -391,7 +391,7 @@ class ConfigManifestTest extends SapphireTest {
 
 	public function testEnvironmentRules() {
 		foreach (array('dev', 'test', 'live') as $env) {
-			Config::inst()->nest();
+			Config::nest();
 
 			Config::inst()->update('Director', 'environment_type', $env);
 			$config = $this->getConfigFixtureValue('Environment');
@@ -403,13 +403,11 @@ class ConfigManifestTest extends SapphireTest {
 				);
 			}
 
-			Config::inst()->unnest();
+			Config::unnest();
 		}
 	}
 
 	public function testDynamicEnvironmentRules() {
-		Config::inst()->nest();
-
 		// First, make sure environment_type is live
 		Config::inst()->update('Director', 'environment_type', 'live');
 		$this->assertEquals('live', Config::inst()->get('Director', 'environment_type'));
@@ -423,8 +421,6 @@ class ConfigManifestTest extends SapphireTest {
 
 		// And that the dynamic rule was calculated correctly
 		$this->assertEquals('dev', Config::inst()->get('ConfigManifestTest', 'DynamicEnvironment'));
-
-		Config::inst()->unnest();
 	}
 
 	public function testMultipleRules() {

tests/dev/DevAdminControllerTest.php

@@ -11,7 +11,7 @@ class DevAdminControllerTest extends FunctionalTest {
 	public function setUp(){
 		parent::setUp();
 
-		Config::nest()->update('DevelopmentAdmin', 'registered_controllers', array(
+		Config::inst()->update('DevelopmentAdmin', 'registered_controllers', array(
 			'x1' => array(
 				'controller' => 'DevAdminControllerTest_Controller1',
 				'links' => array(
@@ -28,11 +28,6 @@ class DevAdminControllerTest extends FunctionalTest {
 		));
 	}
 
-	public function tearDown(){
-		parent::tearDown();
-		Config::unnest();
-	}
-	
 
 	public function testGoodRegisteredControllerOutput(){
 		// Check for the controller running from the registered url above

tests/forms/DatetimeFieldTest.php

@@ -214,6 +214,24 @@ class DatetimeFieldTest extends SapphireTest {
 		);
 	}
 
+	public function testGetName() {
+		$field = new DatetimeField('Datetime');
+		
+		$this->assertEquals('Datetime', $field->getName());
+		$this->assertEquals('Datetime[date]', $field->getDateField()->getName());
+		$this->assertEquals('Datetime[time]', $field->getTimeField()->getName());
+		$this->assertEquals('Datetime[timezone]', $field->getTimezoneField()->getName());
+	}
+
+	public function testSetName() {
+		$field = new DatetimeField('Datetime', 'Datetime');
+		$field->setName('CustomDatetime');
+		$this->assertEquals('CustomDatetime', $field->getName());
+		$this->assertEquals('CustomDatetime[date]', $field->getDateField()->getName());
+		$this->assertEquals('CustomDatetime[time]', $field->getTimeField()->getName());
+		$this->assertEquals('CustomDatetime[timezone]', $field->getTimezoneField()->getName());
+	}
+
 	protected function getMockForm() {
 		return new Form(
 			new Controller(),

tests/forms/TimeFieldTest.php

@@ -91,6 +91,14 @@ class TimeFieldTest extends SapphireTest {
 		$f = new TimeField('Time', 'Time');
 		$f->setValue('23:59:38');
 		$this->assertEquals($f->dataValue(), '23:59:38');
+
+		$f = new TimeField('Time', 'Time');
+		$f->setValue('12:00 am');
+		$this->assertEquals($f->dataValue(), '00:00:00');
+
+		$f = new TimeField('Time', 'Time');
+		$f->setValue('12:00:01 am');
+		$this->assertEquals($f->dataValue(), '00:00:01');
 	}
 
 	public function testOverrideWithNull() {

tests/model/DataObjectSchemaGenerationTest.php

@@ -46,7 +46,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		// Table will have been initially created by the $extraDataObjects setting
 
 		// Let's insert a new field here
-		Config::nest();
 		Config::inst()->update('DataObjectSchemaGenerationTest_DO', 'db', array(
 			'SecretField' => 'Varchar(100)'
 		));
@@ -59,9 +58,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 			$schema->cancelSchemaUpdate();
 			$test->assertTrue($needsUpdating);
 		});
-
-		// Restore db configuration
-		Config::unnest();
 	}
 
 	/**
@@ -84,7 +80,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		});
 
 		// Test with alternate index format, although these indexes are the same
-		Config::nest();
 		Config::inst()->remove('DataObjectSchemaGenerationTest_IndexDO', 'indexes');
 		Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'indexes',
 			Config::inst()->get('DataObjectSchemaGenerationTest_IndexDO', 'indexes_alt')
@@ -98,9 +93,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 			$schema->cancelSchemaUpdate();
 			$test->assertFalse($needsUpdating);
 		});
-
-		// Restore old index format
-		Config::unnest();
 	}
 
 	/**
@@ -114,7 +106,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		// Table will have been initially created by the $extraDataObjects setting
 
 		// Update the SearchFields index here
-		Config::nest();
 		Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'indexes', array(
 			'SearchFields' => array(
 				'value' => 'Title'
@@ -129,9 +120,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 			$schema->cancelSchemaUpdate();
 			$test->assertTrue($needsUpdating);
 		});
-
-		// Restore old indexes
-		Config::unnest();
 	}
 
 	/**

tests/oembed/OembedTest.php

@@ -1,16 +1,6 @@
 <?php
 
 class OembedTest extends SapphireTest {
-	public function setUp() {
-		parent::setUp();
-		Config::nest();
-	}
-
-	public function tearDown() {
-		Config::unnest();
-		parent::tearDown();
-	}
-
 	public function testGetOembedFromUrl() {
 		Config::inst()->update('Oembed', 'providers', array(
 			'http://*.silverstripe.com/watch*'=>'http://www.silverstripe.com/oembed/'

tests/security/BasicAuthTest.php

@@ -14,17 +14,11 @@ class BasicAuthTest extends FunctionalTest {
 		parent::setUp();
 
 		// Fixtures assume Email is the field used to identify the log in identity
-		Config::nest();
 		Member::config()->unique_identifier_field = 'Email';
 		Security::$force_database_is_ready = true; // Prevents Member test subclasses breaking ready test
 		Member::config()->lock_out_after_incorrect_logins = 10;
 	}
 
-	public function tearDown() {
-		Config::unnest();
-		parent::tearDown();
-	}
-
 	public function testBasicAuthEnabledWithoutLogin() {
 		$origUser = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : null;
 		$origPw = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : null;