Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#5589 closed defect (bug) (fixed)

Duplicated fields using Buddypress Xprofile Custom Fields Type and custom theme not from buddypress

Reported by: atallos Owned by: djpaul
Milestone: 2.0.1 Priority: normal
Severity: normal Version: 2.0
Component: Extended Profile Keywords: has-patch needs-testing
Cc: wwhoax@…

Description

This is a follow-up to #5220.

Hi, I'm not sure if this is a bug, maybe this is not and I need to rewrite my plugin.

Some users reported in my support forum. They are using themes not from buddypress like TwentyThirteen, TwentyFourteen. The registration form and edit profile form is displaying two fields when they use a custom type from my plugin. The first one is a textbox, the second one is the custom type from my plugin.

I believe the problem is due to function "bp_xprofile_create_field_type" inside bp-xprofile/bp-xprofile-functions.php. The function receive a custom type and return always BP_XProfile_Field_Type_Textbox if the type doesn't exist. Can this function return null? The template must be modified also to avoid trying to display a field from a null class.

If this is not a bug and returning BP_XProfile_Field_Type_Textbox is necessary then I will need to rewrite my plugin.

Thanks anyway!

Attachments (2)

ticket-5589.01.patch (3.4 KB) - added by DJPaul 6 years ago.
5589.02.patch (1.2 KB) - added by boonebgorges 6 years ago.

Download all attachments as: .zip

Change History (22)

#1 follow-up: @DJPaul
6 years ago

  • Keywords reporter-feedback added

Hi;

We caught something similar before release when we were testing it, didn't we -- there must be another situation, probably involving a duplicated or misplaced action.

Were you able to recreate this? Can you give us step-by-step instructions on how we can recreate it?

#2 @ww_hoax
6 years ago

  • Cc wwhoax@… added

It is very easy tor reproduce the issue:

  1. Installation of wp 3.9 (official version)/ localhost XAMPP
  2. Installation of BuddyPress via Plugin Dashboard (add new plugin)
  3. Set up of Buddypress (activation of the components/ new pages for "register" and "activate")
  4. Installation of BuddyPress via Plugin Dashboard (add new plugin)
  5. Installation of Buddypress Xprofile Custom Fields via Plugin Dashboard (add new plugin)
  6. Add new field (Acceptance Checkbox)

and ... see image

https://www.dropbox.com/s/rwkyhnx8tn37aog/Create%20an%20Account%20-%20wp39e%202014-04-25%2020-02-26.png

Many thanks

Last edited 6 years ago by ww_hoax (previous) (diff)

#3 @boonebgorges
6 years ago

Ah, thanks, I didn't catch that this was the publicly availiable plugin http://wordpress.org/plugins/buddypress-xprofile-custom-fields-type/. I've reproduced the issue. This appears to be a case where your plugin is doing all the work of building the necessary markup, which is more than the XProfile_Field_Type base class is expecting. I'll let DJPaul chime in with thoughts before I dig into it too much more.

#4 in reply to: ↑ 1 @atallos
6 years ago

When we were testing, the issue was in wordpress admin, now it's in the frontend and it's happening when you use a theme which is not prepared from buddypress. I'm not sure if this is a bug because you have rewritten all xprofile implementation so maybe this is something my plugin has to change. I'm just asking to be sure if I have to do it.

Replying to DJPaul:

Hi;

We caught something similar before release when we were testing it, didn't we -- there must be another situation, probably involving a duplicated or misplaced action.

Were you able to recreate this? Can you give us step-by-step instructions on how we can recreate it?

Last edited 6 years ago by atallos (previous) (diff)

#5 @DJPaul
6 years ago

Gotcha, thanks. We'll take a look at it, hopefully this week.

#6 @DJPaul
6 years ago

  • Milestone changed from Awaiting Review to 2.0.1

#7 @DJPaul
6 years ago

The previous fix for the admin that we've obliquely referred to was r8232 for #5511. Miguel's plugin hooks to the bp_custom_profile_edit_fields_pre_visibility action, so this bug occurs on the registration and user profile/edit templates.

  • Not sure about using an IF like I did in r8232 to handle this, because we'll cut off the bp_custom_profile_edit_fields_pre_visibility action for however else it's being used by other plugins and themes.
  • Making bp_xprofile_create_field_type() return a textbox as a fallback was a decision made for future compatibility (i.e. a custom profile field type plugin, registered in the 2.0 way, some fields/data created, and then the plugin being removed).
    • I don't know if reality held up in the same way.
    • Implementing the aforementioned IF technique would skip this intention (probably the fix in r8232 already does this, but at least it had a unique action it could call).
  • Maybe creating a new fake profile type (registered correctly in bp_xprofile_get_field_types(), etc) and having that call relevant actions for backwards compatibility would have been a better solution, but I wouldn't want to look into this for a minor point release.

Not sure best way forward. Maybe just the IF solution for 2.0.1, and revisit the more robust technique for 2.1?

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


6 years ago

#9 @DJPaul
6 years ago

  • Cc DJPaul removed
  • Keywords has-patch added; reporter-feedback removed

Per the IRC discussion (see link above) yesterday, we decided to change the textbox fallback of the bp_xprofile_create_field_type() function to stop it printing out default HTML, which then gets doubled-up with plugins that use nearby actions/filters to print their own custom markup. Patch attached for review; it seems to work OK in my quick-ish testing.

#10 @atallos
6 years ago

Great work! It's working now in my test site.

#11 @MrsAngelD
6 years ago

Hello, myself and others are having a bit of a time trying to figure out exactly how to apply this patch, could someone please explain it a little bit for those who aren't adept at patching.

#12 @DJPaul
6 years ago

MrsAngelD -- if you can hold on a day or two, we should have a 2.0.1 release out with this fix included.

#13 @djpaul
6 years ago

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

In 8336:

xProfile: prevent duplicate profile field templating when using pre-2.0 custom field types.

Prior to the xProfile field type overhaul in 2.0, plugins that added custom field types had to do so with a variety of actions.
One such plugin is "Buddypress Xprofile Custom Fields Type" and the changes in 2.0 caused an issue where the plugin's custom
templating was being rendered, alongside BuddyPress' profile field fallback type, the textbox.

BuddyPress uses a fallback field type to handle situations when extra profile types/data are provided by another plugin, and
then that plugin is removed. The original idea was that everything would fallback to rendering a textbox if a field type was
unregistered to give at least some kind of access to that profile field's data, but in practice this has caused a couple of
issues with templating, as this commit addresses.

This change essentially reverts the fallback behaviour to the pre-2.0 implementation: if a custom field type is unknown to
BuddyPress, that field simply won't be rendered.

Fixes #5589

#14 @djpaul
6 years ago

In 8337:

xProfile: prevent duplicate profile field templating when using pre-2.0 custom field types.

Prior to the xProfile field type overhaul in 2.0, plugins that added custom field types had to do so with a variety of actions.
One such plugin is "Buddypress Xprofile Custom Fields Type" and the changes in 2.0 caused an issue where the plugin's custom
templating was being rendered, alongside BuddyPress' profile field fallback type, the textbox.

BuddyPress uses a fallback field type to handle situations when extra profile types/data are provided by another plugin, and
then that plugin is removed. The original idea was that everything would fallback to rendering a textbox if a field type was
unregistered to give at least some kind of access to that profile field's data, but in practice this has caused a couple of
issues with templating, as this commit addresses.

This change essentially reverts the fallback behaviour to the pre-2.0 implementation: if a custom field type is unknown to
BuddyPress, that field simply won't be rendered.

Fixes #5589 for the 2.0 branch

#15 @atallos
6 years ago

Hi, I'm really sorry but I talk before test everything... :(

When you click on save changes in edit profile, the form show this error: "There was a problem updating some of your profile information, please try again.". This happens because the new class you add "BP_XProfile_Field_Type_Placeholder" is working great for the issue of duplicates fields but when buddypress is setting data with "xprofile_set_field_data", first you check with is_valid if the value of the field is valid and if it is one of my custom field, BP_XProfile_Field_Type_Placeholder will return always false. When you have before the Textbox by default this was working because everything will return true in function is_valid.

I believe then you have to implement is_valid method inside BP_XProfile_Field_Type_Placeholder and return always true.

#16 @DJPaul
6 years ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Ah, that makes sense.

#17 @boonebgorges
6 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

5589.02.patch adds a totally general validation regex (so that anything at all should match it). DJPaul, would like your eyes on this.

#18 @DJPaul
6 years ago

Haven't tested, but looks OK.

#19 @boonebgorges
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 8346:

Ensure that the BP_XProfile_Field_Type_Placeholder class has a validation regex

This ensures that any value will pass the is_valid() test for this field type.

Fixes #5589

#20 @boonebgorges
6 years ago

In 8347:

Ensure that the BP_XProfile_Field_Type_Placeholder class has a validation regex

This ensures that any value will pass the is_valid() test for this field type.

Fixes #5589

Note: See TracTickets for help on using tickets.