Custom xprofile field: URL

[sooskriszta]

URL would be a useful field for many communities. General URL validation rules apply.

Would be interesting if we ask about
Twitter, Facebook, Youtube, Github, StackOverflow etc handles and convert them into URLs.

Would need to convert to links when displaying in profile.

[boonebgorges]

I like this idea. Would be useful to almost any BP installation.

I agree that it'd be interesting to attempt validation between user names and certain website types - I've built this already in ​https://github.com/boonebgorges/bp-social-media-profiles - but it adds several layers of complexity. (Not the least of which: which do we include? What's popular in the US is not necessarily in China; what's useful to a group of programmers is not useful to a group of students; etc etc etc)

That said, a more general URL field would be a starting point for a plugin that does what bp-social-media-profiles does, but more elegantly.

[sooskriszta]

Replying to boonebgorges:

which do we include? What's popular in the US is not necessarily in China; what's useful to a group of programmers is not useful to a group of students; etc etc etc)

As you've said elsewhere, we should look at what proportion of BP users will benefit from it.

Is BP used equally frequently in US and China? By programmers and students?

Aside from Russia and China, I trust that Twitter, Facebook and Youtube will be relatively universally useful.

Considering that many BP communities are built for programmers, it might be a good idea to include GitHub and StackOverflow.

Obviously, it should be up to the site admin which particular ones are included on his particular community. (on that note, time to vastly expand BuddyPress's backend/admin UI footprint)

[boonebgorges]

Let's concentrate on getting the plain URL field in place first. That's something that is definitely within the scope of BP core. I have a strong inclination that the inclusion of specific services should be left to a plugin - it's not a good use of core time to come up with a list of services, only to bicker endlessly with users about what we have/have not included - but in any case, it's something we can consider after we're done with the base field.

One kind of validation that might be a better candidate for core inclusion is a sort of regex/search-and-replace URL concatenator. As in, the admin could enter something like '​https://twitter.com/%%VALUE%%/' into a "pattern" field, and then we could have some validation that (a) detects whether the user-provided value is a valid URL, and if not (b) swaps it into the pattern. Getting the UI and logic for this to be solid would be a bit of work, but might be general enough to warrant inclusion in BP itself.

[sooskriszta]

Replying to boonebgorges:

One kind of validation that might be a better candidate for core inclusion is a sort of regex/search-and-replace URL concatenator. As in, the admin could enter something like '​https://twitter.com/%%VALUE%%/' into a "pattern" field, and then we could have some validation that (a) detects whether the user-provided value is a valid URL, and if not (b) swaps it into the pattern.

Sounds like an idea.

[sooskriszta]

Replying to boonebgorges:

something like '​https://twitter.com/%%VALUE%%/' into a "pattern" field

Prefix%%VALUE%%Suffix
e.g.
Prefix= ​http://youtube.com/
Suffix= null
might work.

A freetext pattern field would work even better.

[williamsba1]

Attached is a patch adding a new URL field type. The regex pattern for validating the URL might need some love, but overall the code works as expected.

[boonebgorges]

Awesome start. Thanks williamsba1!

5501.02.patch adds some unit tests that raise a couple questions related to validation and workflow.

1. '​http://foo.com/?bar=baz' was not validating with your regex, because it was missing ? and = in the character whitelist. I added them. But there are probably other special characters that should be included that are not matched by [\/\w \.-\?\=]. %, ', etc. williamsba1 - I'm assuming you lifted this regex from somewhere (though I'd be pleased if you told me you wrote it yourself :) ) - maybe we can find one that's a bit more generous and faithful to the URL spec.
2. I didn't write any tests for it, but it's likely that this is going to fail with non-Latin characters in URLs. Again, a more generous template regex is probably going to help.
3. 'foo.com' is being validated ((https?:\/\/)?). If the ultimate goal is to display this as a link in the profile, then this can't happen. Ideally, we'd be able to detect this sort of thing, and then throw on an '​http://' when the field is saved. Want to have a crack at this?

[sooskriszta]

Solid stuff, @williamsba1

Wanna have a crack at #5500 as well? :-P

[sooskriszta]

Replying to boonebgorges:

maybe we can find one that's a bit more generous and faithful to the URL spec.

So far, I've found @diegoperini's to be the best:

_^(?:(?:https?|ftp)://)(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\x{00a1}-\x{ffff}0-9]+-?)*[a-z\x{00a1}-\x{ffff}0-9]+)(?:\.(?:[a-z\x{00a1}-\x{ffff}0-9]+-?)*[a-z\x{00a1}-\x{ffff}0-9]+)*(?:\.(?:[a-z\x{00a1}-\x{ffff}]{2,})))(?::\d{2,5})?(?:/[^\s]*)?$_iuS

Wanna take it for a spin?

Here's the JS: ​https://gist.github.com/dperini/729294

[sooskriszta]

P.S. Diego's regex works even for non-Latin letters.

[johnjamesjacoby]

Related: #278.

[johnjamesjacoby]

'foo.com' is being validated ((https?:\/\/)?). If the ultimate goal is to display this as a link in the profile, then this can't happen. Ideally, we'd be able to detect this sort of thing, and then throw on an '​http://' when the field is saved. Want to have a crack at this?

I'd love for us to remove the scheme from the HTML output, and keep it in the HREF. WordPress's make_clickable() function might be a good place to start digging, too.

[boonebgorges]

I'd love for us to remove the scheme from the HTML output, and keep it in the HREF

As a rule, I agree that it's prettier to remove the schema. In some cases, though, seeing the schema can be helpful - in particular, when you want to know whether you're clicking through to https or not. But I suppose that can be seen with a browser status bar?

[johnjamesjacoby]

Replying to boonebgorges:

But I suppose that can be seen with a browser status bar?

That's my thought exactly; we can trim off the unpretty scheme in the output. That said, not a deal breaker IMO, just picking nits.

[williamsba1]

Thanks for all the feedback! Patch 5501.03.patch includes the following updates:

1. Included the regex pattern supplied by sooskriszta. This pattern does validate all of the unit test URLs now. I'm not an expert on regex so this is about as far as I can take it.
2. Added a check for the scheme and if it doesn't exist the scheme is added to the URL when the field is saved. I used the code WP uses in esc_url(), so that should be good to go (Props WordPress!)

[boonebgorges]

See #5630

[danbp]

I initially came here to ask for such a field type enhancement and found this ticket.

I use BP on 3 different environement (prod, online dev and local).
Prod and dev sites are two different accounts on the same server (same host, same settings, and so on). My local site is little different in settings, but nothing crucial.

On my production site (MS) i have a normal text field to let users add their website URL. But the field value comes out with a wrong URL: ​http://mysite.com/members/username/profile/?s=http://user_site.com/

On my local test site (single), i use the same field and when i enter a URL, i get it correctly. This field is a normal text field
On my dev site (MS), i use the same field type and i can enter a URL who is also correctly outputed.

I don't know actually why i have these difference. Even theme and plugins are the same on all sites...

Now the patch 5501.03. I applied it first on local site: OK
Then on dev: OK and finally on prod: OK
The only thing i encountered on the prod whas old URL's, entered before patch was applied. I had just to resave the profile and, bam, anything went fine.

Would be great to have such an URL specific field, not only for converting FB or Twitter, but for any other URL too, like the website field on WP profiles.

[boonebgorges]

This ticket prompted the opening of #5630. That ticket has just been resolved, which means that BP_XProfile_Field_Type classes can now provide their own display_filter() method to customize the display of their content. I'd like to suggest a refresh of this ticket that adds a display_filter() method that does the following:

1. Turns URLs into clickable links with nofollow
2. Strips schemes from the text of the links
3. Strips trailingslashes from the URL displayed, but adds them to the href

So

https://buddypress.trac.wordpress.org

and

https://buddypress.trac.wordpress.org

both become:

<a href="https://buddypress.trac.wordpress.org/" rel="nofollow">buddypress.trac.wordpress.org</a>

[johnjamesjacoby]

In 8632:

Introduce URL field type:

• New field class extending BP_XProfile_Field_Type for URLs
• Clean-up existing BP_XProfile_Field_Type extensions (formatting, bp_parse_args() VS array_merge(), whitespace, brackets, etc...)
• More to do here, specifically regarding output and display_filter() methodology.

Props boonebgorges, williamsba1, sooskriszta. See #5501, #5630.

[johnjamesjacoby]

In 8633:

Add unit test for URL field type. Props williamsba1. See #5501.

[johnjamesjacoby]

Patch attached to #5630 to hammer out display_filter() usage:

​https://buddypress.trac.wordpress.org/ticket/5630#comment:8

[boonebgorges]

In 8634:

Register the BP_XProfile_Field_Type in bp_xprofile_get_field_types()

This bit was omitted from r8632, where the URL field type was introduced.

See #5501

[boonebgorges]

Still missing from ​https://buddypress.trac.wordpress.org/attachment/ticket/5501/5501.03.patch: the logic that appends '​http://' to the beginning of a saved value if it's not found. I still think that this is a good thing to include (we are going to have lots of confused users otherwise) but I don't like putting it directly into xprofile_set_field_data().

I'm thinking that the ability to filter a field value before it's saved is probably a fairly common thing that field types will want to do. In the spirit of #5630, it may be appropriate to add this kind of functionality natively in BP_XProfile_Field_Type. Maybe something like a pre_save_filter() method, which is a pass-through by default, and which would be hooked maybe to 'xprofile_data_value_before_save'. DJPaul and others, what do you think?

Alternatively, if we don't want to do this as part of the field type architecture, we could just put the method in the URL class, and then add_filter() as appropriate right in the class constructor.

[johnjamesjacoby]

In 8638:

Add the URL profile field type to bp-xprofile-functions.php. See #5501.

[DJPaul]

I would very much like to see r8638 moved out of xprofile_set_field_data and into a field type class/hooked to a field type class, something like Boone suggests.

It took a lot of effort to remove all the xprofile field type checks that were hardcoded when I worked on this for 2.0. Plus, parsing a URL without using parse_url, and (for some reason) allowing extensions for .php files only, seems a bit off.

To simplify, I suggest we don't support relative paths in the URL field type (at the moment?), and require absolute URLs. We can then use esc_url_raw() to both sanitise (if we're not already) and it will add a http prefix onto the URL if one is missing.

[DJPaul]

Oh, and if that HTTP validation regex wasn't so ridiculously huge and un-readable (it looks like the protocol is optional), having that require a protocol would have been the easy way to fix this. :)

[sooskriszta]

Actually Diego restricts protocol to http, https and ftp
​https://gist.github.com/dperini/729294

[sooskriszta]

In any case on the gist you'll find a commented, readable version.

[boonebgorges]

I would very much like to see r8638 moved out of xprofile_set_field_data and into a field type class/hooked to a field type class, something like Boone suggests.

+1. Though I'm guessing that committing it in the first place was a mistake on jjj's part - the commit message does not accurately describe what the change does.

Oh, and if that HTTP validation regex wasn't so ridiculously huge and un-readable (it looks like the protocol is optional), having that require a protocol would have been the easy way to fix this. :)

That's not really true though. If an input doesn't pass the is_valid() check, it's simply rejected. What r8638 is supposed to do (though I have not tested whether it actually does) is to catch input *before* it is tested by is_valid(), make an educated guess about what the user "meant", and then modify it so that it does pass validation. The motivation here is that people are used to being able to enter URLs without the protocol, so we want to be able to accept it as a valid input even though the value we save always has a protocol. (Also, the regex *does* in fact require a protocol, assuming I read it correctly: in (?:https?|ftp) the ?: means that this is a non-captured group, not that it's optional.)

[johnjamesjacoby]

Agree on r8638. I mostly noticed I'd forgotten to commit that file from the original patch, and missed that Boone had already committed r8634.

[boonebgorges]

In 8670:

Remove URL-specific pre-validation from xprofile_set_field_data()

This validation is intended to catch URL submissions that do not have a scheme
(​http://) attached, and to add it preemptively. However, we want to add this
validation in a more centralized location than here.

See #5501

[boonebgorges]

In 8672:

Introduce BP_XProfile_Field_Type::pre_validate_filter(), and use in URL field type

The pre_validate_filter() method allows field types to intercept field values,
before they are run through the is_valid() validation routine, and optionally
modify them.

BP_XProfile_Field_Type_URL uses this filter to silently add the '​http://'
protocol to submitted values before run through the validation regex (which
requires the protocol). The result is a smoother UX, as users are not required
to enter the protocol in order to have their field value saved.

To make it possible for protocol-less values to be submitted on the profile
edit form, the 'type' attribute of the URL field was changed from 'url' to
'text' (with a 'url' inputtype). With 'type=url', modern browsers require a
fully-qualified URL before allowing the form to be submitted.

Fixes #5501