Merge branch '3.1'

control/Director.php

@@ -127,7 +127,7 @@ class Director implements TemplateGlobalProvider {
 		}
 
 		// Only resume a session if its not started already, and a session identifier exists
-		if(!isset($_SESSION) && (isset($_COOKIE[session_name()]) || isset($_REQUEST[session_name()]))) {
+		if(!isset($_SESSION) && Session::request_contains_session_id()) {
 			Session::start();
 		}
 		// Initiate an empty session - doesn't initialize an actual PHP session until saved (see belwo)
@@ -714,6 +714,26 @@ class Director implements TemplateGlobalProvider {
 		return Director::protocol() . $login .  $_SERVER['HTTP_HOST'] . Director::baseURL();
 	}
 
+	/**
+	 * Skip any further processing and immediately respond with a redirect to the passed URL.
+	 *
+	 * @param string $destURL - The URL to redirect to
+	 */
+	protected static function force_redirect($destURL) {
+		$response = new SS_HTTPResponse(
+			"<h1>Your browser is not accepting header redirects</h1>".
+			"<p>Please <a href=\"$destURL\">click here</a>",
+			301
+		);
+
+		HTTP::add_cache_headers($response);
+		$response->addHeader('Location', $destURL);
+
+		// TODO: Use an exception - ATM we can be called from _config.php, before Director#handleRequest's try block
+		$response->output();
+		die;
+	}
+
 	/**
 	 * Force the site to run on SSL.
 	 * 
@@ -782,10 +802,7 @@ class Director implements TemplateGlobalProvider {
 			if(class_exists('SapphireTest', false) && SapphireTest::is_running_test()) {
 				return $destURL;
 			} else {
-				if(!headers_sent()) header("Location: $destURL");
-				
-				die("<h1>Your browser is not accepting header redirects</h1>"
-					. "<p>Please <a href=\"$destURL\">click here</a>");
+				self::force_redirect($destURL);
 			}
 		} else {
 			return false;
@@ -800,9 +817,7 @@ class Director implements TemplateGlobalProvider {
 			$destURL = str_replace(Director::protocol(), Director::protocol() . 'www.', 
 				Director::absoluteURL($_SERVER['REQUEST_URI']));
 
-			header("Location: $destURL", true, 301);
-			die("<h1>Your browser is not accepting header redirects</h1>"
-				. "<p>Please <a href=\"$destURL\">click here</a>");
+			self::force_redirect($destURL);
 		}
 	}
 

control/HTTP.php

@@ -338,11 +338,11 @@ class HTTP {
 			$responseHeaders["Cache-Control"] = "max-age=" . self::$cache_age . ", must-revalidate, no-transform";
 			$responseHeaders["Pragma"] = "";
 
-			// To do: User-Agent should only be added in situations where you *are* actually 
+			// To do: User-Agent should only be added in situations where you *are* actually
 			// varying according to user-agent.
 			$responseHeaders['Vary'] = 'Cookie, X-Forwarded-Protocol, User-Agent, Accept';
-
-		} else {
+		}
+		else {
 			$responseHeaders["Cache-Control"] = "no-cache, max-age=0, must-revalidate, no-transform";
 		}
 

control/Session.php

@@ -128,6 +128,14 @@ class Session {
 	
 	protected $changedData = array();
 
+	protected function userAgent() {
+		if (isset($_SERVER['HTTP_USER_AGENT'])) {
+			return $_SERVER['HTTP_USER_AGENT'];
+		} else {
+			return '';
+		}
+	}
+
 	/**
 	 * Start PHP session, then create a new Session object with the given start data.
 	 *
@@ -138,14 +146,8 @@ class Session {
 
 		$this->data = $data;
 		
-		if (isset($_SERVER['HTTP_USER_AGENT'])) {
-			$ua = $_SERVER['HTTP_USER_AGENT'];
-		} else {
-			$ua = '';
-		}
-
 		if (isset($this->data['HTTP_USER_AGENT'])) {
-			if ($this->data['HTTP_USER_AGENT'] != $ua) {
+			if ($this->data['HTTP_USER_AGENT'] != $this->userAgent()) {
 				// Funny business detected!
 				$this->inst_clearAll();
 				
@@ -153,8 +155,6 @@ class Session {
 				Session::start();
 			}
 		}
-		
-		$this->inst_set('HTTP_USER_AGENT', $ua);
 	}
 
 	/**
@@ -460,13 +460,18 @@ class Session {
 	public function inst_getAll() {
 		return $this->data;
 	}
-	
+
+	public function inst_finalize() {
+		$this->inst_set('HTTP_USER_AGENT', $this->userAgent());
+	}
+
 	/**
 	 * Save data to session
 	 * Only save the changes, so that anyone manipulating $_SESSION directly doesn't get burned.
 	 */ 
 	public function inst_save() {
 		if($this->changedData) {
+			$this->inst_finalize();
 			if(!isset($_SESSION)) Session::start();
 			$this->recursivelyApply($this->changedData, $_SESSION);
 		}
@@ -508,6 +513,16 @@ class Session {
 		Session::set("FormInfo.$formname.formError.type", $type);
 	}
 
+	/**
+	 * Is there a session ID in the request?
+	 * @return bool
+	 */
+	public static function request_contains_session_id() {
+		$secure = Director::is_https() && Config::inst()->get('Session', 'cookie_secure');
+		$name = $secure ? 'SECSESSID' : session_name();
+		return isset($_COOKIE[$name]) || isset($_REQUEST[$name]);
+	}
+
 	/**
 	 * Initialize session.
 	 *
@@ -517,7 +532,7 @@ class Session {
 		$path = Config::inst()->get('Session', 'cookie_path');
 		if(!$path) $path = Director::baseURL();
 		$domain = Config::inst()->get('Session', 'cookie_domain');
-		$secure = Config::inst()->get('Session', 'cookie_secure');
+		$secure = Director::is_https() && Config::inst()->get('Session', 'cookie_secure');
 		$session_path = Config::inst()->get('Session', 'session_store_path');
 		$timeout = Config::inst()->get('Session', 'timeout');
 
@@ -533,11 +548,14 @@ class Session {
 			// Allow storing the session in a non standard location
 			if($session_path) session_save_path($session_path);
 
+			// If we want a secure cookie for HTTPS, use a seperate session name. This lets us have a
+			// seperate (less secure) session for non-HTTPS requests
+			if($secure) session_name('SECSESSID');
+
 			// @ is to supress win32 warnings/notices when session wasn't cleaned up properly
 			// There's nothing we can do about this, because it's an operating system function!
 			if($sid) session_id($sid);
 			@session_start();
-
 		}
 
 		// Modify the timeout behaviour so it's the *inactive* time before the session expires.

core/manifest/ClassManifest.php

@@ -273,6 +273,15 @@ class SS_ClassManifest {
 		return $modules;
 	}
 
+	/**
+	 * Used to set up files that we want to exclude from parsing for performance reasons.
+	 */
+	protected function setDefaults()
+	{
+		$this->classes['sstemplateparser'] = FRAMEWORK_PATH.'/view/SSTemplateParser.php';
+		$this->classes['sstemplateparseexception'] = FRAMEWORK_PATH.'/view/SSTemplateParser.php';
+	}
+
 	/**
 	 * Completely regenerates the manifest file.
 	 *
@@ -289,10 +298,12 @@ class SS_ClassManifest {
 			$this->$reset = array();
 		}
 
+		$this->setDefaults();
+
 		$finder = new ManifestFileFinder();
 		$finder->setOptions(array(
 			'name_regex'    => '/^(_config.php|[^_].*\.php)$/',
-			'ignore_files'  => array('index.php', 'main.php', 'cli-script.php'),
+			'ignore_files'  => array('index.php', 'main.php', 'cli-script.php', 'SSTemplateParser.php'),
 			'ignore_tests'  => !$this->tests,
 			'file_callback' => array($this, 'handleFile'),
 			'dir_callback' => array($this, 'handleDir')

core/manifest/ConfigManifest.php

@@ -291,7 +291,7 @@ class SS_ConfigManifest {
 						// For each, parse out into module/file#name, and set any missing to "*"
 						$header[$order] = array();
 						foreach($orderparts as $part) {
-							preg_match('! (?P<module>\*|\w+)? (\/ (?P<file>\*|\w+))? (\# (?P<fragment>\*|\w+))? !x',
+							preg_match('! (?P<module>\*|[^\/#]+)? (\/ (?P<file>\*|\w+))? (\# (?P<fragment>\*|\w+))? !x',
 								$part, $match);
 
 							$header[$order][] = array(

core/manifest/ConfigStaticManifest.php

@@ -100,7 +100,7 @@ class SS_ConfigStaticManifest {
 		$finder = new ManifestFileFinder();
 		$finder->setOptions(array(
 			'name_regex'    => '/^([^_].*\.php)$/',
-			'ignore_files'  => array('index.php', 'main.php', 'cli-script.php'),
+			'ignore_files'  => array('index.php', 'main.php', 'cli-script.php', 'SSTemplateParser.php'),
 			'ignore_tests'  => !$this->tests,
 			'file_callback' => array($this, 'handleFile')
 		));

email/Email.php

@@ -349,8 +349,9 @@ class Email extends ViewableData {
 	 * and it won't be plain email :) 
 	 */
 	protected function parseVariables($isPlain = false) {
+		$origState = Config::inst()->get('SSViewer', 'source_file_comments');
 		Config::inst()->update('SSViewer', 'source_file_comments', false);
-		
+
 		if(!$this->parseVariables_done) {
 			$this->parseVariables_done = true;
 
@@ -373,6 +374,7 @@ class Email extends ViewableData {
 			// Rewrite relative URLs
 			$this->body = HTTP::absoluteURLs($fullBody);
 		}
+		Config::inst()->update('SSViewer', 'source_file_comments', $origState);
 	}
 	
 	/**

forms/TimeField.php

@@ -76,6 +76,33 @@ class TimeField extends TextField {
 	public function Type() {
 		return 'time text';
 	}
+	
+	/**
+	 * Parses a time into a Zend_Date object
+	 * 
+	 * @param string $value Raw value
+	 * @param string $format Format string to check against
+	 * @param string $locale Optional locale to parse against
+	 * @param boolean $exactMatch Flag indicating that the date must be in this
+	 * exact format, and is unchanged after being parsed and written out
+	 * 
+	 * @return Zend_Date Returns the Zend_Date, or null if not in the specified format
+	 */
+	protected function parseTime($value, $format, $locale = null, $exactMatch = false) {
+		// Check if the date is in the correct format
+		if(!Zend_Date::isDate($value, $format)) return null;
+		
+		// Parse the value
+		$valueObject = new Zend_Date($value, $format, $locale);
+			
+		// For exact matches, ensure the value preserves formatting after conversion
+		if($exactMatch && ($value !== $valueObject->get($format))) {
+			return null;
+		} else {
+			return $valueObject;
+		}
+	}
+	
 
 	/**
 	 * Sets the internal value to ISO date format.
@@ -83,6 +110,7 @@ class TimeField extends TextField {
 	 * @param String|Array $val
 	 */
 	public function setValue($val) {
+		
 		// Fuzzy matching through strtotime() to support a wider range of times,
 		// e.g. 11am. This means that validate() might not fire.
 		// Note: Time formats are assumed to be less ambiguous than dates across locales.
@@ -99,13 +127,12 @@ class TimeField extends TextField {
 			$this->valueObj = null;
 		}
 		// load ISO time from database (usually through Form->loadDataForm())
-		else if(Zend_Date::isDate($val, $this->getConfig('datavalueformat'))) {
-			$this->valueObj = new Zend_Date($val, $this->getConfig('datavalueformat'));
+		// Requires exact format to prevent false positives from locale specific times
+		else if($this->valueObj = $this->parseTime($val, $this->getConfig('datavalueformat'), null, true)) {
 			$this->value = $this->valueObj->get($this->getConfig('timeformat'));
 		}
 		// Set in current locale (as string)
-		else if(Zend_Date::isDate($val, $this->getConfig('timeformat'), $this->locale)) {
-			$this->valueObj = new Zend_Date($val, $this->getConfig('timeformat'), $this->locale);
+		else if($this->valueObj = $this->parseTime($val, $this->getConfig('timeformat'), $this->locale)) {
 			$this->value = $this->valueObj->get($this->getConfig('timeformat'));
 		}
 		// Fallback: Set incorrect value so validate() can pick it up

javascript/UploadField.js

@@ -383,6 +383,7 @@
 		$( 'div.ss-upload:not(.disabled):not(.readonly) .ss-uploadfield-item-edit').entwine({
 			onclick: function(e) {
 				var editform = this.closest('.ss-uploadfield-item').find('.ss-uploadfield-item-editform');
+				var itemInfo = editform.prev('.ss-uploadfield-item-info');
 				var disabled;
 				var iframe = editform.find('iframe');
 
@@ -406,8 +407,15 @@
 						disabled=this.find('ss-uploadfield-item-edit').siblings();
 					}
 					editform.parent('.ss-uploadfield-item').removeClass('ui-state-warning');
-					disabled.toggleClass('ui-state-disabled');
 					editform.toggleEditForm();
+					
+					if (itemInfo.find('.toggle-details-icon').hasClass('opened')) {
+						disabled.addClass('ui-state-disabled');
+						disabled.attr('disabled', 'disabled');
+					} else {
+						disabled.removeClass('ui-state-disabled');
+						disabled.removeAttr('disabled');
+					}
 				}
 				e.preventDefault(); // Avoid a form submit
 				return false;

tests/control/SessionTest.php

@@ -99,6 +99,7 @@ class SessionTest extends SapphireTest {
 		// Generate our session
 		$s = new Session(array());
 		$s->inst_set('val', 123);
+		$s->inst_finalize();
 
 		// Change our UA
 		$_SERVER['HTTP_USER_AGENT'] = 'Fake Agent';

tests/core/manifest/ClassManifestTest.php

@@ -36,17 +36,20 @@ class ClassManifestTest extends SapphireTest {
 
 	public function testGetClasses() {
 		$expect = array(
-			'classa' => "{$this->base}/module/classes/ClassA.php",
-			'classb' => "{$this->base}/module/classes/ClassB.php",
-			'classc' => "{$this->base}/module/classes/ClassC.php",
-			'classd' => "{$this->base}/module/classes/ClassD.php"
+			'classb'                   => "{$this->base}/module/classes/ClassB.php",
+			'classa'                   => "{$this->base}/module/classes/ClassA.php",
+			'classb'                   => "{$this->base}/module/classes/ClassB.php",
+			'classc'                   => "{$this->base}/module/classes/ClassC.php",
+			'classd'                   => "{$this->base}/module/classes/ClassD.php",
+			'sstemplateparser'         => FRAMEWORK_PATH."/view/SSTemplateParser.php",
+			'sstemplateparseexception' => FRAMEWORK_PATH."/view/SSTemplateParser.php"
 		);
 		$this->assertEquals($expect, $this->manifest->getClasses());
 	}
 
 	public function testGetClassNames() {
 		$this->assertEquals(
-			array('classa', 'classb', 'classc', 'classd'),
+			array('sstemplateparser', 'sstemplateparseexception', 'classa', 'classb', 'classc', 'classd'),
 			$this->manifest->getClassNames());
 	}
 

tests/core/manifest/ConfigManifestTest.php

@@ -13,6 +13,164 @@ class ConfigManifestTest extends SapphireTest {
 		return $manifest->get('ConfigManifestTest', $name);
 	}
 
+	/**
+	 * This is a helper method for displaying a relevant message about a parsing failure
+	 */
+	protected function getParsedAsMessage($path) {
+		return sprintf('Reference path "%s" failed to parse correctly', $path);
+	}
+
+	/**
+	 * This test checks the processing of before and after reference paths (module-name/filename#fragment)
+	 * This method uses fixture/configmanifest/mysite/_config/addyamlconfigfile.yml as a fixture
+	 */
+	public function testAddYAMLConfigFileReferencePathParsing() {
+		// Use a mock to avoid testing unrelated functionality
+		$manifest = $this->getMockBuilder('SS_ConfigManifest')
+			->disableOriginalConstructor()
+			->setMethods(array('addModule'))
+			->getMock();
+
+		// This tests that the addModule method is called with the correct value
+		$manifest->expects($this->once())
+			->method('addModule')
+			->with($this->equalTo(dirname(__FILE__).'/fixtures/configmanifest/mysite'));
+
+		// Call the method to be tested
+		$manifest->addYAMLConfigFile(
+			'addyamlconfigfile.yml',
+			dirname(__FILE__).'/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml',
+			false
+		);
+
+		// There is no getter for yamlConfigFragments
+		$property = new ReflectionProperty('SS_ConfigManifest', 'yamlConfigFragments');
+		$property->setAccessible(true);
+
+		// Get the result back from the parsing
+		$result = $property->getValue($manifest);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => 'mysite',
+					'file' => 'testfile',
+					'name' => 'fragment',
+				),
+			),
+			@$result[0]['after'],
+			$this->getParsedAsMessage('mysite/testfile#fragment')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => 'test-module',
+					'file' => 'testfile',
+					'name' => 'fragment',
+				),
+			),
+			@$result[1]['after'],
+			$this->getParsedAsMessage('test-module/testfile#fragment')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => '*',
+					'file' => '*',
+					'name' => '*',
+				),
+			),
+			@$result[2]['after'],
+			$this->getParsedAsMessage('*')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => '*',
+					'file' => 'testfile',
+					'name' => '*'
+				),
+			),
+			@$result[3]['after'],
+			$this->getParsedAsMessage('*/testfile')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => '*',
+					'file' => '*',
+					'name' => 'fragment'
+				),
+			),
+			@$result[4]['after'],
+			$this->getParsedAsMessage('*/*#fragment')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => '*',
+					'file' => '*',
+					'name' => 'fragment'
+				),
+			),
+			@$result[5]['after'],
+			$this->getParsedAsMessage('#fragment')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => 'test-module',
+					'file' => '*',
+					'name' => 'fragment'
+				),
+			),
+			@$result[6]['after'],
+			$this->getParsedAsMessage('test-module#fragment')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => 'test-module',
+					'file' => '*',
+					'name' => '*'
+				),
+			),
+			@$result[7]['after'],
+			$this->getParsedAsMessage('test-module')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => 'test-module',
+					'file' => '*',
+					'name' => '*'
+				),
+			),
+			@$result[8]['after'],
+			$this->getParsedAsMessage('test-module/*')
+		);
+
+		$this->assertEquals(
+			array(
+				array(
+					'module' => 'test-module',
+					'file' => '*',
+					'name' => '*'
+				),
+			),
+			@$result[9]['after'],
+			$this->getParsedAsMessage('test-module/*/#*')
+		);
+	}
+
 	public function testClassRules() {
 		$config = $this->getConfigFixtureValue('Class');
 

tests/core/manifest/NamespacedClassManifestTest.php

@@ -12,7 +12,7 @@ class NamespacedClassManifestTest extends SapphireTest {
 
 	public function setUp() {
 		parent::setUp();
-		
+
 		$this->base = dirname(__FILE__) . '/fixtures/namespaced_classmanifest';
 		$this->manifest      = new SS_ClassManifest($this->base, false, true, false);
 	}
@@ -41,17 +41,20 @@ class NamespacedClassManifestTest extends SapphireTest {
 			'silverstripe\test\classe' => "{$this->base}/module/classes/ClassE.php",
 			'silverstripe\test\classf' => "{$this->base}/module/classes/ClassF.php",
 			'silverstripe\test\classg' => "{$this->base}/module/classes/ClassG.php",
-			'silverstripe\test\classh' => "{$this->base}/module/classes/ClassH.php"
+			'silverstripe\test\classh' => "{$this->base}/module/classes/ClassH.php",
+			'sstemplateparser'         => FRAMEWORK_PATH."/view/SSTemplateParser.php",
+			'sstemplateparseexception' => FRAMEWORK_PATH."/view/SSTemplateParser.php"
 		);
-		
+
 		$this->assertEquals($expect, $this->manifest->getClasses());
 	}
 
 	public function testGetClassNames() {
 		$this->assertEquals(
-			array('silverstripe\test\classa', 'silverstripe\test\classb', 'silverstripe\test\classc',
-				'silverstripe\test\classd', 'silverstripe\test\classe', 'silverstripe\test\classf',
-				'silverstripe\test\classg', 'silverstripe\test\classh'),
+			array('sstemplateparser', 'sstemplateparseexception', 'silverstripe\test\classa',
+				'silverstripe\test\classb', 'silverstripe\test\classc', 'silverstripe\test\classd',
+				'silverstripe\test\classe', 'silverstripe\test\classf', 'silverstripe\test\classg',
+				'silverstripe\test\classh'),
 			$this->manifest->getClassNames());
 	}
 
@@ -59,7 +62,7 @@ class NamespacedClassManifestTest extends SapphireTest {
 		$expect = array(
 			'silverstripe\test\classa' => array('silverstripe\test\ClassB', 'silverstripe\test\ClassH'),
 		);
-		
+
 		$this->assertEquals($expect, $this->manifest->getDescendants());
 	}
 
@@ -109,7 +112,7 @@ class NamespacedClassManifestTest extends SapphireTest {
 		$expect = array("{$this->base}/module/_config.php");
 		$this->assertEquals($expect, $this->manifest->getConfigs());
 	}
-	
+
 	public function testGetModules() {
 		$expect = array(
 			"module" => "{$this->base}/module",

tests/core/manifest/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml

@@ -0,0 +1,59 @@
+---
+After: 'mysite/testfile#fragment'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: 'test-module/testfile#fragment'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: '*'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: '*/testfile'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: '*/*#fragment'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: '#fragment'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: 'test-module#fragment'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: 'test-module'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: 'test-module/*'
+---
+ClassManifestTest:
+    testParam: false
+
+---
+After: 'test-module/*#*'
+---
+ClassManifestTest:
+    testParam: false

tests/forms/TimeFieldTest.php

@@ -100,4 +100,36 @@ class TimeFieldTest extends SapphireTest {
 		$field->setValue('');
 		$this->assertEquals($field->dataValue(), '');
 	}
+	
+	/**
+	 * Test that AM/PM is preserved correctly in various situations
+	 */
+	public function testPreserveAMPM() {
+		
+		// Test with timeformat that includes hour
+		
+		// Check pm
+		$f = new TimeField('Time', 'Time');
+		$f->setConfig('timeformat', 'h:mm:ss a');
+		$f->setValue('3:59 pm');
+		$this->assertEquals($f->dataValue(), '15:59:00');
+		
+		// Check am
+		$f = new TimeField('Time', 'Time');
+		$f->setConfig('timeformat', 'h:mm:ss a');
+		$f->setValue('3:59 am');
+		$this->assertEquals($f->dataValue(), '03:59:00');
+		
+		// Check with ISO date/time
+		$f = new TimeField('Time', 'Time');
+		$f->setConfig('timeformat', 'h:mm:ss a');
+		$f->setValue('15:59:00');
+		$this->assertEquals($f->dataValue(), '15:59:00');
+		
+		// ISO am
+		$f = new TimeField('Time', 'Time');
+		$f->setConfig('timeformat', 'h:mm:ss a');
+		$f->setValue('03:59:00');
+		$this->assertEquals($f->dataValue(), '03:59:00');
+	}
 }

tests/templates/SSViewerTestIncludeScopeInheritance.ss

@@ -0,0 +1,3 @@
+<% loop Items %>
+	<% include SSViewerTestIncludeScopeInheritanceInclude %>
+<% end_loop %>

tests/templates/SSViewerTestIncludeScopeInheritanceInclude.ss

@@ -0,0 +1 @@
+$Title <% if ArgA %>- $ArgA <% end_if %>- <%if First %>First-<% end_if %><% if Last %>Last-<% end_if %><%if MultipleOf(2) %>EVEN<% else %>ODD<% end_if %> top:$Top.Title

tests/templates/SSViewerTestIncludeScopeInheritanceWithArgs.ss

@@ -0,0 +1,3 @@
+<% loop Items %>
+	<% include SSViewerTestIncludeScopeInheritanceInclude ArgA=$Title %>
+<% end_loop %>

tests/view/SSViewerTest.php

@@ -29,7 +29,57 @@ class SSViewerTest extends SapphireTest {
 		$result = $data->renderWith("SSViewerTestPartialTemplate");
 		$this->assertEquals('Test partial template: var value', trim(preg_replace("/<!--.*-->/U",'',$result)));
 	}
-	
+
+	public function testIncludeScopeInheritance() {
+		$data = $this->getScopeInheritanceTestData();
+		$expected = array(
+			'Item 1 - First-ODD top:Item 1',
+			'Item 2 - EVEN top:Item 2',
+			'Item 3 - ODD top:Item 3',
+			'Item 4 - EVEN top:Item 4',
+			'Item 5 - ODD top:Item 5',
+			'Item 6 - Last-EVEN top:Item 6',
+		);
+
+		$result = $data->renderWith('SSViewerTestIncludeScopeInheritance');
+		$this->assertExpectedStrings($result, $expected);
+
+		// reset results for the tests that include arguments (the title is passed as an arg)
+		$expected = array(
+			'Item 1 - Item 1 - First-ODD top:Item 1',
+			'Item 2 - Item 2 - EVEN top:Item 2',
+			'Item 3 - Item 3 - ODD top:Item 3',
+			'Item 4 - Item 4 - EVEN top:Item 4',
+			'Item 5 - Item 5 - ODD top:Item 5',
+			'Item 6 - Item 6 - Last-EVEN top:Item 6',
+		);
+
+		$result = $data->renderWith('SSViewerTestIncludeScopeInheritanceWithArgs');
+		$this->assertExpectedStrings($result, $expected);
+	}
+
+	private function getScopeInheritanceTestData() {
+		return new ArrayData(array(
+			'Title' => 'TopTitleValue',
+			'Items' => new ArrayList(array(
+				new ArrayData(array('Title' => 'Item 1')),
+				new ArrayData(array('Title' => 'Item 2')),
+				new ArrayData(array('Title' => 'Item 3')),
+				new ArrayData(array('Title' => 'Item 4')),
+				new ArrayData(array('Title' => 'Item 5')),
+				new ArrayData(array('Title' => 'Item 6'))
+			))
+		));
+	}
+
+	private function assertExpectedStrings($result, $expected) {
+		foreach ($expected as $expectedStr) {
+			$this->assertTrue(
+				(boolean) preg_match("/{$expectedStr}/", $result),
+				"Didn't find '{$expectedStr}' in:\n{$result}"
+			);
+		}
+	}
 	
 	/**
 	 * Small helper to render templates from strings

view/SSTemplateParser.php

@@ -3242,7 +3242,7 @@ class SSTemplateParser extends Parser {
 		$arguments = $res['arguments'];
 
 		$res['php'] = '$val .= SSViewer::execute_template('.$template.', $scope->getItem(), array(' . 
-			implode(',', $arguments)."));\n";
+			implode(',', $arguments)."), \$scope);\n";
 
 		if($this->includeDebuggingComments) { // Add include filename comments on dev sites
 			$res['php'] =

view/SSTemplateParser.php.inc

@@ -701,7 +701,7 @@ class SSTemplateParser extends Parser {
 		$arguments = $res['arguments'];
 
 		$res['php'] = '$val .= SSViewer::execute_template('.$template.', $scope->getItem(), array(' . 
-			implode(',', $arguments)."));\n";
+			implode(',', $arguments)."), \$scope);\n";
 
 		if($this->includeDebuggingComments) { // Add include filename comments on dev sites
 			$res['php'] =

view/SSViewer.php

@@ -47,11 +47,17 @@ class SSViewer_Scope {
 	private $localIndex;
 
 
-	public function __construct($item){
+	public function __construct($item, $inheritedScope = null) {
 		$this->item = $item;
 		$this->localIndex = 0;
 		$this->localStack = array();
-		$this->itemStack[] = array($this->item, null, 0, null, null, 0);
+		if ($inheritedScope instanceof SSViewer_Scope) {
+			$this->itemIterator = $inheritedScope->itemIterator;
+			$this->itemIteratorTotal = $inheritedScope->itemIteratorTotal;
+			$this->itemStack[] = array($this->item, $this->itemIterator, $this->itemIteratorTotal, null, null, 0);
+		} else {
+			$this->itemStack[] = array($this->item, null, 0, null, null, 0);
+		}
 	}
 	
 	public function getItem(){
@@ -357,8 +363,8 @@ class SSViewer_DataPresenter extends SSViewer_Scope {
 	 */
 	protected $underlay;
 
-	public function __construct($item, $overlay = null, $underlay = null){
-		parent::__construct($item);
+	public function __construct($item, $overlay = null, $underlay = null, $inheritedScope = null) {
+		parent::__construct($item, $inheritedScope);
 
 		// Build up global property providers array only once per request
 		if (self::$globalProperties === null) {
@@ -553,7 +559,7 @@ class SSViewer {
 	 * @var boolean $source_file_comments
 	 */
 	private static $source_file_comments = false;
-	
+
 	/**
 	 * Set whether HTML comments indicating the source .SS file used to render this page should be
 	 * included in the output.  This is enabled by default
@@ -895,10 +901,11 @@ class SSViewer {
 	 * @param Object $item - The item to use as the root scope for the template
 	 * @param array|null $overlay - Any variables to layer on top of the scope
 	 * @param array|null $underlay - Any variables to layer underneath the scope
+	 * @param Object $inheritedScope - the current scope of a parent template including a sub-template
 	 *
 	 * @return string - The result of executing the template
 	 */
-	protected function includeGeneratedTemplate($cacheFile, $item, $overlay, $underlay) {
+	protected function includeGeneratedTemplate($cacheFile, $item, $overlay, $underlay, $inheritedScope = null) {
 		if(isset($_GET['showtemplate']) && $_GET['showtemplate'] && Permission::check('ADMIN')) {
 			$lines = file($cacheFile);
 			echo "<h2>Template: $cacheFile</h2>";
@@ -910,7 +917,7 @@ class SSViewer {
 		}
 
 		$cache = $this->getPartialCacheStore();
-		$scope = new SSViewer_DataPresenter($item, $overlay, $underlay);
+		$scope = new SSViewer_DataPresenter($item, $overlay, $underlay, $inheritedScope);
 		$val = '';
 
 		include($cacheFile);
@@ -930,11 +937,12 @@ class SSViewer {
 	 * Note: You can call this method indirectly by {@link ViewableData->renderWith()}.
 	 * 
 	 * @param ViewableData $item
-	 * @param SS_Cache $cache Optional cache backend.
+	 * @param array|null $arguments - arguments to an included template
+	 * @param Object $inheritedScope - the current scope of a parent template including a sub-template
 	 *
 	 * @return HTMLText Parsed template output.
 	 */
-	public function process($item, $arguments = null) {
+	public function process($item, $arguments = null, $inheritedScope = null) {
 		SSViewer::$topLevel[] = $item;
 
 		if ($arguments && $arguments instanceof Zend_Cache_Core) {
@@ -979,7 +987,7 @@ class SSViewer {
 			}
 		}
 
-		$output = $this->includeGeneratedTemplate($cacheFile, $item, $arguments, $underlay);
+		$output = $this->includeGeneratedTemplate($cacheFile, $item, $arguments, $underlay, $inheritedScope);
 		
 		if($this->includeRequirements) {
 			$output = Requirements::includeInHTML($template, $output);
@@ -1009,11 +1017,11 @@ class SSViewer {
 	 * Execute the given template, passing it the given data.
 	 * Used by the <% include %> template tag to process templates.
 	 */
-	public static function execute_template($template, $data, $arguments = null) {
+	public static function execute_template($template, $data, $arguments = null, $scope = null) {
 		$v = new SSViewer($template);
 		$v->includeRequirements(false);
 
-		return $v->process($data, $arguments);
+		return $v->process($data, $arguments, $scope);
 	}
 
 	public static function parseTemplateContent($content, $template="") {
@@ -1071,7 +1079,7 @@ class SSViewer_FromString extends SSViewer {
 		$this->content = $content;
 	}
 	
-	public function process($item, $arguments = null) {
+	public function process($item, $arguments = null, $scope = null) {
 		if ($arguments && $arguments instanceof Zend_Cache_Core) {
 			Deprecation::notice('3.0', 'Use setPartialCacheStore to override the partial cache storage backend, ' .
 				'the second argument to process is now an array of variables.');
@@ -1086,7 +1094,7 @@ class SSViewer_FromString extends SSViewer {
 		fwrite($fh, $template);
 		fclose($fh);
 
-		$val = $this->includeGeneratedTemplate($tmpFile, $item, $arguments, null);
+		$val = $this->includeGeneratedTemplate($tmpFile, $item, $arguments, null, $scope);
 
 		unlink($tmpFile);
 		return $val;