Issue status update for http://drupal.org/node/10888 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Bèr Kessels Reported by: jhriggs Updated by: Bèr Kessels Status: patch Allthough this works, is clean AND does what it should do, I still am in favour of a better nitegrated solution. So please do not yet commit this, untill I upload my patch. We can then discuss what solution to take. It will not differ a big lot though. Bèr Kessels Previous comments: ------------------------------------------------------------------------ September 16, 2004 - 16:01 : jhriggs Attachment: http://drupal.org/files/issues/full_url.patch (1.97 KB) This little patch allows full URLs to be used for the url() and l() functions, giving modules a single interface for creating links. Currently, modules must use l() for drupal links and raw HTML for external links. This patch changes url() to handle -- basically pass through -- full URLs while still doing the proper handling of internal paths and works with clean URLs enabled or disabled. It also shortens (and hopefully simplifies) the url() code a bit. Some examples of calls that now work: <?php l('a link', 'some/path');  // same as before l('an external link', 'http://drupal.org/'); l('full with query', 'http://google.com/search?q=define:foo'); l('full with separate query', 'http://google.com/search', array(), 'q=define:foo'); // same link as above l('full with fragment', 'http://drupal.org/node/5942#comment-12374'); l('full with separate fragment', 'http://drupal.org/node/5942', array(), NULL, 'comment-12374'); // same link as above ?> ------------------------------------------------------------------------ September 30, 2004 - 12:09 : moshe weitzman +1. less HTML in code is a good thing for readability, if nothing else. ------------------------------------------------------------------------ January 24, 2005 - 21:06 : mathias Attachment: http://drupal.org/files/issues/external_urls.patch (838 bytes) Revisiting this issue one year later shows we're almost there. The only thing that's left is the ability to handle external urls when clean_urls is disabled. Once implemented, menu.module can happily accept external urls and not just Drupal paths, making the menu system much more attractive to end users. I'll update the menu.module docs if this patch (or an alternative solution) is accepted. ------------------------------------------------------------------------ March 13, 2005 - 23:36 : killes@www.drop.org Attachment: http://drupal.org/files/issues/external_urls_0.patch (909 bytes) Updated from Drupal root directory. ------------------------------------------------------------------------ March 14, 2005 - 01:05 : gordon +1 This is a good thing for people who can't using clean urls. ------------------------------------------------------------------------ April 2, 2005 - 07:23 : TDobes This patch seems like a good idea, but does not apply anymore. Also, shouldn't the comments for l() and url() be updated to indicate that the functions will work with both internal and external URL's? ------------------------------------------------------------------------ April 2, 2005 - 08:25 : asimmonds Attachment: http://drupal.org/files/issues/external_urls_1.patch (897 bytes) The &'s had been changed to & amp; , here's a valid patch ------------------------------------------------------------------------ May 12, 2005 - 15:57 : adrian This is a new patch against cvs that allows url() to create external links, with the side-effect that menu can now be used to make external links. Also closes http://drupal.org/node/10177 This will be even more important with the primary links menu integration. (This patch already cleans up a special case in phptemplate for that). Killes benchmarked this, and it turns out the difference is negligible. Requests per second: 1.33 [#/sec] (mean) = with patch Requests per second: 1.31 [#/sec] (mean) = without patch. ------------------------------------------------------------------------ May 12, 2005 - 15:59 : killes@www.drop.org I had benchmarked /node as anon user without cache. ------------------------------------------------------------------------ May 25, 2005 - 05:50 : Steven +1 on the idea of this patch. It allows people to use l() for external links. However, I don't like the implementation. Running that complicated regexp for every url() call is simply wasteful. 95% or more of url() calls have a fixed string as parameter and point to Drupal's own paths. I suggest we create an url_external() wrapper around url() which allows external URLs. Any path which we want to allow external URLs in can use url_external() instead. The places where we use it will not be many anyway. ------------------------------------------------------------------------ May 25, 2005 - 06:05 : gordon splitting into url() url_external is not a good idea. The main reason for this patch that I see is for putting external urls into the navigation menu using the menu.module, and if all goes to plan, the primary and secondary menus. ------------------------------------------------------------------------ May 25, 2005 - 07:00 : Steven gordon: I still think this is a bit excessive. Perhaps instead of valid_url() we could use a simpler regexp !^[a-z]://!i or even strpos('://'). ------------------------------------------------------------------------ May 25, 2005 - 07:15 : gordon I am not says that it is or is not excessive. I was just saying that this is where it is going to be used, so creating url() and url_external() is not really an option ------------------------------------------------------------------------ May 25, 2005 - 07:57 : Bèr Kessels we /do/ need an external_url() thing. The links bundle (handling weblinks and the likes) willprovide this /and/ need this. People will get all sorts of tools to do /stuff/ with external links, like add permissions, track clicks etc. depending on the modules they have and the settings. All outgoing weblinks, should somehow respect those. Patches for core are coming, though. On top of that, a external_l() finction is simple. And if people want them in primary links they can do the regexping /before/ the function, not /in/ the function. The calling code can decide whether to call l() or external_l(), instead of putting logic in l() that is unneded 95% of the times. But i hereby promise to introduce such a function tbefore the end of this week :) ------------------------------------------------------------------------ May 25, 2005 - 08:17 : gordon The only thing I do not see in this plan is how with this work with the menu module and the navigation menu. ------------------------------------------------------------------------ May 25, 2005 - 11:04 : adrian Attachment: http://drupal.org/files/issues/url_external_links.patch (1.75 KB) What happened to the patch i uploaded ? It doesn't use a complex regexp, but uses strpos($url, '://') < 5.