Skip to:
Content

BuddyPress.org

Opened 6 years ago

Last modified 6 years ago

#8047 new defect (bug)

Some profile fields create empty links

Reported by: shanebp's profile shanebp Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version: 4.1.0
Component: Extended Profile Keywords: needs-patch good-first-bug
Cc: shane@…

Description (last modified by shanebp)

The phone number and url fields create empty links if there is no data for that user.
[ Other fields may also do that. ]

For example, create a Phone Number profile field and set autolink to disabled.
Leave the field empty for all users.
Then add this to bp-custom, after adjusting the args for xprofile_get_field_data:

function xprofile_link_test() {
	
	$field_value = xprofile_get_field_data( 3, 1, $multi_format = 'comma' );
	
	var_dump( $field_value ); // string(36) "" 
		
	echo '<br>';
		
	echo 'field value: ' . $field_value;

	// the fields looks empty, but is really:
        // <a href="tel://" rel="nofollow"></a>
		
}
add_action ( 'wp_head', 'xprofile_link_test' );

As you can see, it is impossible to test for an empty field value.
And that can cause layout and formatting issues.

The same thing happens in the members-loop.
Switch the hook in the snip above from :

add_action ( 'wp_head', 'xprofile_link_test' );

to:

add_action ( 'bp_directory_members_item', 'xprofile_link_test' );

Questions:

  1. Why are empty links being created ?
  1. Why is the autolink setting being ignored for some fields? If there is field data for a member, that field is displayed as a link regardless of the state of the autolink setting.

Attachments (1)

8047.patch (3.4 KB) - added by shanebp 6 years ago.

Download all attachments as: .zip

Change History (4)

#1 @shanebp
6 years ago

  • Description modified (diff)

#2 @shanebp
6 years ago

This patch solves the issues re the creation of empty links and the skipping of checking the do_autolink value of a profile field.

I was unable to determine a consistent BP approach to checking the value of do_autolink.
It seems to be skipped sometimes and perhaps varies depending on whether the field is being viewed in a profile loop or via xprofile_get_field_data.
It seems like a code tangle.

iow. While this patch works, the core devs may want to approach it differently.

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

@shanebp
6 years ago

#3 @boonebgorges
6 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Awaiting Contributions

Hi @shanebp - Thanks for diagnosing the problem in detail, and for proposing a fix.

Regarding empty fields - you are totally correct that we should not be outputting empty links. If the field value is '', we should just be outputting an empty string.

Regarding do_autolink. This setting is specifically intended to toggle the behavior where BP links the field value to a member directory search on that term. In other words, if the field is 'City', and I enter 'Chicago', then do_autolink=true would result in 'Chicago' being linked to /members/?members_search=Chicago. The text of the Dashboard setting is: On user profiles, link this field to a search of the Members directory, using the field value as a search term:.

URL and telephone fields are different. Neither links to a directory search. URLs link to the intended URL, and telephone fields link to the tel:// protocol - primarily so that they become links on mobile browsers. In both cases, I'd argue that the behavior is far more straightforward and generally desirable than the "autolink" behavior that do_autolink focuses on.

So, I don't think it makes sense to obey do_autolink here. However, I do think that the following probably makes sense:

  1. Don't show the do_autolink dialog for field types where it doesn't make any sense.
  2. Introduce filters that allow the linking behavior to be disabled for the URL and Telephone field types.
  3. If we think that there's widespread need for it, we could introduce a metabox similar to the do_autolink metabox, but customized for each of the field types: "On user profiles, turn this field value into a clickable telephone link" or whatever.

a and b seem like good changes and easy wins. I'm not convinced that c is worth the effort if we have the filters, but if someone wrote the patch, I guess it would be OK to include it :)

Note: See TracTickets for help on using tickets.