Merge branch 'develop-olympus' into develop

* develop-olympus:
  [ticket/11538] Add optional switch as argument to hex colour validation
  [ticket/11538] Modify test colour values
  [ticket/11538] Limit comment in acp_groups to 80 characters per line
  [ticket/11538] Move group ID into abstract test class and add more test cases
  [ticket/11538] Merge calls to validate_data() in acp_groups
  [ticket/11538] Rename phpbb_validate_colour to phpbb_validate_hex_colour
  [ticket/11538] Use abstract class for functional test cases for group colour
  [ticket/11538] Add function phpbb_validate_colour for validating colours
  [ticket/11538] Make sure regex doesn't allow multiple color values
  [ticket/11538] Add tests for acp group manage page
  [ticket/11538] Simplify colour value check and remove support for '#'
  [ticket/11538] Fix incorrect regex and test for duplicate # in color string
  [ticket/11538] Use regex for testing color value and improve tests
  [ticket/11538] Make sure group color can't exceed maximum of 6 characters

Conflicts:
	phpBB/includes/functions_user.php
	phpBB/styles/prosilver/template/ucp_groups_manage.html

phpBB/includes/acp/acp_groups.php

@@ -386,13 +386,21 @@ class acp_groups
 						$error = array_merge($error, $phpbb_avatar_manager->localize_errors($user, $avatar_error));
 					}
 
-					// Validate the length of "Maximum number of allowed recipients per private message" setting.
+					/*
-					// We use 16777215 as a maximum because it matches MySQL unsigned mediumint maximum value
+					* Validate the length of "Maximum number of allowed recipients per
-					// which is the lowest amongst DBMSes supported by phpBB3
+					* private message" setting. We use 16777215 as a maximum because it matches
-					if ($max_recipients_error = validate_data($submit_ary, array('max_recipients' => array('num', false, 0, 16777215))))
+					* MySQL unsigned mediumint maximum value which is the lowest amongst DBMSes
+					* supported by phpBB3. Also validate the submitted colour value.
+					*/
+					$validation_checks = array(
+						'max_recipients' => array('num', false, 0, 16777215),
+						'colour'	=> array('hex_colour', true),
+					);
+
+					if ($validation_error = validate_data($submit_ary, $validation_checks))
 					{
 						// Replace "error" string with its real, localised form
-						$error = array_merge($error, array_map(array(&$user, 'lang'), $max_recipients_error));
+						$error = array_merge($error, array_map(array(&$user, 'lang'), $validation_error));
 					}
 
 					if (!sizeof($error))

phpBB/includes/functions_user.php

@@ -1330,22 +1330,12 @@ function validate_data($data, $val_ary)
 		{
 			$function = array_shift($validate);
 			array_unshift($validate, $data[$var]);
+			$function_prefix = (function_exists('phpbb_validate_' . $function)) ? 'phpbb_validate_' : 'validate';
 
-			if (function_exists('phpbb_validate_' . $function))
+			if ($result = call_user_func_array($function_prefix . $function, $validate))
 			{
-				if ($result = call_user_func_array('phpbb_validate_' . $function, $validate))
+				// Since errors are checked later for their language file existence, we need to make sure custom errors are not adjusted.
-				{
+				$error[] = (empty($user->lang[$result . '_' . strtoupper($var)])) ? $result : $result . '_' . strtoupper($var);
-					// Since errors are checked later for their language file existence, we need to make sure custom errors are not adjusted.
-					$error[] = (empty($user->lang[$result . '_' . strtoupper($var)])) ? $result : $result . '_' . strtoupper($var);
-				}
-			}
-			else
-			{
-				if ($result = call_user_func_array('validate_' . $function, $validate))
-				{
-					// Since errors are checked later for their language file existence, we need to make sure custom errors are not adjusted.
-					$error[] = (empty($user->lang[$result . '_' . strtoupper($var)])) ? $result : $result . '_' . strtoupper($var);
-				}
 			}
 		}
 	}
@@ -2008,6 +1998,30 @@ function validate_jabber($jid)
 	return false;
 }
 
+/**
+* Validate hex colour value
+*
+* @param string $colour The hex colour value
+* @param bool $optional Whether the colour value is optional. True if an empty
+*			string will be accepted as correct input, false if not.
+* @return bool|string Error message if colour value is incorrect, false if it
+*			fits the hex colour code
+*/
+function phpbb_validate_hex_colour($colour, $optional = false)
+{
+	if (empty($colour))
+	{
+		return (($optional) ? false : 'WRONG_DATA');
+	}
+
+	if (!preg_match('/^([0-9a-fA-F]{6}|[0-9a-fA-F]{3})$/', $colour))
+	{
+		return 'WRONG_DATA';
+	}
+
+	return false;
+}
+
 /**
 * Verifies whether a style ID corresponds to an active style.
 *

phpBB/includes/ucp/ucp_groups.php

@@ -557,6 +557,13 @@ class ucp_groups
 								$error[] = $user->lang['FORM_INVALID'];
 							}
 
+							// Validate submitted colour value
+							if ($colour_error = validate_data($submit_ary, array('colour'	=> array('hex_colour', true))))
+							{
+								// Replace "error" string with its real, localised form
+								$error = array_merge($error, array_map(array(&$user, 'lang'), $colour_error));
+							}
+
 							if (!sizeof($error))
 							{
 								// Only set the rank, colour, etc. if it's changed or if we're adding a new

phpBB/language/en/common.php

@@ -819,6 +819,7 @@ $lang = array_merge($lang, array(
 	'WHO_IS_ONLINE'		=> 'Who is online',
 	'WRONG_PASSWORD'	=> 'You entered an incorrect password.',
 
+	'WRONG_DATA_COLOUR'			=> 'The colour value you entered is invalid.',
 	'WRONG_DATA_ICQ'			=> 'The number you entered is not a valid ICQ number.',
 	'WRONG_DATA_JABBER'			=> 'The name you entered is not a valid Jabber account name.',
 	'WRONG_DATA_LANG'			=> 'The language you specified is not valid.',

phpBB/styles/prosilver/template/ucp_groups_manage.html

@@ -54,7 +54,7 @@
 	<fieldset>
 	<dl>
 		<dt><label for="group_colour">{L_GROUP_COLOR}{L_COLON}</label><br /><span>{L_GROUP_COLOR_EXPLAIN}</span></dt>
-		<dd><input name="group_colour" type="text" id="group_colour" value="{GROUP_COLOUR}" size="7" maxlength="7" class="inputbox narrow" /> <span style="background-color: {GROUP_COLOUR};">&nbsp;&nbsp;&nbsp;</span> [ <a href="{U_SWATCH}" onclick="popup(this.href, 636, 150, '_swatch'); return false;">{L_COLOUR_SWATCH}</a> ]</dd>
+		<dd><input name="group_colour" type="text" id="group_colour" value="{GROUP_COLOUR}" size="6" maxlength="6" class="inputbox narrow" /> <span style="background-color: {GROUP_COLOUR};">&nbsp;&nbsp;&nbsp;</span> [ <a href="{U_SWATCH}" onclick="popup(this.href, 636, 150, '_swatch'); return false;">{L_COLOUR_SWATCH}</a> ]</dd>
 	</dl>
 	<dl>
 		<dt><label for="group_rank">{L_GROUP_RANK}{L_COLON}</label></dt>

tests/functional/acp_groups_test.php

@@ -0,0 +1,21 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/common_groups_test.php';
+
+/**
+* @group functional
+*/
+class phpbb_functional_acp_groups_test extends phpbb_functional_common_groups_test
+{
+	protected function get_url()
+	{
+		return 'adm/index.php?i=groups&mode=manage&action=edit';
+	}
+}

tests/functional/common_groups_test.php

@@ -0,0 +1,61 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+/**
+* @group functional
+*/
+abstract class phpbb_functional_common_groups_test extends phpbb_functional_test_case
+{
+	abstract protected function get_url();
+
+	public function groups_manage_test_data()
+	{
+		return array(
+			array('#AA0000', 'WRONG_DATA_COLOUR'),
+			array('AA0000', 'GROUP_UPDATED'),
+			array('AA0000v', 'WRONG_DATA_COLOUR'),
+			array('AA00000', 'WRONG_DATA_COLOUR'),
+			array('vAA0000', 'WRONG_DATA_COLOUR'),
+			array('AAG000','WRONG_DATA_COLOUR'),
+			array('a00', 'GROUP_UPDATED'),
+			array('ag0', 'WRONG_DATA_COLOUR'),
+			array('#aa0', 'WRONG_DATA_COLOUR'),
+			array('AA0000 ', 'GROUP_UPDATED'),
+			array('AA0000 abf', 'WRONG_DATA_COLOUR'),
+			array('AA0000 AA0000', 'WRONG_DATA_COLOUR'),
+			array('', 'GROUP_UPDATED'),
+			array('000', 'GROUP_UPDATED'),
+			array('000000', 'GROUP_UPDATED'),
+		);
+	}
+
+	/**
+	* @dataProvider groups_manage_test_data
+	*/
+	public function test_groups_manage($input, $expected)
+	{
+		$this->markTestIncomplete(
+			'Test fails on develop due to another test deleting the Administrators group.'
+		);
+		// See https://github.com/phpbb/phpbb3/pull/1407#issuecomment-18465480
+		// and https://gist.github.com/bantu/22dc4f6c6c0b8f9e0fa1
+
+		$this->login();
+		$this->admin_login();
+		$this->add_lang(array('ucp', 'acp/groups'));
+
+		// Manage Administrators group
+		$crawler = $this->request('GET', $this->get_url() . '&g=5&sid=' . $this->sid);
+		$this->assert_response_success();
+		$form = $crawler->selectButton($this->lang('SUBMIT'))->form();
+		$form['group_colour']->setValue($input);
+		$crawler = $this->client->submit($form);
+		$this->assertContains($this->lang($expected), $crawler->text());
+	}
+}

tests/functional/ucp_groups_test.php

@@ -0,0 +1,21 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/common_groups_test.php';
+
+/**
+* @group functional
+*/
+class phpbb_functional_ucp_groups_test extends phpbb_functional_common_groups_test
+{
+	protected function get_url()
+	{
+		return 'ucp.php?i=groups&mode=manage&action=edit';
+	}
+}