Make WordPress Core

Opened 13 years ago

Closed 12 years ago

Last modified 8 years ago

#18268 closed enhancement (fixed)

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

Reported by: sanchothefat's profile 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 13 years ago.
Patch file for esc_url protocols enhancement
esc_url-protocols-2.patch (1.9 KB) - added by sanchothefat 13 years ago.
2nd patch with a few extra protocols including skype
18268.diff (1.9 KB) - added by duck_ 12 years ago.
18268.2.diff (649 bytes) - added by SergeyBiryukov 12 years ago.
18268.3.diff (806 bytes) - added by georgestephanis 12 years ago.

Download all attachments as: .zip

Change History (25)

#1 follow-up: @scribu
13 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

@sanchothefat
13 years ago

Patch file for esc_url protocols enhancement

#2 in reply to: ↑ 1 @sanchothefat
13 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.

#3 @scribu
13 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.

#4 @sanchothefat
13 years ago

Got it. Sorry about that

#5 @johnbillion
13 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.

#6 @sanchothefat
13 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.

#7 @sanchothefat
13 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.

@sanchothefat
13 years ago

2nd patch with a few extra protocols including skype

#8 @ocean90
13 years ago

  • Milestone changed from Awaiting Review to 3.3

+1 for only a filter.

#9 @jb510
13 years ago

  • Cc jbrown510@… added

#10 @ryan
12 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_
12 years ago

#11 @duck_
12 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.

#12 @scribu
12 years ago

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

#13 follow-up: @ryan
12 years ago

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

#14 in reply to: ↑ 13 @SergeyBiryukov
12 years ago

Replying to ryan:

$allowed_protocols should override the default when not empty.

Done in 18268.2.diff.

#15 @ryan
12 years ago

In [18856]:

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

#16 @ryan
12 years ago

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

#18 follow-ups: @georgestephanis
12 years 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 12 years ago by georgestephanis (next)

#19 in reply to: ↑ 18 @SergeyBiryukov
12 years 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

#20 in reply to: ↑ 18 @SergeyBiryukov
8 years 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.

Follow-up: #36033

Note: See TracTickets for help on using tickets.