Change second parameter in page_header function

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
User avatar
RMcGirr83
Registered User
Posts: 360
Joined: Fri Mar 09, 2007 1:51 am
Contact:

Change second parameter in page_header function

Post by 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]
Last edited by RMcGirr83 on Fri Feb 22, 2013 2:22 pm, edited 1 time in total.
Do not hire Christian Bullock he won't finish the job and will keep your money


User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: [RFC] change second parameter in page_header function

Post by 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)

User avatar
RMcGirr83
Registered User
Posts: 360
Joined: Fri Mar 09, 2007 1:51 am
Contact:

Re: Change second parameter in page_header function

Post by RMcGirr83 »

Due to the lack of response, I assume that this RFC is going no where?
Do not hire Christian Bullock he won't finish the job and will keep your money

User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: Change second parameter in page_header function

Post by 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

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: Change second parameter in page_header function

Post by bantu »

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

User avatar
RMcGirr83
Registered User
Posts: 360
Joined: Fri Mar 09, 2007 1:51 am
Contact:

Re: Change second parameter in page_header function

Post by 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?
Do not hire Christian Bullock he won't finish the job and will keep your money

User avatar
RMcGirr83
Registered User
Posts: 360
Joined: Fri Mar 09, 2007 1:51 am
Contact:

Re: Change second parameter in page_header function

Post by RMcGirr83 »

El bumpo and given this https://area51.phpbb.com/phpBB/viewtopi ... 93#p254593 possible to get this included?
Do not hire Christian Bullock he won't finish the job and will keep your money

User avatar
RMcGirr83
Registered User
Posts: 360
Joined: Fri Mar 09, 2007 1:51 am
Contact:

Re: Change second parameter in page_header function

Post by RMcGirr83 »

Okay, never mind. No response to me means "move along, we don't care".
Do not hire Christian Bullock he won't finish the job and will keep your money

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: Change second parameter in page_header function

Post by bantu »

RMcGirr83 wrote: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
RMcGirr83 wrote:Okay, never mind. No response to me means "move along, we don't care".
Send a patch against develop and we'll merge it.

Post Reply