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

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

[Jürgen Kreileder]

Signed-off-by: Juergen Kreileder <jk@blackdown.de>
---
 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');
 		}
-- 
1.7.5.4

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

[Jakub Narebski]

Jürgen Kreileder <jk@blackdown.de> writes:

> Signed-off-by: Juergen Kreileder <jk@blackdown.de>
> ---
>  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 <jnareb@gmail.com>
---
 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';

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

[Jürgen Kreileder]

On Tue, Nov 29, 2011 at 20:28, Jakub Narebski <jnareb@gmail.com> wrote:
> Jürgen Kreileder <jk@blackdown.de> writes:
[...]
> 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 won't work because there are more than a few external links.  Think of
links added in the header or footer or via a project specific README.html.

You would have to do it the other way round: Mark all internal links.

> 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 <jnareb@gmail.com>
> ---
>  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';
>
>
>



-- 
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/

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

[Jakub Narebski]

On Tue, 29 Nov 2011, Jürgen Kreileder wrote:
> On Tue, Nov 29, 2011 at 20:28, Jakub Narebski <jnareb@gmail.com> wrote:
> > Jürgen Kreileder <jk@blackdown.de> writes:
> [...]
> > 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 won't work because there are more than a few external links.  Think of
> links added in the header or footer or via a project specific README.html.

Thanks for noticing that.
 
> You would have to do it the other way round: Mark all internal links.

In this case your solution is better... provided that you either check
that all internal links generated by gitweb are absolute URLs in all cases
(they always include $my_url), or add ';js=(0|1)' also for relative
URLs (i.e. not starting with http:// or https://).


Well... the "mark external" idea would also work, provided that we rework
how ';js=(0|1)' is generated.  Instead of fixing links on load, we could
add an event handler[1] which on clicking the link[2] would add 
';js=(0|1)'... and there you can check if we are inside README.html etc.
by examining DOM (perhaps those externally added fragments would need to
be wrapped in div / span with "external" class, though).
 
[1] You assign event handler to 'html' or 'body' element, and check in
    handler where you clicked; that is what we do for JavaScript
    timezone stuff.

[2] I am not sure if "Open in new tab" from context menu or Ctrl-Click
    would trigger adding ';js=(0|1)' or not... and whether it is a bad
    thing or not.


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

I think this advantage might be worth it, even without changes to
javascript-detection.js

-- 
Jakub Narebski
Poland