{"thread":{"id":"37190","subject":"Two gitweb bugs related to javascript-actions","startedAt":"2014-07-22T20:41:43Z","lastAt":"2019-10-27T10:27:17Z","messageCount":4,"participants":["Robert Luberda","Jonathan Nieder","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"246523","messageId":"53CECC87.7030209@debian.org","threadId":"37190","inReplyTo":null,"subject":"Two gitweb bugs related to javascript-actions","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2014-07-22T20:41:43Z","receivedAt":"2014-07-22T20:41:43Z","isPatch":false,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"Hi\n\nSome time ago I reported the two following bugs to Debian BTS,\ncould you please look at them?\n\n 1. http://bugs.debian.org/741883 -- gitweb blame does not work\ncorrectly when $feature{'javascript-actions'} is enabled\n\nThis should be one-line change fix, which I really would like to be\napplied to the git package itself, not to do the same change over and\nover again every time my gitweb package is updated on my system.\n\n 2. https://bugs.debian.org/741884 feature{'javascript-actions'} breaks\nexternal links\n\nThis might be more challenging, but the simplest approach would be avoid\nadding those strange '[#?]js=1' parts to non-gitweb-generated links.\n\nThanks a lot,\nrobert\n"},{"id":"365471","messageId":"20181216232429.GJ75890@google.com","threadId":"37190","inReplyTo":"53CECC87.7030209@debian.org","subject":"[PATCH] gitweb: correctly store previous rev in javascript-actions mode","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-12-16T23:24:29Z","receivedAt":"2018-12-16T23:24:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Robert Luberda \nDate: Sun, 16 Mar 2014 22:57:19 +0100\n\nWithout this change, the setting\n\n $feature{'javascript-actions'}{'default'} = [1];\n\nin gitweb.conf breaks gitweb's blame page: clicking on line numbers\ndisplayed in the second column on the page has no effect.\n\nFor comparison, with javascript-actions disabled, clicking on line\nnumbers loads the previous version of the line.\n\nAddresses https://bugs.debian.org/741883.\n\nSigned-off-by: Jonathan Nieder \n---\nHi Robert,\n\nYears ago, you sent this obviously correct patch to the link mentioned\nabove, but it got lost in the noise. Sorry about that. Hopefully\nlate is better than never.\n\nMay we forge your sign-off? See\nhttps://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off\nfor more details about what this means.\n\nJakub et al, any thoughts? I don't see any unit tests in gitweb/static\nthat could avoid this regressing --- am I missing some, or if not any\nhints for someone who would want to add a test framework?\n\nThanks,\nJonathan\n\n gitweb/static/js/blame_incremental.js | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitweb/static/js/blame_incremental.js b/gitweb/static/js/blame_incremental.js\nindex db6eb50584..e100d8206b 100644\n--- a/gitweb/static/js/blame_incremental.js\n+++ b/gitweb/static/js/blame_incremental.js\n@@ -484,7 +484,7 @@ function processBlameLines(lines) {\n \t\t\tcase 'previous':\n \t\t\t\tcurCommit.nprevious++;\n \t\t\t\t// store only first 'previous' header\n-\t\t\t\tif (!'previous' in curCommit) {\n+\t\t\t\tif (!('previous' in curCommit)) {\n \t\t\t\t\tvar parts = data.split(' ', 2);\n \t\t\t\t\tcurCommit.previous = parts[0];\n \t\t\t\t\tcurCommit.file_parent = unquote(parts[1]);\n-- \n2.20.0.405.gbc1bbc6f85\n\n"},{"id":"384928","messageId":"8073f537-4380-4571-6997-546a9ca919f4@debian.org","threadId":"37190","inReplyTo":"20181216232429.GJ75890@google.com","subject":"Re: [PATCH] gitweb: correctly store previous rev in javascript-actions mode","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2019-10-27T09:14:26Z","receivedAt":"2019-10-27T09:21:55Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"From: Robert Luberda \nDate: Sun, 16 Mar 2014 22:57:19 +0100\n\nWithout this change, the setting\n\n $feature{'javascript-actions'}{'default'} = [1];\n\nin gitweb.conf breaks gitweb's blame page: clicking on line numbers\ndisplayed in the second column on the page has no effect.\n\nFor comparison, with javascript-actions disabled, clicking on line\nnumbers loads the previous version of the line.\n\nAddresses https://bugs.debian.org/741883.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Robert Luberda \n---\n> Hi Robert,\n\n> Years ago, you sent this obviously correct patch to the link mentioned\n> above, but it got lost in the noise. Sorry about that. Hopefully\n> late is better than never.\n\nHi,\n\nSomehow I missed your e-mail and just have found it today by a chance :(\n\n> May we forge your sign-off? See\nhttps://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off\nfor more details about what this means.\n\nDone, I've added the Signed-off-line above.\n\n> Jakub et al, any thoughts? I don't see any unit tests in gitweb/static\n> that could avoid this regressing --- am I missing some, or if not any\nhints for someone who would want to add a test framework?\n\nThanks,\nRobert\n\n gitweb/static/js/blame_incremental.js | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitweb/static/js/blame_incremental.js\nb/gitweb/static/js/blame_incremental.js\nindex db6eb50584..e100d8206b 100644\n--- a/gitweb/static/js/blame_incremental.js\n+++ b/gitweb/static/js/blame_incremental.js\n@@ -484,7 +484,7 @@ function processBlameLines(lines) {\n \t\t\tcase 'previous':\n \t\t\t\tcurCommit.nprevious++;\n \t\t\t\t// store only first 'previous' header\n-\t\t\t\tif (!'previous' in curCommit) {\n+\t\t\t\tif (!('previous' in curCommit)) {\n \t\t\t\t\tvar parts = data.split(' ', 2);\n \t\t\t\t\tcurCommit.previous = parts[0];\n \t\t\t\t\tcurCommit.file_parent = unquote(parts[1]);\n-- \n2.20.0.405.gbc1bbc6f85\n\n\n\n"},{"id":"384929","messageId":"86eeyybi0z.fsf@gmail.com","threadId":"37190","inReplyTo":"8073f537-4380-4571-6997-546a9ca919f4@debian.org","subject":"Re: [PATCH] gitweb: correctly store previous rev in javascript-actions mode","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2019-10-27T10:27:08Z","receivedAt":"2019-10-27T10:27:17Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Robert Luberda writes:\n\n> From: Robert Luberda \n> Date: Sun, 16 Mar 2014 22:57:19 +0100\n>\n> Without this change, the setting\n>\n> $feature{'javascript-actions'}{'default'} = [1];\n>\n> in gitweb.conf breaks gitweb's blame page: clicking on line numbers\n> displayed in the second column on the page has no effect.\n>\n> For comparison, with javascript-actions disabled, clicking on line\n> numbers loads the previous version of the line.\n>\n> Addresses https://bugs.debian.org/741883.\n>\n> Signed-off-by: Jonathan Nieder \n> Signed-off-by: Robert Luberda \n\nFor what it is worth it (because I am not active in gitweb development):\n\nAcked-by: Jakub Narębski \n\n> ---\n>> Hi Robert,\n>\n>> Years ago, you sent this obviously correct patch to the link mentioned\n>> above, but it got lost in the noise. Sorry about that. Hopefully\n>> late is better than never.\n>\n> Hi,\n>\n> Somehow I missed your e-mail and just have found it today by a chance :(\n>\n>> May we forge your sign-off? See\n>> https://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off\n>> for more details about what this means.\n>\n> Done, I've added the Signed-off-line above.\n\nThanks for following this up.\n\n>> Jakub et al, any thoughts? I don't see any unit tests in gitweb/static\n>> that could avoid this regressing --- am I missing some, or if not any\n>> hints for someone who would want to add a test framework?\n\nWe currently have no tests for the JavaScript in gitweb code; I am not\nsure how one would go to add such tests (and whether it would be\npossible while gitweb is part of git - if they need externel\ndependencies like Node.js or Selenium they would need to be able to be\ndisabled or enabled with builld option).\n\n> gitweb/static/js/blame_incremental.js | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/gitweb/static/js/blame_incremental.js\n> b/gitweb/static/js/blame_incremental.js\n> index db6eb50584..e100d8206b 100644\n> --- a/gitweb/static/js/blame_incremental.js\n> +++ b/gitweb/static/js/blame_incremental.js\n> @@ -484,7 +484,7 @@ function processBlameLines(lines) {\n> \t\t\tcase 'previous':\n> \t\t\t\tcurCommit.nprevious++;\n> \t\t\t\t// store only first 'previous' header\n> -\t\t\t\tif (!'previous' in curCommit) {\n> +\t\t\t\tif (!('previous' in curCommit)) {\n> \t\t\t\t\tvar parts = data.split(' ', 2);\n> \t\t\t\t\tcurCommit.previous = parts[0];\n> \t\t\t\t\tcurCommit.file_parent = unquote(parts[1]);\n\nThanks,\n-- \nJakub Narębski\n"}]}