Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 2 years ago

#8619 closed defect (bug) (fixed)

Undefined variable when handle avatar crop

Reported by: oztaser's profile oztaser Owned by: imath's profile imath
Milestone: 10.1.0 Priority: normal
Severity: normal Version: 10.0.0
Component: Core Keywords: has-patch
Cc:

Description

I noticed this problem when I was doing a detailed review after BuddyPress 10 release.

There is no variable called $r in bp_members_screen_change_avatar() function. It should be $args. Commit https://buddypress.trac.wordpress.org/changeset/13177/trunk/src/bp-members/screens/change-avatar.php

I don't see any problem with in Legacy and Nouveau template pack. We can fix this in 10.1 if I don't missing something important.

Change History (7)

#1 @oztaser
2 years ago

BTW I wish I had seen this problem sooner but I was testing new release as an end user. I noticed the problem when I was doing full code review for production release.

Patch:

diff --git a/src/bp-members/screens/change-avatar.php b/src/bp-members/screens/change-avatar.php
index 6a421ec77..8a64dd95e 100644
--- a/src/bp-members/screens/change-avatar.php
+++ b/src/bp-members/screens/change-avatar.php
@@ -62,7 +62,7 @@ function bp_members_screen_change_avatar() {
                );

                // Handle crop.
-               $cropped_avatar = bp_core_avatar_handle_crop( $r, 'array' );
+               $cropped_avatar = bp_core_avatar_handle_crop( $args, 'array' );

                if ( ! $cropped_avatar ) {
                        bp_core_add_message( __( 'There was a problem cropping your profile photo.', 'buddypress' ), 'error' );
Last edited 2 years ago by oztaser (previous) (diff)

#2 @imath
2 years ago

  • Keywords has-patch added
  • Version set to 10.0.0

Good catch and thanks for finding this early!

No problemo, we'll fix this asap.

#3 @oztaser
2 years ago

I've found an another undefined variable usage in bp_nouveau_ajax_post_update() function. I think $group_id should be $item_id.

diff --git a/src/bp-templates/bp-nouveau/includes/activity/ajax.php b/src/bp-templates/bp-nouveau/includes/activity/ajax.php
index 0043e9d4c..6e81f3a88 100644
--- a/src/bp-templates/bp-nouveau/includes/activity/ajax.php
+++ b/src/bp-templates/bp-nouveau/includes/activity/ajax.php
@@ -552,7 +552,7 @@ function bp_nouveau_ajax_post_update() {
                                if ( ! empty( $bp->groups->current_group->status ) ) {
                                        $status = $bp->groups->current_group->status;
                                } else {
-                                       $group  = groups_get_group( array( 'group_id' => $group_id ) );
+                                       $group  = groups_get_group( array( 'group_id' => $item_id ) );
                                        $status = $group->status;
                                }

#4 @imath
2 years ago

In 13219:

Members: make sure pre-2.3 avatar crop handler is using the right args

Before BuddyPress 2.3 was released, the xprofile_screen_change_avatar() screen handler (which has been deprecated in favor of the bp_members_screen_change_avatar() in version 6.0) used to process the avatar crop using an array built out of posted variables. During the 6.0 deprecation process, this variable has been wrongly named $r instead of $args. This commit is replacing the wrong variable name by the right one.

Props oztaser

See #8619 (branch 10.0)

#5 @imath
2 years ago

In 13220:

Members: make sure pre-2.3 avatar crop handler is using the right args

Before BuddyPress 2.3 was released, the xprofile_screen_change_avatar() screen handler (which has been deprecated in favor of the bp_members_screen_change_avatar() in version 6.0) used to process the avatar crop using an array built out of posted variables. During the 6.0 deprecation process, this variable has been wrongly named $r instead of $args. This commit is replacing the wrong variable name by the right one.

Props oztaser

See #8619 (trunk)

#6 @imath
2 years ago

In 13221:

BP Nouveau: use right variable names in the post_update Ajax handler

Props oztaser

See #8619 (branch 10.0)

#7 @imath
2 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 13222:

BP Nouveau: use right variable names in the post_update Ajax handler

Props oztaser

Fixes #8619 (trunk)

Note: See TracTickets for help on using tickets.