chat failed validation...

[spaceace]

failed for the following reasons...

from ajaxchat_module.php

Code: Select all

#region Submit

# should not be used as comment, use // instead. - DONE

Code: Select all

				$sql1 = 'DELETE FROM ' . CHAT_TABLE . '
					WHERE `message_id` <= ' . (int) $row . '';

You should remove the ` around your field as they are mysql only.
Also, the empty string at the end is not needed and should be removed. - DONE

Code: Select all

return '<a href="' . append_sid('?i=' . $this->id . '&mode=' . $this->mode . '&action=prune_chat') . '" data-ajax="true"><input class="button2" type="submit" id="' . $key . '_enable" name="' . $key . '_enable" value="' . $this->user->lang['PRUNE_NOW'] . '" /></a>';

Incorrect usage of append_sid, the first parameter should be a file, not a query string. In ACP module you should use $this->u_action. & should be replaced with &. - NOT DONE

Code: Select all

$sql1 = 'TRUNCATE ' . CHAT_TABLE . '';

Empty string at the end should be removed. - DONE

Code: Select all

$action		 = append_sid('?i=' . $this->id . '&mode=' . $this->mode . '&action=truncate_chat');

Same as above. - NOT DONE

Code: Select all

protected function do_submit_stuff($display_vars, $special_functions = [])

Do submit stuff is not really a descriptive function name. - DONE

from controller/chat.php:

Code: Select all

		include $this->root_path . 'includes/functions_posting.' . $this->php_ext;
		include $this->root_path . 'includes/functions_display.' . $this->php_ext;

Missing () around the path, also you should check first with function_exists to see if the files were already included. - DONE

Code: Select all

$this->post = $this->request->get_super_global(\phpbb\request\request_interface::POST);

Why do you need all $_POST items? Also, the property doesn't seem to be used at all in your file. - DONE

Code: Select all

		if ($this->mode === 'default')
		{
			$this->defaultAction();
		}
		else if ($this->mode === 'read')
		{
			$this->readAction();
		}
		else if ($this->mode === 'add')
		{
			$this->addAction();
		}
		else if ($this->mode === 'smilies')
		{
			$this->smiliesAction();
		}
		else if ($this->mode === 'delete')
		{
			$this->delAction();
		}

You should create seperate controller methods (And as such routes) instead of 1. The method smiliesAction doesn't exists in your file. - NOT DONE

Code: Select all

$sql	 = 'SELECT `user_lastpost` FROM ' . CHAT_SESSIONS_TABLE . " WHERE user_id = {$this->user->data['user_id']}";

` should be removed. - DONE

Code: Select all

$details = base64_decode('Jm5ic3A7PGEgaHJlZj0iaHR0cDovL3d3dy5saXZlbWVtYmVyc29ubHkuY29tIiBzdHlsZT0iZm9udC13ZWlnaHQ6IGJvbGQ7Ij5BSkFYJm5ic3A7Q2hhdCZuYnNwOyZjb3B5OyZuYnNwOzIwMTU8L2E+Jm5ic3A7PHN0cm9uZz5MaXZlJm5ic3A7TWVtYmVycyZuYnNwO09ubHk8L3N0cm9uZz4=');

Hardcoded language, HTML should be placed in the template files. Just have it like everything in your files instead of doing it like this. - DONE

Code: Select all

'EXT_STYLE_PATH'		=> '' . $this->ext_path_web . 'styles/',

empty string at the start should be removed. - DONE

Code: Select all

		$sql = 'SELECT c.*, p.post_visibility, u.user_avatar, u.user_avatar_type, u.user_avatar_width, u.user_avatar_height
			FROM ' . CHAT_TABLE . ' as c
			LEFT JOIN ' . USERS_TABLE . ' as u ON c.user_id = u.user_id
			LEFT JOIN ' . POSTS_TABLE . ' as p ON c.post_id = p.post_id
			WHERE c.message_id > ' . $this->last_id . '
			ORDER BY c.message_id DESC';

Please cast $this->last_id with (int). - DONE

Code: Select all

$message = utf8_normalize_nfc($this->request->variable('message', '', true));

You don't need utf8_normalize_nfc with $request->variable if you set the 3rd parameter to true. - DONE

Code: Select all

		$sql = 'SELECT `session_viewonline` '
				. 'FROM ' . SESSIONS_TABLE .' '
				. 'WHERE `session_user_id` = "' . $uid . '"';

Remove `. $uid should be casted if a integer, or escaped if it is a string. If it is a integer, it should not be in quotes et all, otherwise it should be in ' instead of " in your query. - DONE

From prune_ajaxchat.php:

Code: Select all

		$sql1 = 'DELETE FROM ' . CHAT_TABLE . '
			WHERE `message_id` <= ' . (int) $row . '';

Remove ` and the empty string at the end. - DONE

from listener.php:

Code: Select all

		include_once $this->root_path . 'includes/functions_posting.' . $this->php_ext;
		include_once $this->root_path . 'includes/functions_display.' . $this->php_ext;

Please add (), also you should use function_exists instead of include_once. - DONE

Code: Select all

		$sql = 'SELECT c.*, p.post_visibility, u.user_avatar, u.user_avatar_type, u.user_avatar_width, u.user_avatar_height
			FROM ' . CHAT_TABLE . ' as c
			LEFT JOIN ' . USERS_TABLE . ' as u ON c.user_id = u.user_id
			LEFT JOIN ' . POSTS_TABLE . ' as p ON c.post_id = p.post_id
			WHERE c.message_id > ' . $this->last_id . '
			ORDER BY c.message_id DESC';

Cast $this->last_id. - DONE

Code: Select all

		$sql = 'SELECT `session_viewonline` '
			. 'FROM ' . SESSIONS_TABLE .' '
			. 'WHERE `session_user_id` = "' . $uid . '"';

Remove `, cast or escape $uid, and use single quote instead of double when a string (Otherwise remove completly). - DONE

from ajaxchat.js:

Code: Select all

function handle_send(mode, f)
{
	if (xmlHttp.readyState == 4 || xmlHttp.readyState == 0)
	{
		indicator_switch('on');
		type = 'receive';
		param = 'mode=' + mode;
		param += '&last_id=' + last_id;
		param += '&last_time=' + last_time;
		param += '&last_post=' + post_time;
		param += '&read_interval=' + read_interval;

		if (mode == 'add' && document.postform.message.value != '')
		{
			type = 'send';
			for (var i = 0; i < f.elements.length; i++)
			{
				elem = f.elements[i];
				param += '&' + elem.name + '=' + blkopen + "" + encodeURIComponent(elem.value) + blkclose;
			}
			document.postform.message.value = '';
		}
		else if (mode == 'add' && document.postform.message.value == '')

		{
			alert(chat_empty);
			return false;
		}

		else if (mode == 'delete')
		{
			type = 'delete';
			param += '&chat_id=' + f;
		}
		xmlHttp.open("POST", query_url, true);
		xmlHttp.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded');
		xmlHttp.onreadystatechange = handle_return;
		xmlHttp.send(param);
	}
}

Please use jQuery instead. - WILL NOT REMOVE AJAX AT ALL IN MY JS

Code: Select all

					document.getElementById(fieldname).innerHTML = results[0];
				}
				else
				{
					document.getElementById(fieldname).innerHTML = results[0] + document.getElementById(fieldname).innerHTML;

Please use jQuery instead. This applies to other places as well. - WILL NOT REMOVE AJAX AT ALL IN MY JS

Your template files in all/ seems to be prosilver specific. There should be no prosilver specific styles in all/, they should be in prosilver/.

The black template only different on a extension with the all template. As such, this should be combined into 1 instead of duplicated code.
The same applies to some of the other templates. You should create smaller template files (Having 1 function for 1 template file, instead of multiple functions), and create template which can be used for more then 1 style.
You should have as less as possible code duplication, also in your styles. - DONE

From ucp_ajaxchat_info.php:

Code: Select all

'auth'  => 'ext_spaceace/ajaxchat && acl_u_chgprofileinfo',

Are you sure this is the correct auth? Only admins with a very specific auth can view this module. if it is correct, this module should be moved to the ACP instead of the UCP. - DON"T THINK THIS IS AN ISSUE AS THIS AUTH DOES EXACTLY WHAT IT SHOULD

[Gnome!]

Do as they say and re submit if they say it's no good then you can throw back into their faces their own words. Also if they know what the heck needs to be done then why have they not come out with the chat as theirs?

[clight77]

After all their recommendations, they could almost as easily changed what was needed and sent it back to you.... lol
At least they explained what was needed fairly concisely...

[spaceace]

yep... fairly simple changes

i also have it so when they review it, they are not allowed to change anything in it as that is what happened when i submitted my Prospace style... they changed what they thought needed to be changed to fix a minor html issue and screwed up my menu but released the messed up version. i had even contacted the styles team telling them they screwed it up but they wouldn't fix it

so, me and the guy helping will fix everything so they can't mess it up

[spaceace]

almost have everything done to pass validation as can be seen in my first post. next submission to the CDB will be with quote and edit functions in place as well as the new layout

[sploinker]

Nice work man!

[spaceace]

Nice work man!

thank you