Change second parameter in page_header function

[RMcGirr83]

Currently the page_header function will always run queries to display the who is online listing if the second parameter is not set to false. This affects everything that uses the page_header function. EG, with in functions_posting.php there is this line within the function generate_smilies

page_header($user->lang['SMILIES']);

even though the online list is not displayed in the pop up for the smilies (the same is also true for the report.php file, search.php and others). I would like to recommend that the page_header function second parameter be changed from true to false as I can only find three instances of where the list is displayed and that is within index.php, viewforum.php and viewtopic.php.

In fact in viewonline.php there is a kludge to not display this list

Code: Select all

// We do not need to load the who is online box here. ;)
$config['load_online'] = false;

but when using the whois lookup

Code: Select all

	// Output the page
	page_header($user->lang['WHO_IS_ONLINE']);

This would save at least 3 queries throughout the forum where page_header function is called which is, basically, just about every PHP file.

Granted I am looking at 3.0.11 files, I am not able to look at 3.1 files ATM because my computer is a gigantic steaming turd ATM.

[EDIT]FYI, if accepted I am willing to provide the patch[/EDIT]

[Senky]

+1

Makes sense.

[EXreaction]

Yes, we should not be generating that by default I think.

But instead of just changing the default to false, I would propose a different way to do this:

Move the load online to a new function
rename page_header to phpbb_page_header and remove the load online code
create a compatibility function page_header that has the existing parameters of page_header (in it call the new load online function if required and the new page_header function)

[RMcGirr83]

Due to the lack of response, I assume that this RFC is going no where?

[EXreaction]

If someone creates a PR following the guidelines I wrote above I would merge it (I wouldn't expect any objections to it).

Please make it for develop though and not 3.0.

There is a ticket for it already:
http://tracker.phpbb.com/browse/PHPBB3-11360

[bantu]

I do not have anything against changing the default value for the $display_online_list in 3.1.x.

[RMcGirr83]

I am a bit confused. Nathan, I am not sure why it needs to be that tedious for a simple fix and Andreas states he is not against changing the default value. So which is it to be?

[RMcGirr83]

El bumpo and given this https://area51.phpbb.com/phpBB/viewtopi ... 93#p254593 possible to get this included?

[RMcGirr83]

Okay, never mind. No response to me means "move along, we don't care".

[bantu]

El bumpo and given this https://area51.phpbb.com/phpBB/viewtopi ... 93#p254593 possible to get this included?

No. See http://tracker.phpbb.com/browse/PHPBB3- ... ment-38942

Okay, never mind. No response to me means "move along, we don't care".

Send a patch against develop and we'll merge it.