threads / patch / 29041

patchgitweb: Don't append ';js=(0|1)' to external links

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

## tl;dr

4 messages between Nov 29, 2011 and Dec 2, 2011. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Jürgen Kreileder· Nov 29, 2011, 00:53 UTC · lore
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(-)
Show changes to 2 files +3 −3

gitweb/gitweb.perl, gitweb/static/js/javascript-detection.js

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
Jakub Narebski· Nov 29, 2011, 19:28 UTC · re: Jürgen Kreileder · lore

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

Jürgen Kreileder <jk@blackdown.de> writes:
Show 40 quoted lines
> 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(-)
Show changes to 2 files +9 −1

gitweb/gitweb.perl, gitweb/static/js/javascript-detection.js

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';
Jürgen Kreileder· Nov 29, 2011, 21:31 UTC · re: Jakub Narebski · lore

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

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.
Show 66 quoted lines
> 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/
Jakub Narebski· Dec 2, 2011, 20:44 UTC · re: Jürgen Kreileder · lore

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

On Tue, 29 Nov 2011, Jürgen Kreileder wrote:
Show 9 quoted lines
> 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

← back to recent threads