The comment in url_is_external() is wrong:

<?php
function url_is_external($path) {
 
$colonpos = strpos($path, ':');
 
// Only call the slow drupal_strip_dangerous_protocols() if $path contains a
  // ':' before any / ? or #.
 
return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && drupal_strip_dangerous_protocols($path) == $path;
}
?>

In fact, we only call drupal_strip_dangerous_protocols() if $path does not contain a / ? or # before the first ':'.

A good explanation is in drupal_strip_dangerous_protocols()

      // If a colon is preceded by a slash, question mark or hash, it cannot
      // possibly be part of the URL scheme. This must be a relative URL, which
      // inherits the (safe) protocol of the base document.
Files: 
CommentFileSizeAuthor
#14 1113702-url_is_external_comment.patch754 bytesfranz
PASSED: [[SimpleTest]]: [MySQL] 33,651 pass(es).
[ View ]
#6 1113702-url_is_external_comment.patch755 bytesfranz
PASSED: [[SimpleTest]]: [MySQL] 33,646 pass(es).
[ View ]
#2 1113702-url_is_external_comment.patch756 bytesfranz
PASSED: [[SimpleTest]]: [MySQL] 33,578 pass(es).
[ View ]

Comments

Priority:Normal» Minor

Setting the priority to minor, as it's just a bug in the comments.

Status:Active» Needs review
StatusFileSize
new756 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,578 pass(es).
[ View ]

I'm giving a shot at it.

Status:Needs review» Reviewed & tested by the community

This is ready to go. Thanks, franz!

This review was done at DrupalCamp LA 2011 during a live session. You're famous!

cool. =)

Status:Reviewed & tested by the community» Needs work

+++ b/includes/common.inc
@@ -2197,8 +2197,9 @@ function url($path = NULL, array $options = array()) {
+  // Avoid calling drupal_strip_dangerous_protocols() if there is any ¶

trailing white space

20 days to next Drupal core point release.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new755 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,646 pass(es).
[ View ]

Status:Reviewed & tested by the community» Needs work
Issue tags:-documentation, -Novice

The last submitted patch, 1113702-url_is_external_comment.patch, failed testing.

Status:Needs work» Needs review

#6: 1113702-url_is_external_comment.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 1113702-url_is_external_comment.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+documentation, +Novice

#6: 1113702-url_is_external_comment.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community
Issue tags:+needs backport to D7

White space issue has been fixed.

Status:Reviewed & tested by the community» Needs review

Shouldn't it be 'a URL' instead of 'an URL'?

It seems to depend on how you pronnounce it: http://grammar.ccc.commnet.edu/grammar/abbreviations.htm (Using articles with abbreviations and acronyms) and http://grammar.ccc.commnet.edu/grammar/grammarlogs1/grammarlogs254.htm (Ctrl + F to searh for "an URL")

In my case I should've used "a URL" but I don't know about the standards adopted - if any - for this.

StatusFileSize
new754 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,651 pass(es).
[ View ]

However, my sources could be outdated, I checked a free online dictionary and pronunciation is like "yoo-arr-ell" (and not something ugly as "earl") so it should be "a URL".

Status:Needs review» Reviewed & tested by the community

Simple change, doesn't need testing again.

Version:8.x-dev» 7.x-dev

Committed to 8.x, back to 7.x for webchick.

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 7.x. Thanks!

Status:Fixed» Closed (fixed)
Issue tags:-documentation, -Novice, -needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.