Skip to:
Content

Opened 6 months ago

Last modified 3 weeks ago

#5212 assigned defect (bug)

Enable upgrade route for BP-Default for future BP releases

Reported by: DJPaul Owned by: boonebgorges
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Theme Keywords: needs-docs
Cc: rachel@…, mercijavier@…, hugoashmore@…

Description

The BuddyPress team and many contributors have previously discussed moving BP-Default out of the main BuddyPress package and onto the WordPress.org/themes repo. When this happens is TBC, but some prep work is required to make the transition as straightforward as possible.

We have two main reasons to do this:

  • After BP 1.9, we plan no further improvements or bug fixes for BP-Default (unless it's a security issue).
  • We'd like to remove BP-Default from the BuddyPress codebase to bring down the size of the plugin, and have a kind-of clean slate for future theme development with the theme compatibility approach that we've adopted.

What needs to happen is to move the theme from inside the BuddyPress plugin folder into the /themes/ folder. Or to put it another way, for 1.9, I suggest that we do not register the /buddypress/bp-themes/ folder if BP-Default exists in /themes/.

The attached patch is an implementation of this. It's a pretty simple change. I couldn't get any of the helpful methods in wp-includes/theme.php to work here, partly because static variables are used to save theme information, and this caused problems with checking/registering the themes here. It looks like it *should* be possible; it might be a load-order issue.

How will this help us? Some point after 1.9 ships, we get BP-Default up on wordpress.org/themes/, and bump the version number. This will cause everyone's WordPresses to see that there's a new version of BP-Default, and hopefully they'll click the update button. WordPress will download that updated theme into /themes/. On next page load, our patch then takes affect, and doesn't register the version of BP-Default bundled with BuddyPress.

At some version in the future, we'd just delete the bundled BP-Default from BuddyPress.

Attachments (1)

theme-registration-001.patch (581 bytes) - added by DJPaul 6 months ago.

Download all attachments as: .zip

Change History (27)

comment:1 boonebgorges6 months ago

  • Milestone changed from Awaiting Review to 2.0

Nice, this seems pretty well thought out. Thanks for starting the conversation here.

Inevitably, there will be path issues - places where people hardcoded (or otherwise incorrectly) inferred the URL and file paths of assets, etc assumed to be in bp-default. I'm not sure there's much we can do about this technically, but it does suggest the requirement of having lots of warning and documentation for end users. I do *not* think there's time to do that for 1.9.

One thing we *could* do now is to bail out of registering the theme directory for anyone who is not currently running bp-default or a child theme. That would prevent new installations from switching to the EOL bp-default. Maybe a separate issue?

What'd be helpful (and here's a chance for non-devs to jump in) is to work up a list of the different scenarios where one would be using bp-default, or assets thereof, and to run a series of tests using DJPaul's patch. Things like: bp-default as active theme, as parent theme, plugins like Genesis Connect that pull assets from bp-default, etc.

Let's slate this for 2.0, and just get it done and over with :)

comment:2 r-a-y6 months ago

One thing we *could* do now is to bail out of registering the theme directory for anyone who is not currently running bp-default or a child theme. That would prevent new installations from switching to the EOL bp-default.

I like this.

What'd be helpful (and here's a chance for non-devs to jump in) is to work up a list of the different scenarios where one would be using bp-default, or assets thereof, and to run a series of tests using DJPaul's patch. Things like: bp-default as active theme, as parent theme, plugins like Genesis Connect that pull assets from bp-default, etc.

Here's some quick code to redirect any asset from /bp-themes/bp-default/ to /themes/bp-default/:
http://pastebin.com/aLhRRs0x

For this code to work, the /bp-themes/bp-default/ asset must not exist and the wp-content/themes/bp-default/ directory must exist. It's better to do this in .htaccess, but thought I'd put up some initial code for now.

Last edited 6 months ago by r-a-y (previous) (diff)

comment:3 follow-up: johnjamesjacoby6 months ago

Paul's patch looks okay to me at a cursory glance.

I agree with Boone that we need to be very clear about breaking changes BuddyPress 2.0 is going to cause outdated installations, both in this regard and also with my wanting to remove old bbPress and/or bp-forums completely.

Paul - I don't think your code will do very much. How often is a *.php file included inside of WordPress in such a way to cause the bp_loaded action to fire? If template files, scripts, or images are loaded directly, this code will never hook in to do anything there either.

Instead of Paul's patch, can we move bp-default into wp-content/themes/bp-default, maybe on update, like the old Template Pack used to do? I'm thinking:

  • Check if the parent/child themes on the root site are using bp-default
  • If so, backup the theme options, and try copy the theme.
  • If the theme is copied successfully, update the appropriate theme option(s) and try to load the site.
  • If the site responds with a 200, delete bp-default from the BuddyPress directory, delete the backed-up options, assume success.
  • If anything fails, revert back to the original configuration and alert the site admin they need to move the theme folder manually.

comment:4 in reply to: ↑ 3 ; follow-up: boonebgorges6 months ago

Replying to johnjamesjacoby:

  • Check if the parent/child themes on the root site are using bp-default
  • If so, backup the theme options, and try copy the theme.
  • If the theme is copied successfully, update the appropriate theme option(s) and try to load the site.
  • If the site responds with a 200, delete bp-default from the BuddyPress directory, delete the backed-up options, assume success.
  • If anything fails, revert back to the original configuration and alert the site admin they need to move the theme folder manually.

I like this as a first pass. A couple of thoughts:

  1. What's the advantage of copying over the files from bp-themes, rather than (a) ensuring that bp-default is available on wp.org/extend on the day 2.0 drops, and then (b) using WP's theme downloader library to force the download? It seems like we *might* need to write less code if we use WP's library, and somehow it seems "right" to download the theme anew rather than copying from within BP.
  2. Related to 1. We have to consider installations where file permissions etc will not allow BP to move files around. Maybe another argument for using as much of the existing WP interface as possible.
  3. I'll have to look at the theme options stuff, but do some of them save theme-directory paths? If so, we'll have to run some processing on the theme options when moving them over.
  4. I think there's not much purpose in actively deleting bp-default out of the buddypress directory. Let's either: ship it with 2.0 and use it as a fallback if the automated migration fails (and don't register the theme directory otherwise; see #5223), or don't ship it with 2.0 at all and force admins to complete the migration before the site is usable. Deleting files on the fly from BP seems sketchy to me, especially in production environments that are under version control, etc.

comment:5 rachelbaker6 months ago

  • Cc rachel@… added

comment:6 in reply to: ↑ 4 johnjamesjacoby6 months ago

Replying to boonebgorges:

  1. What's the advantage of copying over the files from bp-themes, rather than (a) ensuring that bp-default is available on wp.org/extend on the day 2.0 drops, and then (b) using WP's theme downloader library to force the download? It seems like we *might* need to write less code if we use WP's library, and somehow it seems "right" to download the theme anew rather than copying from within BP.
  2. Related to 1. We have to consider installations where file permissions etc will not allow BP to move files around. Maybe another argument for using as much of the existing WP interface as possible.
  3. I'll have to look at the theme options stuff, but do some of them save theme-directory paths? If so, we'll have to run some processing on the theme options when moving them over.
  4. I think there's not much purpose in actively deleting bp-default out of the buddypress directory. Let's either: ship it with 2.0 and use it as a fallback if the automated migration fails (and don't register the theme directory otherwise; see #5223), or don't ship it with 2.0 at all and force admins to complete the migration before the site is usable. Deleting files on the fly from BP seems sketchy to me, especially in production environments that are under version control, etc.
  1. Only advantage is knowing we tried to be helpful I suppose.
  2. Falls under my 'if anything fails' clause.
  3. Falls under my 'update the appropriate theme options' clause.
  4. Agree, and on board.

comment:7 mercime6 months ago

  • Cc mercijavier@… added
  • Keywords needs-docs added

comment:8 boonebgorges5 months ago

In 7569:

Sound the knell for the BuddyPress Default theme

bp-default is being sunsetted as of BP 1.9. The theme will continue to receive
security updates and other critical fixes, but will otherwise no longer be
under active development by the core team.

We maintain backward compatibility with sites currently using bp-default by
continuing to register the bp-themes theme directory when bp-default, or a
child theme thereof, is in use on a given site. Those sites will continue to
be able to use the theme. Installations where bp-default is not already in use
will no longer see it listed on Dashboard > Appearance.

Site administrators or developers who want to override this behavior may do so
with the 'bp_do_register_theme_directory' filter. Please note that bp-themes
will be removed altogether from BuddyPress in a future version, likely migrated
to wordpress.org/extend. See #5212.

Now cracks a noble heart. Good-night, sweet prince;
And flights of angels sing thee to thy rest.

Fixes #5223

comment:9 needle5 months ago

@boonebgorges, just to be clear, does that mean that, for example, if I have a fix/improvement to the BP Javascript, I only need to submit a patch for bp-templates/bp-legacy/buddypress.js and can happily ignore bp-themes/bp-default/_inc/global.js?

comment:10 boonebgorges5 months ago

I only need to submit a patch for bp-templates/bp-legacy/buddypress.js and can happily ignore bp-themes/bp-default/_inc/global.js?

Correct, and happy indeed!

comment:11 ircbot3 months ago

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

comment:12 DJPaul7 weeks ago

I'm still very keen in seeing BP-Default deleted from BuddyPress. I also think my initial patch is still the right way to go; and my opinion is that if someone's hardcoded the URL in some other theme or plugin... tough cookies. That's why JS/CSS in WordPress uses the enqueues system for those assets.

I'd like to re-start discussion on this, perhaps at one of the next dev chats, about if we can add my patch in for 2.0, and at release, get BP-Default into wp.org/themes/, and then bump something trivial inside the wp.org/themes/ BP-Default to get people's sites to download/switch to that version. We'd leave BP-Default in BuddyPress core for the time being (2.0) to see if the above switch is as seamless as I believe it to be.

comment:13 boonebgorges7 weeks ago

I'm still very keen in seeing BP-Default deleted from BuddyPress.

Seems like a waste of perfectly good keenness. Save it for problems that actually exist :)

That said, I'm fine with the strategy you outline. It seems safe enough for 2.0.

comment:14 ircbot6 weeks ago

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

comment:15 hnla6 weeks ago

  • Cc hugoashmore@… added

comment:16 boonebgorges4 weeks ago

In 8158:

Don't register the bp-themes themes directory if bp-default is found elsewhere in the installation

This will be part of the migration path away from the packaged version of
bp-default. bp-default will be added to the wordpress.org theme repository with
a higher version number, prompting an upgrade that will result in an
installation into the standard WP themes directory. Once that happens, most
installations will no longer be reliant on the version of bp-default packaged
with BuddyPress.

See #5212

Props DJPaul

comment:17 boonebgorges4 weeks ago

  • Keywords has-patch removed
  • Milestone changed from 2.0 to Future Release

Let's go with the solution in r8158 for now. After 2.0 is released, we can put bp-default into the theme repo, and take things from there. Moving to Future Release.

comment:18 hnla4 weeks ago

Perhaps not a helpful comment but looking at that patch it seems a shame we look for '/bp-default' and can't perhaps rename or should rename 'bp-default' when it's moved to WP repo; bp-default just seems to suggest this is the de facto theme for BP which it isn't really, moving forward theme compatibility and whatever theme template packs available would be the standard offering most would use.

'/bp-the-theme-what-was-the-one-around-before'

or

'/bp-classic'

Last edited 4 weeks ago by hnla (previous) (diff)

comment:19 boonebgorges4 weeks ago

bp-default just seems to suggest this is the de facto theme for BP which it isn't really

Good point, but WP differentiates themes based on the directory name, so I don't think there's any way around this.

comment:20 hnla4 weeks ago

But does that mean we can't set a new name and have BP core check for that name existing - I'll admit though having trouble seeing the impediments we have to get past in this process, slow brain day.

comment:21 boonebgorges4 weeks ago

  • Owner set to boonebgorges
  • Status changed from new to assigned

Oh, right. Yes, we can do that. If you want to name it, say, buddypress-classic, we can check for the existence of this too.

comment:22 hnla4 weeks ago

Well needs a consensus opinion as to whether this is a good thing to do, to my mind it makes a clean break, states in the name it's a bygone era :) and might be less confusing and fend of those forum posts that are confused as they thought this is the theme they had to install from the repo.

I ought to read the changes in detail but guessing we end up moving bp-default to repo bumping version ( as I think I did read) forcing a request to upgrade to a new version so the issue really is for that to work requires the name staying the same to facilitate a switch over point so likely the name can't be changed that simply which is a shame.

comment:23 DJPaul3 weeks ago

  • Milestone changed from Future Release to 2.0
  • Severity changed from normal to critical

This change (or something related) has caused BP-Default to not appear in the list of themes on new installs.

comment:24 boonebgorges3 weeks ago

bp-default does not appear in the list of themes of new installs by design. See https://buddypress.trac.wordpress.org/changeset/7569

comment:25 DJPaul3 weeks ago

  • Milestone changed from 2.0 to Future Release
  • Severity changed from critical to normal

Cool, I must have missed that. Thanks.

comment:26 hnla3 weeks ago

Added a overview of state of play for bp-default to codex a while back:

http://codex.buddypress.org/themes/bp-default-theme-moving-forward-with-bp-1-9/

Based on:
http://bpdevel.wordpress.com/2013/11/13/the-future-of-the-bp-default-theme/

The only change I thought had happened was the need for the add_filter 'bp_do_register_theme_directory' being needed to restore the theme to selection page on existing installs, seemed to remember that was changed to an automatic process if bp-default was a selected theme but can't find the reference to that.

Note: See TracTickets for help on using tickets.