Skip to:
Content

Opened 4 years ago

Closed 4 years ago

Last modified 2 years ago

#5418 closed defect (bug) (fixed)

Theme compatibility - Post type problems

Reported by: chochal Owned by: r-a-y
Milestone: 2.1 Priority: high
Severity: major Version: 1.7
Component: Templates Keywords: has-patch commit
Cc: henry.wright

Description

This is a follow-up to #5301.

Hi, I’m experiencing the same problem using the Stargazer theme from themehybrid, and the problem persists even after applying the fix above. It’s also a free theme that can be found at:
http://themehybrid.com/themes/stargazer

Attachments (3)

5418.01.patch (4.3 KB) - added by r-a-y 4 years ago.
5418.02.patch (948 bytes) - added by r-a-y 4 years ago.
5418.03.patch (6.1 KB) - added by r-a-y 4 years ago.
Updated patch to fix warnings.

Download all attachments as: .zip

Change History (18)

#1 @r-a-y
4 years ago

  • Keywords has-patch added; needs-patch removed

Confirmed.

The problem is due to the way BuddyPress is adding a post type during theme compat that does not exist.

Stargazer uses a is_singular( get_post_type() ) call to determine if it should use the_content() or the_excerpt().

Since our fake post types (bp_members, bp_groups, etc.) do not exist, the is_singular( get_post_type() ) check will always fail.

Attached patch is made against trunk and should fix this.

One potential problem is this will cause the body class to revert to 'page' instead of the fake BP post type. We should probably add our own body classes to emulate this for theme devs taking advantage of this existing behavior.

Last edited 4 years ago by r-a-y (previous) (diff)

@r-a-y
4 years ago

#2 @boonebgorges
4 years ago

Is this going to cause weird problems with people using is_page() as well? What about actually registering post types (that do nothing)?

#3 @r-a-y
4 years ago

  • Cc henry.wright added

Is this going to cause weird problems with people using is_page() as well?

Yes, depending on the use-case. I think henry.wright was doing something with is_page(). Going to CC him on this. If people want to detect BuddyPress pages, they should use is_buddypress().

What about actually registering post types (that do nothing)?

I'm not really a fan of this. Seems like a hack.

#4 @boonebgorges
4 years ago

I'm not really a fan of this. Seems like a hack.

Yes, but all of theme compat is a hack, and registering a dummy post is *definitely* a hack. We're going to have to be inconsistent somewhere, so I'm just trying to narrow down the location to where it'll do the least amount of damage.

I'll be curious to hear what henry.wright says about his use of is_page(). I'm not so concern about those trying to use is_page() to detect BP pages. I'm more concerned about false positives, where is_page() returns true on BP pages when it *shouldn't*.

#5 @r-a-y
4 years ago

Yes, but all of theme compat is a hack

Touché :)

02.patch is a less-all-encompassing "hack", which retains our existing functionality.

It's still susceptible to is_page() calls during the post loop though. Could probably do some toggling during bp_replace_the_content / loop_end similar to the comments_template() fix in #5339.

See comment:7.

Last edited 4 years ago by r-a-y (previous) (diff)

@r-a-y
4 years ago

#6 @henry.wright
4 years ago

I'll be curious to hear what henry.wright says about his use of is_page()

I use is_page() mainly in my functions.php file to enqueue various JavaScript files conditionally (instead of loading the resource site-wide). I also get my money's worth out of is_single(), is_user_logged_in() and quite a few more of these sort of functions.

Regarding false positives, is_page always returns true in BP 1.9.2 when used on a BP component page - but only when I have root profiles enabled.

See this ticket for some background:
https://buddypress.trac.wordpress.org/ticket/5307

#7 @r-a-y
4 years ago

Is this going to cause weird problems with people using is_page() as well?

I'm more concerned about false positives, where is_page() returns true on BP pages when it *shouldn't*.

I should have answered the first question as "No, it will not make a difference".

is_page() is returning true on all BP pages since #5301 where we switched is_archive to is_page to accommodate themes like Thesis and Attitude.

So 01.patch doesn't make a difference to is_page() whether it's applied or not. It does affect the body post classes though.

#8 @boonebgorges
4 years ago

Ah, thanks for the clarification, r-a-y. If we're not going to be messing with the return value of template conditionals like is_page(), and all we really need to worry about is CSS classes, then I say we just go with your first patch (which is functionally very similar to your second one, but more straightforward in the execution I think) and put in some fixes to make sure that the CSS classes are correct.

#9 @DJPaul
4 years ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 2.1

#10 @r-a-y
4 years ago

  • Keywords needs-refresh removed
  • Summary changed from theme compatibility still a problem even after #5301 patch to Theme compatibility - Post type problems
  • Version changed from 1.9.2 to 1.7

03.patch refreshes 01.patch and emulates the post classes before this patch was implemented using the new bp_get_the_post_class() function.

@r-a-y
4 years ago

Updated patch to fix warnings.

This ticket was mentioned in IRC in #buddypress-dev by r-a-y. View the logs.


4 years ago

#12 @DJPaul
4 years ago

  • Keywords commit added

Looks good

This ticket was mentioned in IRC in #buddypress-dev by paulgibbs. View the logs.


4 years ago

#14 @r-a-y
4 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 8682:

Theme compatibility: Fix issues with themes using post type conditionals.

Previously, when BP resets a post for theme compatibility, we set the
'post_type' to a post type that does not exist (eg. 'bp_activity'). For
themes doing conditional template loading based on the post type, BP's
theme compatibility would not kick in due to the non-existent post type.

This commit replaces all 'post_type' values in bp_theme_compat_reset_post()
to 'page' to address this problem.

For backward compatibility, this commit also modifies the post class to
emulate the older post classes prior to this commit resulting from the
'page' post type change.

Fixes #5418.

#15 @DJPaul
2 years ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.