threads / discuss / 37190

Two gitweb bugs related to javascript-actions

Subject: Two gitweb bugs related to javascript-actions

## tl;dr

4 messages between Jul 22, 2014 and Oct 27, 2019.

replies: 3people: 3as markdown or json

Robert Luberda· Jul 22, 2014, 20:41 UTC · lore
Hi

Some time ago I reported the two following bugs to Debian BTS, could you please look at them?

  1. http://bugs.debian.org/741883 -- gitweb blame does not work
correctly when $feature{'javascript-actions'} is enabled

This should be one-line change fix, which I really would like to be applied to the git package itself, not to do the same change over and over again every time my gitweb package is updated on my system.

  2. https://bugs.debian.org/741884 feature{'javascript-actions'} breaks
external links

This might be more challenging, but the simplest approach would be avoid adding those strange '[#?]js=1' parts to non-gitweb-generated links.

Thanks a lot, robert

Jonathan Nieder· Dec 16, 2018, 23:24 UTC · re: Robert Luberda · lore

[PATCH] gitweb: correctly store previous rev in javascript-actions mode

From: Robert Luberda <robert@debian.org>
Date: Sun, 16 Mar 2014 22:57:19 +0100
Without this change, the setting
 $feature{'javascript-actions'}{'default'} = [1];

in gitweb.conf breaks gitweb's blame page: clicking on line numbers displayed in the second column on the page has no effect.

For comparison, with javascript-actions disabled, clicking on line numbers loads the previous version of the line.

Addresses https://bugs.debian.org/741883.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi Robert,

Years ago, you sent this obviously correct patch to the link mentioned above, but it got lost in the noise. Sorry about that. Hopefully late is better than never.

May we forge your sign-off? See https://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off for more details about what this means.

Jakub et al, any thoughts? I don't see any unit tests in gitweb/static that could avoid this regressing --- am I missing some, or if not any hints for someone who would want to add a test framework?

Thanks, Jonathan

 gitweb/static/js/blame_incremental.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/static/js/blame_incremental.js b/gitweb/static/js/blame_incremental.js
index db6eb50584..e100d8206b 100644
--- a/gitweb/static/js/blame_incremental.js
+++ b/gitweb/static/js/blame_incremental.js
@@ -484,7 +484,7 @@ function processBlameLines(lines) {
 			case 'previous':
 				curCommit.nprevious++;
 				// store only first 'previous' header
-				if (!'previous' in curCommit) {
+				if (!('previous' in curCommit)) {
 					var parts = data.split(' ', 2);
 					curCommit.previous    = parts[0];
 					curCommit.file_parent = unquote(parts[1]);

-- 
2.20.0.405.gbc1bbc6f85
Robert Luberda· Oct 27, 2019, 09:14 UTC · re: Jonathan Nieder · lore

Re: [PATCH] gitweb: correctly store previous rev in javascript-actions mode

From: Robert Luberda <robert@debian.org>
Date: Sun, 16 Mar 2014 22:57:19 +0100
Without this change, the setting
 $feature{'javascript-actions'}{'default'} = [1];

in gitweb.conf breaks gitweb's blame page: clicking on line numbers displayed in the second column on the page has no effect.

For comparison, with javascript-actions disabled, clicking on line numbers loads the previous version of the line.

Addresses https://bugs.debian.org/741883.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Robert Luberda <robert@debian.org>
---
> Hi Robert,
> Years ago, you sent this obviously correct patch to the link mentioned
> above, but it got lost in the noise.  Sorry about that.  Hopefully
> late is better than never.
Hi,
Somehow I missed your e-mail and just have found it today by a chance :(
> May we forge your sign-off?  See

https://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off for more details about what this means.

Done, I've added the Signed-off-line above.
> Jakub et al, any thoughts?  I don't see any unit tests in gitweb/static
> that could avoid this regressing --- am I missing some, or if not any
hints for someone who would want to add a test framework?

Thanks, Robert

 gitweb/static/js/blame_incremental.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/static/js/blame_incremental.js
b/gitweb/static/js/blame_incremental.js
index db6eb50584..e100d8206b 100644
--- a/gitweb/static/js/blame_incremental.js
+++ b/gitweb/static/js/blame_incremental.js
@@ -484,7 +484,7 @@ function processBlameLines(lines) {
 			case 'previous':
 				curCommit.nprevious++;
 				// store only first 'previous' header
-				if (!'previous' in curCommit) {
+				if (!('previous' in curCommit)) {
 					var parts = data.split(' ', 2);
 					curCommit.previous    = parts[0];
 					curCommit.file_parent = unquote(parts[1]);

-- 
2.20.0.405.gbc1bbc6f85
Jakub Narebski· Oct 27, 2019, 10:27 UTC · re: Robert Luberda · lore

Re: [PATCH] gitweb: correctly store previous rev in javascript-actions mode

Robert Luberda <robert@debian.org> writes:
Show 17 quoted lines
> From: Robert Luberda <robert@debian.org>
> Date: Sun, 16 Mar 2014 22:57:19 +0100
>
> Without this change, the setting
>
>  $feature{'javascript-actions'}{'default'} = [1];
>
> in gitweb.conf breaks gitweb's blame page: clicking on line numbers
> displayed in the second column on the page has no effect.
>
> For comparison, with javascript-actions disabled, clicking on line
> numbers loads the previous version of the line.
>
> Addresses https://bugs.debian.org/741883.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Robert Luberda <robert@debian.org>
For what it is worth it (because I am not active in gitweb development):
Acked-by: Jakub Narębski <jnareb@gmail.com>
Show 16 quoted lines
> ---
>> Hi Robert,
>
>> Years ago, you sent this obviously correct patch to the link mentioned
>> above, but it got lost in the noise.  Sorry about that.  Hopefully
>> late is better than never.
>
> Hi,
>
> Somehow I missed your e-mail and just have found it today by a chance :(
>
>> May we forge your sign-off?  See
>> https://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off
>> for more details about what this means.
>
> Done, I've added the Signed-off-line above.
Thanks for following this up.
>> Jakub et al, any thoughts?  I don't see any unit tests in gitweb/static
>> that could avoid this regressing --- am I missing some, or if not any
>> hints for someone who would want to add a test framework?

We currently have no tests for the JavaScript in gitweb code; I am not sure how one would go to add such tests (and whether it would be possible while gitweb is part of git - if they need externel dependencies like Node.js or Selenium they would need to be able to be disabled or enabled with builld option).

Show 17 quoted lines
>  gitweb/static/js/blame_incremental.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/static/js/blame_incremental.js
> b/gitweb/static/js/blame_incremental.js
> index db6eb50584..e100d8206b 100644
> --- a/gitweb/static/js/blame_incremental.js
> +++ b/gitweb/static/js/blame_incremental.js
> @@ -484,7 +484,7 @@ function processBlameLines(lines) {
>  			case 'previous':
>  				curCommit.nprevious++;
>  				// store only first 'previous' header
> -				if (!'previous' in curCommit) {
> +				if (!('previous' in curCommit)) {
>  					var parts = data.split(' ', 2);
>  					curCommit.previous    = parts[0];
>  					curCommit.file_parent = unquote(parts[1]);
Thanks,
-- 
Jakub Narębski

← back to recent threads