WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 21 months ago

#18268 closed enhancement (fixed)

Add tel, sms, callto and fax protocols to esc_url() and make the array filterable

Reported by: sanchothefat Owned by:
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: Formatting Keywords: has-patch dev-feedback
Focuses: Cc:

Description

There are loads of protocols that people may need to support.

Currently esc_url() obliterates urls that don't match the allowed list _before_ it reaches the 'clean_url' filter.

This means I can't put tel: or callto: links in a menu via the admin.

The attached patch adds in some more useful protocols and makes the array filterable to cover all the protocols listed on http://en.wikipedia.org/wiki/URI_scheme

Attachments (5)

esc_url-protocols.patch (1.9 KB) - added by sanchothefat 3 years ago.
Patch file for esc_url protocols enhancement
esc_url-protocols-2.patch (1.9 KB) - added by sanchothefat 3 years ago.
2nd patch with a few extra protocols including skype
18268.diff (1.9 KB) - added by duck_ 3 years ago.
18268.2.diff (649 bytes) - added by SergeyBiryukov 3 years ago.
18268.3.diff (806 bytes) - added by georgestephanis 22 months ago.

Download all attachments as: .zip

Change History (24)

comment:1 follow-up: scribu3 years ago

+1 on the esc_url_protocols filter, but please add some spacing, as per the coding standards:

http://codex.wordpress.org/WordPress_Coding_Standards#Space_Usage

sanchothefat3 years ago

Patch file for esc_url protocols enhancement

comment:2 in reply to: ↑ 1 sanchothefat3 years ago

Replying to scribu:

+1 on the esc_url_protocols filter, but please add some spacing, as per the coding standards:

http://codex.wordpress.org/WordPress_Coding_Standards#Space_Usage

Thanks scribu, I've updated the patch including the white-space changes across the whole esc_url() function.

comment:3 scribu3 years ago

Cool.

One more thing: next time you upload an updated version of a patch, don't overwrite the old one, to preserve context.

comment:4 sanchothefat3 years ago

Got it. Sorry about that

comment:5 johnbillion3 years ago

I wonder whether it might be worth adding in some of the more popular proprietary protocols. eg. 'skype:'? Might be opening a can of worms though.

comment:6 sanchothefat3 years ago

@john My patch has a filter on the allowed protocols so if you wanted to you could add it. Something like skype links would be ok I think, it's widely used enough to warrant it. I should probably add im: and some others too.

comment:7 sanchothefat3 years ago

There's steam: teamspeak: git: spotify: gtalk: javascript: cvs: facetime: - there's even one for secondlife!

If I add an updated patch which additional protocols should be supported out of the box? Check the wikipedia link in the initial report for a reference.

sanchothefat3 years ago

2nd patch with a few extra protocols including skype

comment:8 ocean903 years ago

  • Milestone changed from Awaiting Review to 3.3

+1 for only a filter.

comment:9 jb5103 years ago

  • Cc jbrown510@… added

comment:10 ryan3 years ago

In IRC we discussed putting the protocols array in a wp_allowed_protocols() function and using the kses_allowed_protocols filter on it. wp_kses() and esc_url() would use this function. The function can make the array static and perform the filtering only once, avoiding thousands of apply_filters() calls.

duck_3 years ago

comment:11 duck_3 years ago

In [18826]:

Introduce wp_allowed_protocols() for use in wp_kses() and esc_url(). See #18268.

This allows plugins to filter the list of protocols used for esc_url() too, and helps us keep the list of protocols in sync.

comment:12 scribu3 years ago

On a related note: why is wp_parse_args() used on a non-associative array in wp_kses()?

comment:13 follow-up: ryan3 years ago

The use of wp_parse_args() is indeed odd. $allowed_protocols should override the default when not empty.

SergeyBiryukov3 years ago

comment:14 in reply to: ↑ 13 SergeyBiryukov3 years ago

Replying to ryan:

$allowed_protocols should override the default when not empty.

Done in 18268.2.diff.

comment:15 ryan3 years ago

In [18856]:

Don't use wp_parse_args() on non associative array. Props SergeyBiryukov. see #18268

comment:16 ryan3 years ago

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

georgestephanis22 months ago

comment:18 follow-up: georgestephanis22 months ago

Just added a modification of the wp_allowed_protocols() function -- as-is, if you call the function once, say, in a plugin for whatever reason, the static is set and the filter triggers, and no other plugins / theme stuff / whatever will be able to add a filter, as empty() will always be false.

I like optimizing things as much as the next guy and caching it with static, but we could potentially be caching bad data.

Perhaps leave it as a static, and call doing_it_wrong() if they activate it before a certain point, to ensure everyone at the table has an opportunity to attach filters?

Version 0, edited 22 months ago by georgestephanis (next)

comment:19 in reply to: ↑ 18 SergeyBiryukov21 months ago

Replying to georgestephanis:

as-is, if you call the function once, say, in a plugin for whatever reason, the static is set and the filter triggers, and no other plugins / theme stuff / whatever will be able to add a filter, as empty() will always be false.

This might be worth a new ticket. Related: #21299

Note: See TracTickets for help on using tickets.