Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 2 years ago

Last modified 2 years 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 2 years ago.

Download all attachments as: .zip

Change History (7)

@r-a-y
2 years ago

#1 @imath
2 years 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
2 years 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 2 years ago by r-a-y (previous) (diff)

#3 @imath
2 years 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
2 years ago

  • Cc hnla added

#5 @r-a-y
2 years 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
2 years 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.