Skip to:
Content

BuddyPress.org

Opened 3 months ago

Closed 3 months ago

#9052 closed defect (bug) (fixed)

Duplicate "page on front" option values when BP Classic Add-on is activated

Reported by: emaralive's profile emaralive Owned by: imath's profile imath
Milestone: 12.1.0 Priority: normal
Severity: normal Version: 12.0.0
Component: Core Keywords: has-patch has-screenshots commit
Cc:

Description

Environment

WordPress:
Version: 6.5-alpha-57235

Parent Theme:
Name : Twenty Eleven (twentyeleven)
Version: 4.5

Server:
PHP version: 7.4.33 (Supports 64bit values)

BuddyPress:
Version : 14.0.0-alpha
Active components : Community Members, Extended Profiles, Account Settings, Friend Connections, Private Messaging, Activity Streams, Notifications, User Groups, Site Tracking
Active template pack: BuddyPress Nouveau 14.0.0-alpha
URL Parser : Legacy Parser (used to illustrate issue [2nd result], to test patch and illustrate final (3rd) result)
URL Parser : BP Rewrites API (used to illustrate 1st result and to test patch)


The issue can be seen by having the BP Classic Add-on activated and viewing either the "Reading Settings" screen (Dashboard > Settings > Reading) or the "Homepage Settings" panel in Customizer (Dashboard > Appearance > Customize > Homepage Settings) and then click the "static page" radio button and then click the "Hompage" dropdown box. However, the following is the option values with the BP Classic Add-on deactivated (see screenshot w/o patch BP Classic deactivated):

wp_dropdown_pages(array('name' => 'page_on_front','echo' => 0,'show_option_none' => __( '— Select —' ),'option_none_value' => '0','selected' => get_option( 'page_on_front' ),))
=> string(375) "<select name='page_on_front' id='page_on_front'>
	<option value="0">&mdash; Select &mdash;</option>
	<option class="level-0" value="45">Activity</option>
	<option class="level-0" value="55">Blog</option>
	<option class="level-0" value="49">Groups</option>
	<option class="level-0" value="47">Members</option>
	<option class="level-0" value="2">Sample Page</option>
</select>

The following is when the BP Classic Add-on is activated, notice that Activity, Groups and Members option values are duplicated (see screenshot w/o patch BP Classic activated):

wp_dropdown_pages(array('name' => 'page_on_front','echo' => 0,'show_option_none' => __( '&mdash; Select &mdash;' ),'option_none_value' => '0','selected' => get_option( 'page_on_front' ),))
=> string(642) "<select name='page_on_front' id='page_on_front'>
	<option value="0">&mdash; Select &mdash;</option>
	<option class="level-0" value="51">Activate</option>
	<option class="level-0" value="45">Activity</option>
	<option class="level-0" value="45">Activity</option>
	<option class="level-0" value="55">Blog</option>
	<option class="level-0" value="49">Groups</option>
	<option class="level-0" value="49">Groups</option>
	<option class="level-0" value="47">Members</option>
	<option class="level-0" value="47">Members</option>
	<option class="level-0" value="53">Register</option>
	<option class="level-0" value="2">Sample Page</option>
</select>

Finally, with patch applied and the BP Classic Add-on activated, the option values are what should have been initially expected (see screenshot w/ patch BP Classic activated)):

wp_dropdown_pages(array('name' => 'page_on_front','echo' => 0,'show_option_none' => __( '&mdash; Select &mdash;' ),'option_none_value' => '0','selected' => get_option( 'page_on_front' ),))
=> string(483) "<select name='page_on_front' id='page_on_front'>
	<option value="0">&mdash; Select &mdash;</option>
	<option class="level-0" value="51">Activate</option>
	<option class="level-0" value="45">Activity</option>
	<option class="level-0" value="55">Blog</option>
	<option class="level-0" value="49">Groups</option>
	<option class="level-0" value="47">Members</option>
	<option class="level-0" value="53">Register</option>
	<option class="level-0" value="2">Sample Page</option>
</select>

The concept of the patch is to remove the filter for 'get_pages' with the callback to the function bp_core_include_directory_on_front, when the 'legacy' BuddyPress URL Parser is in use thus, preventing the duplication of the Activity, Groups & Members 'page on front' option values.

Attachments (5)

9052.patch (1.4 KB) - added by emaralive 3 months ago.
screenshot-win10-me-2023.12.20-06_42_56.png (68.0 KB) - added by emaralive 3 months ago.
screenshot w/o patch BP Classic deactivated
screenshot-win10-me-2023.12.20-07_03_24.png (83.7 KB) - added by emaralive 3 months ago.
screenshot w/o patch BP Classic activated
screenshot-win10-me-2023.12.20-07_21_43.png (70.1 KB) - added by emaralive 3 months ago.
screenshot w/ patch BP Classic activated
9052.01.patch (1.0 KB) - added by emaralive 3 months ago.
Revised Patch

Download all attachments as: .zip

Change History (12)

@emaralive
3 months ago

@emaralive
3 months ago

screenshot w/o patch BP Classic deactivated

@emaralive
3 months ago

screenshot w/o patch BP Classic activated

@emaralive
3 months ago

screenshot w/ patch BP Classic activated

#1 follow-up: @imath
3 months ago

  • Milestone changed from Awaiting Review to 12.1.0

Great catch @emaralive 👍.

My first feedbacks are:

  1. I wonder if this filter should be removed from the BP Classic plugin instead.
  2. If we need to do it from BuddyPress Core, I would only filter get_pages if the URL Parser is set to rewrites instead of adding a new filter to remove another one.

I'd prolly go with option 2.

#2 in reply to: ↑ 1 @emaralive
3 months ago

@imath

  1. I wonder if this filter should be removed from the BP Classic plugin instead.

I'm chuckling (at myself) because this was my initial thought, I had modified the function bp_classic_use_legacy_parser (bp-classic/inc/core/filters.php) to remove the filter but then opted not to do that because it would cause a new release of the BP Classic plugin and since there is going to be a 12.1.0 release of BuddyPress, try and to it there/here instead. Although, you would probably do it at some other place in the BP Classic plugin.

  1. If we need to do it from BuddyPress Core, I would only filter get_pages if the URL Parser is set to rewrites instead of adding a new filter to remove another one.

I'm struggling with this option, not sure how I would do this since the 'get_pages' filter is added before the function bp_core_get_query_parser is defined, e.g., add_filter( 'get_pages', 'bp_core_include_directory_on_front', 10, 2 ); is found on line 1017 of bp-core/bp-core-filters.php. But then, I'm most likely, not seeing what you are seeing; IOW, it's not that obvious to me (not grasping what you have conveyed).

Now as a counter-offer, I can see how I could modify the conditional (checks if the "legacy' parser is in use) to remove the "get_pages" filter within the function bp_core_setup_query_parser found on line 89 of bp-core/bp-core-actions.php.

Other than that, you will have to enlighten me as to your 2nd option or if need be I'm OK with you applying a remedy of your own discretion because this is still a learning experience for me.

#3 @imath
3 months ago

Thanks for looking into each option and for your detailed explanation. I think that when the bp_core_include_directory_on_front() will be used by our filter, bp_core_get_query_parser() should be available.

@emaralive Could you test adding this kind of check at the very beginning of the bp_core_include_directory_on_front() function's code?

function bp_core_include_directory_on_front( $pages = array(), $args = array() ) {
	if ( 'rewrites' !== bp_core_get_query_parser() ) {
		return $pages;
	}

I believe it's the fix we should include into 12.1.0.
Thanks in advance.

@emaralive
3 months ago

Revised Patch

#4 @emaralive
3 months ago

@imath Now option 2 makes a whole lot of sense (laughing at myself, once again), thanks for the enlightenment!! And, yes, your code works like a charm. I've attached a new patch for your review (9052.01.patch).

#5 @imath
3 months ago

  • Keywords commit added

Awesome!

Thanks a lot for your great work on this. I'll commit your new patch asap 👍

#6 @imath
3 months ago

In 13687:

Add directory post type entries to "page on front" choices if needed

When the rewrites URL parser is in use, to preserve the possibility to use a BuddyPress directory as the site's front page, we need to append the corresponding post type IDs to the available pages listed into the WP "page on front" reading setting as BP Directories are stored as buddypress post types instead of the page one.

On the other hand, when the legacy URL parser is in use (eg: BP Classic is active), to prevent duplicates, it's important to leave the "page on front" reading setting choices the way WP sets it as BP Directories are stored as page post types.

Props emaralive

See #9052 (trunk)

#7 @imath
3 months ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 13688:

Add directory post type entries to "page on front" choices if needed

When the rewrites URL parser is in use, to preserve the possibility to use a BuddyPress directory as the site's front page, we need to append the corresponding post type IDs to the available pages listed into the WP "page on front" reading setting as BP Directories are stored as buddypress post types instead of the page one.

On the other hand, when the legacy URL parser is in use (eg: BP Classic is active), to prevent duplicates, it's important to leave the "page on front" reading setting choices the way WP sets it as BP Directories are stored as page post types.

Props emaralive

Fixes #9052 (branch 12.0)

Note: See TracTickets for help on using tickets.