Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4092 closed defect (bug) (no action required)

Buddypress will show wrong user avatars if they contain -bpfull or -bpthumb anywhere in their filename

Reported by: defunctlife Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.5.4
Component: Core Keywords: needs-patch
Cc:

Description

I think a theme created some avatars in the format of -bpfull-24x24.jpg. Not sure yet where they came from, but it does show a bug.

When Buddypress is checking for user avatars, it will pick whichever was last in the array, in my case it happened to be this file rather than the expected -bpfull.jpg.

In bp-core-avatars.php, line 211:

				// Check for current avatar
				foreach( $avatar_files as $key => $value ) {
					if ( strpos ( $value, $avatar_size )!== false )
						$avatar_url = $avatar_folder_url . '/' . $avatar_files[$key];
				}

it just overwrites the $avatar_url variable with as many matches as it finds.

I did a fix, but I'm sure someone has a better way to do it

				// Check for current avatar
				foreach( $avatar_files as $key => $value ) {				
					$avatar_length = strlen( $avatar_size );					
					$avatar_file_length = -11;
					
					if( $avatar_size == '-bpthumb')
						$avatar_file_length = -12;
									
					if( substr( $value, $avatar_file_length, $avatar_length ) == $avatar_size )
						$avatar_url = $avatar_folder_url . '/' . $avatar_files[$key];
				}

Change History (7)

#1 @johnjamesjacoby
9 years ago

Those avatars are the old file structure, 1.1 and earlier if I recall. What it finds is actually by design, to encourage users to update their avatars and clear the old ones out.

I think this is probably a wontfix.

#2 @defunctlife
9 years ago

Well the problem still exists when themes are creating the different sized avatars without the users even noticing. The theme in question is this: http://themeforest.net/item/salutation-wordpress-buddypress-theme/548199 which is installed on a sub domain with multisite and if you visit a page with a comment option on it, it generates these extra avatars. So there will be hash-bpfull.jpg and whatever this creates, e.g. hash-bpfull-35x35.jpg and when using Buddypress on our main site it's using these other versions of the avatars instead.

In this scenario users would have to clear their avatars after every visit to this other site.

#3 @defunctlife
9 years ago

I removed his filter from the theme and deleted all the extra files so this no longer happens. But you're sure Buddypress should allow it to display any possible file with -bpfull in the name?

#4 @boonebgorges
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Hm. I think that the old-style avatar naming that John is thinking of is -avatar2, -groupavatar-full, etc. See eg https://buddypress.trac.wordpress.org/browser/tags/1.1/bp-core/bp-core-avatars.php#L125 I don't recall a time in BP history when the dimensions of the image were attached to the filename in this way.

I'm guessing that the theme in question is using some of WP's media handling functions that automatically produce multiple versions of an uploaded file at different cropped sizes. Since BP's avatar handling functions don't do this, it must mean that the themeforest theme is doing its own avatar logic - but, strangely, it is putting those uploaded files into BP-specific avatar directories. I don't think it's appropriate for BP to be accounting for this sort of behavior by themes - if themes want to handle their own avatar uploads, they should either conform to BP's naming standards, or they should override BP's avatars altogether by filtering bp_core_fetch_avatar or removing our get_avatar filters.

I'm going to close this ticket as invalid on this logic. defunctlife, if it turns out that I'm misunderstanding the situation, and this really is somehow BP's fault (or indicative of a larger issue than a single rogue theme), please reopen with more details. In the meantime, you can consider simply filtering bp_core_fetch_avatar and using preg_replace() to strip off any trailing dimensions from the filename.

#5 @defunctlife
9 years ago

The theme is indeed creating its own sized avatars, but my question was others could do this down the road and BP would still handle it the same, overwriting the $avatar_url variable with as many files as it finds with "bpfull" or "bpthumb" in the name and using the last one it finds in the loop.

#6 @boonebgorges
9 years ago

others could do this down the road

Right, but my point is (insofar as I have a point) that themes and plugins can do whatever the heck they want, and BP has to draw the line somewhere :) It's just not possible for us to account for every possible weird thing that a theme might do. IMO this straightforwardly a bug in the Salutation theme - obviously, the theme author intended for the avatars to be BP-compatible, but clearly he/she overlooked the fact that there may be naming conflicts in this way. So the real solution is to get it fixed in Salutation.

One patch that I guess I would consider appropriate is to add a break when a match is found:

// Check for current avatar
foreach( $avatar_files as $key => $value ) {
    if ( strpos ( $value, $avatar_size )!== false ) {
        $avatar_url = $avatar_folder_url . '/' . $avatar_files[$key];
        break;
    }
}

Not because it would fix Salutation's bug (though it would, in this case, do so), but because there's no reason to continue to loop through the directory looking for other versions of the avatar once one is found - breaking out of the loop will save a few wasted CPU cycles. This would need testing before implementing, though.

#7 @defunctlife
9 years ago

Thanks, and I agree with you. I pointed the author here, but commented out their avatar creation code.

Note: See TracTickets for help on using tickets.