Skip to:
Content

BuddyPress.org

Opened 16 months ago

Closed 16 months ago

Last modified 16 months ago

#7905 closed defect (bug) (fixed)

Upload photos broken in older bp-default themes

Reported by: r-a-y Owned by: r-a-y
Milestone: 3.2.0 Priority: normal
Severity: normal Version: 3.0.0
Component: Core Keywords: has-patch
Cc: hnla

Description

On new installations, bp-nouveau is the default template pack.

However, for themes still using a version of bp-default, attempting to upload a photo results in a fatal error:

PHP Fatal error:  Uncaught Error: Call to undefined function bp_nouveau_user_feedback() in \wp-content\plugins\buddypress\bp-templates\bp-nouveau\buddypress\assets\_attachments\avatars\index.php:38
Stack trace:
#0 \wp-includes\template.php(690): require()
#1 \wp-content\plugins\buddypress\bp-core\bp-core-template-loader.php(155): load_template('..', false)
#2 \wp-content\plugins\buddypress\bp-core\bp-core-template-loader.php(61): bp_locate_template(Array, true, false)
#3 \wp-content\plugins\buddypress\bp-core\bp-core-attachments.php(993): bp_get_template_part('assets/_attachm...')
#4 \wp-content\plugins\buddypress\bp-core\bp-core-avatars.php(2025): bp_attachments_get_template_part('avatars/index')
#5 \wp-includes\class-wp-hook.php(286): bp_avatar_template_check('')
#6 \wp-includes\class-wp in \wp-content\plugins\buddypress\bp-templates\bp-nouveau\buddypress\assets\_attachments\avatars\index.php on line 38

There are three ways to address this:

1) Ask bp-default theme developers to add add_theme_support( 'buddypress-use-legacy' ) to their functions.php, but this requires theme developers to patch their themes.

2) Patch up bp-nouveau to work with bp-default themes. Don't know how much work this will be.

3) Patch up bp_attachments_get_template_part() to call on bp-legacy if the theme is using an older bp-default theme.

I opted for option 3, but can I get some feedback from @imath about this?

Attachments (1)

7905.01.patch (1.4 KB) - added by r-a-y 16 months ago.

Download all attachments as: .zip

Change History (7)

@r-a-y
16 months ago

#1 @imath
16 months ago

Ouch 😳 Thanks a lot for your patch @r-a-y 👍

I think I’d add the _attachments templates to BP Default if it’s possible. It’s making me feel weird to create a dependency we risk to forget later, unless BP Legacy is eternal! In this case we should probably go with your patch.

I wonder if for standalone themes like BP Default for future features we should display an error template instead of trying to look into the active template pack.

#2 @r-a-y
16 months ago

Thanks for your comments, imath.

I think I’d add the _attachments templates to BP Default if it’s possible.

Are you recommending that we copy over bp-legacy's _attachments folder to bp-default? If so, this wouldn't address themes that copied older, bp-default templates into their theme's directory.

It’s making me feel weird to create a dependency we risk to forget later, unless BP Legacy is eternal! In this case we should probably go with your patch.

Yeah, I understand your apprehension, but in the admin area, we continue to have a dependency on bp-legacy ever since the Backbone upload functionality was introduced:
https://buddypress.trac.wordpress.org/browser/tags/3.1.0/src/bp-core/bp-core-attachments.php?marks=981#L968

Also check out #7672 for more about bp-legacy potentially being the fallback template pack, particularly:
https://buddypress.trac.wordpress.org/ticket/7672#comment:11

(Also related: #7116)

I think my patch is the easiest approach to address this problem, but we should definitely not forget about your comments, imath.

Let me know what you think.

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

#3 @imath
16 months ago

I think having to think about BP Default, themes that copied templates from there, BP Legacy and BP Nouveau will become a nightmare to maintain :)

I also think you're right, the immediate trouble should be fixed for 3.2.0. So let's go with your patch 👌

#4 @hnla
16 months ago

  • Cc hnla added

#5 @r-a-y
16 months ago

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

In 12187:

Attachments: Ensure older bp-default themes can still use the Backbone interface.

Since v3.0.0, bp-nouveau is the default template pack. However, for
themes still using a version of the bp-default theme, attempting to
upload results in a fatal error due to a Nouveau function not being
available.

To address this, for bp-default themes, we revert to using the more,
portable bp-legacy attachment template parts.

Props imath, r-a-y.

Fixes #7905 (3.x branch).

#6 @r-a-y
16 months ago

In 12188:

Attachments: Ensure older bp-default themes can still use the Backbone interface.

Since v3.0.0, bp-nouveau is the default template pack. However, for
themes still using a version of the bp-default theme, attempting to
upload results in a fatal error due to a Nouveau function not being
available.

To address this, for bp-default themes, we revert to using the more,
portable bp-legacy attachment template parts.

Props imath, r-a-y.

See #7905 (trunk).

Note: See TracTickets for help on using tickets.