{"thread":{"id":"29041","subject":"[PATCH] gitweb: Don't append ';js=(0|1)' to external links","startedAt":"2011-11-29T00:53:10Z","lastAt":"2011-12-02T20:44:44Z","messageCount":4,"participants":["Jürgen Kreileder","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"180073","messageId":"CAKD0UuzU4hAe7RGYukGyPpvfGeYJ3pgJ5pynupneMpQSaX5Cjw@mail.gmail.com","threadId":"29041","inReplyTo":null,"subject":"[PATCH] gitweb: Don't append ';js=(0|1)' to external links","fromName":"Jürgen Kreileder","fromEmail":"jk@blackdown.de","sentAt":"2011-11-29T00:53:10Z","receivedAt":"2011-11-29T00:53:10Z","isPatch":true,"sender":{"key":"jk@blackdown.de","avatar":"https://avatars.githubusercontent.com/u/4390544?v=4"},"body":"Signed-off-by: Juergen Kreileder <jk@blackdown.de>\n---\n gitweb/gitweb.perl                       |    2 +-\n gitweb/static/js/javascript-detection.js |    4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4f0c3bd..dfe3407 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3974,7 +3974,7 @@ sub git_footer_html {\n \t\tprint qq!<script type=\"text/javascript\">\\n!.\n \t\t      qq!window.onload = function () {\\n!;\n \t\tif (gitweb_check_feature('javascript-actions')) {\n-\t\t\tprint qq!\tfixLinks();\\n!;\n+\t\t\tprint qq!\tfixLinks('$my_url');\\n!;\n \t\t}\n \t\tif ($jstimezone && $tz_cookie && $datetime_class) {\n \t\t\tprint qq!\tvar tz_cookie = { name: '$tz_cookie', expires: 14, path:\n'/' };\\n!. # in days\ndiff --git a/gitweb/static/js/javascript-detection.js\nb/gitweb/static/js/javascript-detection.js\nindex fa2596f..36964ad 100644\n--- a/gitweb/static/js/javascript-detection.js\n+++ b/gitweb/static/js/javascript-detection.js\n@@ -29,11 +29,11 @@ var jsExceptionsRe = /[;?]js=[01](#.*)?$/;\n  *\n  * @globals jsExceptionsRe\n  */\n-function fixLinks() {\n+function fixLinks(baseurl) {\n \tvar allLinks = document.getElementsByTagName(\"a\") || document.links;\n \tfor (var i = 0, len = allLinks.length; i < len; i++) {\n \t\tvar link = allLinks[i];\n-\t\tif (!jsExceptionsRe.test(link)) {\n+\t\tif (!jsExceptionsRe.test(link) && !link.href.indexOf(baseurl)) {\n \t\t\tlink.href = link.href.replace(/(#|$)/,\n \t\t\t\t(link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1$1');\n \t\t}\n-- \n1.7.5.4\n"},{"id":"180103","messageId":"m3pqgaloda.fsf@localhost.localdomain","threadId":"29041","inReplyTo":"CAKD0UuzU4hAe7RGYukGyPpvfGeYJ3pgJ5pynupneMpQSaX5Cjw@mail.gmail.com","subject":"Re: [PATCH] gitweb: Don't append ';js=(0|1)' to external links","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-11-29T19:28:36Z","receivedAt":"2011-11-29T19:28:36Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jürgen Kreileder <jk@blackdown.de> writes:\n\n> Signed-off-by: Juergen Kreileder <jk@blackdown.de>\n> ---\n>  gitweb/gitweb.perl                       |    2 +-\n>  gitweb/static/js/javascript-detection.js |    4 ++--\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 4f0c3bd..dfe3407 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3974,7 +3974,7 @@ sub git_footer_html {\n>  \t\tprint qq!<script type=\"text/javascript\">\\n!.\n>  \t\t      qq!window.onload = function () {\\n!;\n>  \t\tif (gitweb_check_feature('javascript-actions')) {\n> -\t\t\tprint qq!\tfixLinks();\\n!;\n> +\t\t\tprint qq!\tfixLinks('$my_url');\\n!;\n>  \t\t}\n>  \t\tif ($jstimezone && $tz_cookie && $datetime_class) {\n>  \t\t\tprint qq!\tvar tz_cookie = { name: '$tz_cookie', expires: 14, path:\n> '/' };\\n!. # in days\n> diff --git a/gitweb/static/js/javascript-detection.js\n> b/gitweb/static/js/javascript-detection.js\n> index fa2596f..36964ad 100644\n> --- a/gitweb/static/js/javascript-detection.js\n> +++ b/gitweb/static/js/javascript-detection.js\n> @@ -29,11 +29,11 @@ var jsExceptionsRe = /[;?]js=[01](#.*)?$/;\n>   *\n>   * @globals jsExceptionsRe\n>   */\n> -function fixLinks() {\n> +function fixLinks(baseurl) {\n>  \tvar allLinks = document.getElementsByTagName(\"a\") || document.links;\n>  \tfor (var i = 0, len = allLinks.length; i < len; i++) {\n>  \t\tvar link = allLinks[i];\n> -\t\tif (!jsExceptionsRe.test(link)) {\n> +\t\tif (!jsExceptionsRe.test(link) && !link.href.indexOf(baseurl)) {\n>  \t\t\tlink.href = link.href.replace(/(#|$)/,\n>  \t\t\t\t(link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1$1');\n>  \t\t}\n> -- \n\nThanks for this, but I think a better solution would be to explicitly\nmark the few external links we have e.g. with 'class=\"external\"', and\nuse that to avoid adding ';js=(0|1)' to them.\n\nThis has the advantage that we can use different style to mark\noutgoing external links.\n\nI even have such patch somewhere in the StGit stack...\n-- >8 --\nSubject: [PATCH] gitweb: Mark external links\n\n...and do not add 'js=1' to them with JavaScript.\n\nBoth $logo_url and $home_link links are now marked with \"external\"\nclass, and fixLink does not add 'js=1' to them on click.  We add\n'js=1' to internal link to make server-side of gitweb know that it can\nuse JavaScript-only actions; we shouldn't do this for extrenal links,\nas 'js=1' might mean something else to them.\n\nNote that only links using A element matter: images (linked using\nIMG), stylesheets (linked using STYLE) and JavaScript files (linked\nusing SCRIPT) were never affected.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl                       |    5 ++++-\n gitweb/static/js/javascript-detection.js |    5 +++++\n 2 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7456a4b..f1c1caa 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3626,13 +3626,16 @@ EOF\n \tprint \"<div class=\\\"page_header\\\">\\n\";\n \tif (defined $logo) {\n \t\tprint $cgi->a({-href => esc_url($logo_url),\n+\t\t               -class => \"external\",\n \t\t               -title => $logo_label},\n \t\t              $cgi->img({-src => esc_url($logo),\n \t\t                         -width => 72, -height => 27,\n \t\t                         -alt => \"git\",\n \t\t                         -class => \"logo\"}));\n \t}\n-\tprint $cgi->a({-href => esc_url($home_link)}, $home_link_str) . \" / \";\n+\tprint $cgi->a({-href => esc_url($home_link)\n+\t               -class => \"external\"},\n+\t              $home_link_str) . \" / \";\n \tif (defined $project) {\n \t\tprint $cgi->a({-href => href(action=>\"summary\")}, esc_html($project));\n \t\tif (defined $action) {\ndiff --git a/gitweb/static/js/javascript-detection.js b/gitweb/static/js/javascript-detection.js\nindex 2b51e55..fc59e42 100644\n--- a/gitweb/static/js/javascript-detection.js\n+++ b/gitweb/static/js/javascript-detection.js\n@@ -60,6 +60,11 @@ function fixLink(link) {\n \t */\n \tvar jsExceptionsRe = /[;?]js=[01]$/;\n \n+\t// don't change links marked as external ($logo_url, $home_link)\n+\tif (link.className === 'external') {\n+\t\treturn;\n+\t}\n+\n \tif (!jsExceptionsRe.test(link)) { // =~ /[;?]js=[01]$/;\n \t\tlink.href +=\n \t\t\t(link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1';\n"},{"id":"180114","messageId":"CAKD0UuyH3m0RR2=jk5apFjVMgbD5iWeztR94mE-m7q9dyYKR2Q@mail.gmail.com","threadId":"29041","inReplyTo":"m3pqgaloda.fsf@localhost.localdomain","subject":"Re: [PATCH] gitweb: Don't append ';js=(0|1)' to external links","fromName":"Jürgen Kreileder","fromEmail":"jk@blackdown.de","sentAt":"2011-11-29T21:31:20Z","receivedAt":"2011-11-29T21:31:20Z","isPatch":true,"sender":{"key":"jk@blackdown.de","avatar":"https://avatars.githubusercontent.com/u/4390544?v=4"},"body":"On Tue, Nov 29, 2011 at 20:28, Jakub Narebski <jnareb@gmail.com> wrote:\n> Jürgen Kreileder <jk@blackdown.de> writes:\n[...]\n> Thanks for this, but I think a better solution would be to explicitly\n> mark the few external links we have e.g. with 'class=\"external\"', and\n> use that to avoid adding ';js=(0|1)' to them.\n\nThis won't work because there are more than a few external links.  Think of\nlinks added in the header or footer or via a project specific README.html.\n\nYou would have to do it the other way round: Mark all internal links.\n\n> This has the advantage that we can use different style to mark\n> outgoing external links.\n>\n> I even have such patch somewhere in the StGit stack...\n> -- >8 --\n> Subject: [PATCH] gitweb: Mark external links\n>\n> ...and do not add 'js=1' to them with JavaScript.\n>\n> Both $logo_url and $home_link links are now marked with \"external\"\n> class, and fixLink does not add 'js=1' to them on click.  We add\n> 'js=1' to internal link to make server-side of gitweb know that it can\n> use JavaScript-only actions; we shouldn't do this for extrenal links,\n> as 'js=1' might mean something else to them.\n>\n> Note that only links using A element matter: images (linked using\n> IMG), stylesheets (linked using STYLE) and JavaScript files (linked\n> using SCRIPT) were never affected.\n>\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> ---\n>  gitweb/gitweb.perl                       |    5 ++++-\n>  gitweb/static/js/javascript-detection.js |    5 +++++\n>  2 files changed, 9 insertions(+), 1 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7456a4b..f1c1caa 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3626,13 +3626,16 @@ EOF\n>        print \"<div class=\\\"page_header\\\">\\n\";\n>        if (defined $logo) {\n>                print $cgi->a({-href => esc_url($logo_url),\n> +                              -class => \"external\",\n>                               -title => $logo_label},\n>                              $cgi->img({-src => esc_url($logo),\n>                                         -width => 72, -height => 27,\n>                                         -alt => \"git\",\n>                                         -class => \"logo\"}));\n>        }\n> -       print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . \" / \";\n> +       print $cgi->a({-href => esc_url($home_link)\n> +                      -class => \"external\"},\n> +                     $home_link_str) . \" / \";\n>        if (defined $project) {\n>                print $cgi->a({-href => href(action=>\"summary\")}, esc_html($project));\n>                if (defined $action) {\n> diff --git a/gitweb/static/js/javascript-detection.js b/gitweb/static/js/javascript-detection.js\n> index 2b51e55..fc59e42 100644\n> --- a/gitweb/static/js/javascript-detection.js\n> +++ b/gitweb/static/js/javascript-detection.js\n> @@ -60,6 +60,11 @@ function fixLink(link) {\n>         */\n>        var jsExceptionsRe = /[;?]js=[01]$/;\n>\n> +       // don't change links marked as external ($logo_url, $home_link)\n> +       if (link.className === 'external') {\n> +               return;\n> +       }\n> +\n>        if (!jsExceptionsRe.test(link)) { // =~ /[;?]js=[01]$/;\n>                link.href +=\n>                        (link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1';\n>\n>\n>\n\n\n\n-- \nhttp://blog.blackdown.de/\nhttp://www.flickr.com/photos/jkreileder/\n"},{"id":"180269","messageId":"201112022144.45531.jnareb@gmail.com","threadId":"29041","inReplyTo":"CAKD0UuyH3m0RR2=jk5apFjVMgbD5iWeztR94mE-m7q9dyYKR2Q@mail.gmail.com","subject":"Re: [PATCH] gitweb: Don't append ';js=(0|1)' to external links","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-12-02T20:44:44Z","receivedAt":"2011-12-02T20:44:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 29 Nov 2011, Jürgen Kreileder wrote:\n> On Tue, Nov 29, 2011 at 20:28, Jakub Narebski <jnareb@gmail.com> wrote:\n> > Jürgen Kreileder <jk@blackdown.de> writes:\n> [...]\n> > Thanks for this, but I think a better solution would be to explicitly\n> > mark the few external links we have e.g. with 'class=\"external\"', and\n> > use that to avoid adding ';js=(0|1)' to them.\n> \n> This won't work because there are more than a few external links.  Think of\n> links added in the header or footer or via a project specific README.html.\n\nThanks for noticing that.\n \n> You would have to do it the other way round: Mark all internal links.\n\nIn this case your solution is better... provided that you either check\nthat all internal links generated by gitweb are absolute URLs in all cases\n(they always include $my_url), or add ';js=(0|1)' also for relative\nURLs (i.e. not starting with http:// or https://).\n\n\nWell... the \"mark external\" idea would also work, provided that we rework\nhow ';js=(0|1)' is generated.  Instead of fixing links on load, we could\nadd an event handler[1] which on clicking the link[2] would add \n';js=(0|1)'... and there you can check if we are inside README.html etc.\nby examining DOM (perhaps those externally added fragments would need to\nbe wrapped in div / span with \"external\" class, though).\n \n[1] You assign event handler to 'html' or 'body' element, and check in\n    handler where you clicked; that is what we do for JavaScript\n    timezone stuff.\n\n[2] I am not sure if \"Open in new tab\" from context menu or Ctrl-Click\n    would trigger adding ';js=(0|1)' or not... and whether it is a bad\n    thing or not.\n\n\n> > This has the advantage that we can use different style to mark\n> > outgoing external links.\n\nI think this advantage might be worth it, even without changes to\njavascript-detection.js\n\n-- \nJakub Narebski\nPoland\n"}]}