Audit use of extract( $r, EXTR_SKIP )

[dontdream]

In 'bp-xprofile-template.php', function bp_has_profile:

Only show empty fields if we're on the Dashboard, or we're on a user's profile edit page, or this is a registration page

The problem is that a profile search plugin I wrote for the Members page, BP Profile Search, needs to show also empty fields in its search form, so this is a blocker for my plugin.

Could you please fix this? Thanks!

[boonebgorges]

If by "fix this" you mean revert the change, then no :) It was done for a reason, which has to do with zebra striping as well as some performance issues.

We introduced a new parameter in bp_has_profile(), 'show_empty_fields'. If you need to show those fields, then pass this parameter with the value true.

If this doesn't work, please share a bit more information about how your plugin works. It's possible that we could put a filter in there to help.

[dontdream]

Thanks for your reply.

I found the 'hide_empty_fields' parameter in bp_has_profile(), but it doesn't work because it's overridden inside the function, effectively ignoring the value received by the caller.

[boonebgorges]

That shouldn't be the case. That test at the beginning of bp_has_profile() ​http://buddypress.trac.wordpress.org/browser/trunk/bp-xprofile/bp-xprofile-template.php#L156 only sets the *default* value for show_empty_fields. In other words, that logic only matters if you *don't* pass an explicit show_empty_fields value. Whatever you pass to the function will override this default.

Give it a try and let me know if it's successful.

[DJPaul]

Boone means hide_empty_fields instead of show_empty_fields, I'm sure :)
Also there's hide_empty_groups which you may also want to try setting, too.

[boonebgorges]

er, yeah - what DJPaul said :)

[dontdream]

I found why bp_has_profile() ignores the hide_empty_fields parameter it receives, that's because inside the function it uses:

extract( $r, EXTR_SKIP );

instead of:

extract( $r, EXTR_OVERWRITE );

Since $hide_empty_fields already exists, it is not extracted.

Sorry I didn't realize this before.

[DJPaul]

Good catch! We have a lot of EXTR_SKIP in BP. We should audit them all.

[boonebgorges]

Yes, good catch! To be extra safe, we should probably use EXTR_OVERWRITE in addition to different variable names when setting up function defaults, eg $hide_empty_fields_default

[DJPaul]

I'm not sure how useful changing the default variable names would be. Also, as overwrite is the default for the extract function, I think this should be a relatively simple change -- I'll take a look tomorrow unless i'm beaten to it

[boonebgorges]

When you set the default with a value like $hide_empty_fields, and then use extract( $r, EXTR_SKIP );, $r['hide_empty_fields'?] will not be extracted, but $hide_empty_fields will retain its previous value. If we leave out EXTR_SKIP, it's irrelevant - but in any case it seems like bad practice to use the same variable name, in the same function scope, for two different purposes.

[DJPaul]

More I think about this, the issue is the default name collision as Boone's eloquently described, rather than SKIP vs OVERWRITE. Working on patch.

[djpaul]

(In [4958]) Correct handling of default value of the profile field loop's hide_empty_fields.
Also correct potential PHP warnings.
Fixes #3447