Subject: Re: [PATCH] gitweb: Don't append ';js=(0|1)' to external links

From: Jakub Narebski <[email-masked]>
Date: Tue, 29 Nov 2011 11:28:36 -0800 (PST)

Jürgen Kreileder <[email-masked]> writes:

> Signed-off-by: Juergen Kreileder <[email-masked]>
> ---
>  gitweb/gitweb.perl                       |    2 +-
>  gitweb/static/js/javascript-detection.js |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4f0c3bd..dfe3407 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3974,7 +3974,7 @@ sub git_footer_html {
>  		print qq!<script type="text/javascript">\n!.
>  		      qq!window.onload = function () {\n!;
>  		if (gitweb_check_feature('javascript-actions')) {
> -			print qq!	fixLinks();\n!;
> +			print qq!	fixLinks('$my_url');\n!;
>  		}
>  		if ($jstimezone && $tz_cookie && $datetime_class) {
>  			print qq!	var tz_cookie = { name: '$tz_cookie', expires: 14, path:
> '/' };\n!. # in days
> diff --git a/gitweb/static/js/javascript-detection.js
> b/gitweb/static/js/javascript-detection.js
> index fa2596f..36964ad 100644
> --- a/gitweb/static/js/javascript-detection.js
> +++ b/gitweb/static/js/javascript-detection.js
> @@ -29,11 +29,11 @@ var jsExceptionsRe = /[;?]js=[01](#.*)?$/;
>   *
>   * @globals jsExceptionsRe
>   */
> -function fixLinks() {
> +function fixLinks(baseurl) {
>  	var allLinks = document.getElementsByTagName("a") || document.links;
>  	for (var i = 0, len = allLinks.length; i < len; i++) {
>  		var link = allLinks[i];
> -		if (!jsExceptionsRe.test(link)) {
> +		if (!jsExceptionsRe.test(link) && !link.href.indexOf(baseurl)) {
>  			link.href = link.href.replace(/(#|$)/,
>  				(link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1$1');
>  		}
> -- 

Thanks for this, but I think a better solution would be to explicitly
mark the few external links we have e.g. with 'class="external"', and
use that to avoid adding ';js=(0|1)' to them.

This has the advantage that we can use different style to mark
outgoing external links.

I even have such patch somewhere in the StGit stack...
-- >8 --
Subject: [PATCH] gitweb: Mark external links

...and do not add 'js=1' to them with JavaScript.

Both $logo_url and $home_link links are now marked with "external"
class, and fixLink does not add 'js=1' to them on click.  We add
'js=1' to internal link to make server-side of gitweb know that it can
use JavaScript-only actions; we shouldn't do this for extrenal links,
as 'js=1' might mean something else to them.

Note that only links using A element matter: images (linked using
IMG), stylesheets (linked using STYLE) and JavaScript files (linked
using SCRIPT) were never affected.

Signed-off-by: Jakub Narebski <[email-masked]>
---
 gitweb/gitweb.perl                       |    5 ++++-
 gitweb/static/js/javascript-detection.js |    5 +++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7456a4b..f1c1caa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3626,13 +3626,16 @@ EOF
 	print "<div class=\"page_header\">\n";
 	if (defined $logo) {
 		print $cgi->a({-href => esc_url($logo_url),
+		               -class => "external",
 		               -title => $logo_label},
 		              $cgi->img({-src => esc_url($logo),
 		                         -width => 72, -height => 27,
 		                         -alt => "git",
 		                         -class => "logo"}));
 	}
-	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
+	print $cgi->a({-href => esc_url($home_link)
+	               -class => "external"},
+	              $home_link_str) . " / ";
 	if (defined $project) {
 		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
 		if (defined $action) {
diff --git a/gitweb/static/js/javascript-detection.js b/gitweb/static/js/javascript-detection.js
index 2b51e55..fc59e42 100644
--- a/gitweb/static/js/javascript-detection.js
+++ b/gitweb/static/js/javascript-detection.js
@@ -60,6 +60,11 @@ function fixLink(link) {
 	 */
 	var jsExceptionsRe = /[;?]js=[01]$/;
 
+	// don't change links marked as external ($logo_url, $home_link)
+	if (link.className === 'external') {
+		return;
+	}
+
 	if (!jsExceptionsRe.test(link)) { // =~ /[;?]js=[01]$/;
 		link.href +=
 			(link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1';