JS refactoring

This patch looks like a good improvement. I don't think the backwards compatible function is really needed though, because extlinkAttach() is already locally scoped inside of the (function ($) { wrapper at the top of the file.

This is a huge improvement! Looking at the current extlink.js makes me vomit a bit.

+++ b/extlink.jsundefined
@@ -22,73 +25,64 @@ function extlinkAttach(context) {
   // In jQuery 1.1 and higher, we'd use a filter method here, but it is not
   // available in jQuery 1.0 (Drupal 5 default).
   var external_links = new Array();
   var mailto_links = new Array();

We're on Drupal 7, let's get rid of all the old jQuery version checks.

+++ b/extlink.jsundefined
@@ -22,73 +25,64 @@ function extlinkAttach(context) {
     try {
-      var url = this.href.toLowerCase();
+      var url = link.href.toLowerCase();
       if (url.indexOf('http') == 0 && (!url.match(internal_link) || (extInclude && url.match(extInclude))) && !(extExclude && url.match(extExclude))) {
-        external_links.push(this);
+        external_links.push(link);
       }
       // Do not include area tags with begin with mailto: (this prohibits
       // icons from being added to image-maps).
       else if (this.tagName != 'AREA' && url.indexOf('mailto:') == 0) {
-        mailto_links.push(this);
+        mailto_links.push(link);
       }
     }
     // IE7 throws errors often when dealing with irregular links, such as:
     // <a href="node/10"></a> Empty tags.
     // <a href="http://user:pass@example.com">example</a> User:pass syntax.
-    catch(error) {
+    catch (error) {
       return false;

Would be nice to not need that try/catch. Is it just the url.match calls that throw it on IE7?

+++ b/extlink.jsundefined
@@ -96,12 +90,46 @@ function extlinkAttach(context) {
+  if (parseFloat($().jquery) < 1.2) {
+    $links_to_process = $(links).not('[img]');

Probably don't need that version check.

+++ b/extlink.jsundefined
@@ -96,12 +90,46 @@ function extlinkAttach(context) {
+  var length = $links_to_process.length;
+  for (i = 0; i < length; i++) {
+    var $link = $($links_to_process[i]);
+    if ($link.css('display') == 'inline') {
+      $link.after('<span class=' + class_name + '></span>');

$.each($links_to_process, function(index, link) {
});

Edit: ignore this patch

Slight reroll of #1 to properly apply with drush make.

This is a real worthwhile change and should really be done before #2038235: Add minified version of extlink.js.

I will work on it soon, unless someone wants to re-roll a patch.

Can I get some input on whether we want to go with the for loop instead of each?

#5 - Do you know how much time it will save doing it this way?

#10 - if you re-roll it without the for loop I will include it. There have been a few changes to the javascript in the master branch, so be sure to check that first.

We can then add it later pending input from other maintainers.

Setting the priority to Major. I will re-roll this patch soon, unless someone does it first.

Re-worked the patch. Uses jQuery.each

I am happy to move away from it if quicksketch agrees to it. He knows more than me about it.

Sorry, bad patch format.

White space issues.

I think it is failing because it's applying to 7.x-1.13 and not the master branch.

Trying again anyway.

You should create a 7.x-1.x-dev release for this project so that you can properly assign versions to things in development here in the issue queue.

Added the new branch for 7.x-1.x and dev release.

#23: extlink-1329786-23.patch queued for re-testing.

#23: extlink-1329786-23.patch queued for re-testing.

Commited to Git. Needs port to 8.x

By having the backwards compatibility section we add 0.05% to the page execution time.

This doesn't sound like much, but it is 45% of the execution time for external links. Is there a better way we can do it?

Moving to the correct branch.

It looks like this was already done as apart of the conversion.

Agreed, it looks like this patch is already in the 8.x branch.