Opened 6 years ago
Last modified 6 years ago
#8047 new defect (bug)
Some profile fields create empty links
Reported by: | 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 )
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:
- Why are empty links being created ?
- 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)
Change History (4)
#2
@
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.
#3
@
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:
- Don't show the
do_autolink
dialog for field types where it doesn't make any sense. - Introduce filters that allow the linking behavior to be disabled for the URL and Telephone field types.
- 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 :)