* [PATCH] gitweb: fix "next" link on bottom of commit log page
@ 2008-05-29 18:10 Gerrit Pape
2008-05-30 8:12 ` Jakub Narebski
0 siblings, 1 reply; 5+ messages in thread
From: Gerrit Pape @ 2008-05-29 18:10 UTC (permalink / raw)
To: git, Junio C Hamano
When viewing a gitweb repository commit log, the "next" link at the top
of the page works as expected, the "next" link on the bottom of the page
has a=search instead of a=log and thus fails to get you to the next
page. This commit replaces the bottom "next" link with the same links
as shown at the top of the page.
The bad link was reported by Kai Blin through
http://bugs.debian.org/481902
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
gitweb/gitweb.perl | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 57a1905..f7b9ac2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4703,12 +4703,7 @@ sub git_log {
git_print_log($co{'comment'}, -final_empty_line=> 1);
print "</div>\n";
}
- if ($#commitlist >= 100) {
- print "<div class=\"page_nav\">\n";
- print $cgi->a({-href => href(-replay=>1, page=>$page+1),
- -accesskey => "n", -title => "Alt-n"}, "next");
- print "</div>\n";
- }
+ git_print_page_nav('log','', $hash,undef,undef, $paging_nav);
git_footer_html();
}
--
1.5.5.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gitweb: fix "next" link on bottom of commit log page
2008-05-29 18:10 [PATCH] gitweb: fix "next" link on bottom of commit log page Gerrit Pape
@ 2008-05-30 8:12 ` Jakub Narebski
2008-06-02 8:52 ` Jakub Narebski
2008-06-02 9:54 ` [PATCH] gitweb: Fix "next" link on bottom of page Jakub Narebski
0 siblings, 2 replies; 5+ messages in thread
From: Jakub Narebski @ 2008-05-30 8:12 UTC (permalink / raw)
To: Gerrit Pape; +Cc: git, Junio C Hamano
Gerrit Pape <pape@smarden.org> writes:
> When viewing a gitweb repository commit log, the "next" link at the top
> of the page works as expected, the "next" link on the bottom of the page
> has a=search instead of a=log and thus fails to get you to the next
> page. This commit replaces the bottom "next" link with the same links
> as shown at the top of the page.
> - print $cgi->a({-href => href(-replay=>1, page=>$page+1),
> - -accesskey => "n", -title => "Alt-n"}, "next");
Should not happen: href(-replay=>1, ...) should have the same value
of 'a' parameter as the page it is in, so it should be 'log' not 'search'.
Will investigate.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gitweb: fix "next" link on bottom of commit log page
2008-05-30 8:12 ` Jakub Narebski
@ 2008-06-02 8:52 ` Jakub Narebski
2008-06-02 9:54 ` [PATCH] gitweb: Fix "next" link on bottom of page Jakub Narebski
1 sibling, 0 replies; 5+ messages in thread
From: Jakub Narebski @ 2008-06-02 8:52 UTC (permalink / raw)
To: Gerrit Pape; +Cc: git, Junio C Hamano
Jakub Narebski <jnareb@gmail.com> writes:
> Gerrit Pape <pape@smarden.org> writes:
>
> > When viewing a gitweb repository commit log, the "next" link at the top
> > of the page works as expected, the "next" link on the bottom of the page
> > has a=search instead of a=log and thus fails to get you to the next
> > page. This commit replaces the bottom "next" link with the same links
> > as shown at the top of the page.
>
> > - print $cgi->a({-href => href(-replay=>1, page=>$page+1),
> > - -accesskey => "n", -title => "Alt-n"}, "next");
>
> Should not happen: href(-replay=>1, ...) should have the same value
> of 'a' parameter as the page it is in, so it should be 'log' not 'search'.
>
> Will investigate.
Now I know what is happening. git_header_html() modifies parameters
in $cgi, and href(-replay, ...) uses paramemeters values from $cgi,
not saved in variables (although it could).
So the correct solution would be to change the part which generates
search form in git_header_html() to _not_ modify $cgi->params().
Patch will follow...
P.S. I wonder if using Test::WWW::Mechanize (from CPAN) to test
gitweb's HTML output would be a good idea...
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] gitweb: Fix "next" link on bottom of page
2008-05-30 8:12 ` Jakub Narebski
2008-06-02 8:52 ` Jakub Narebski
@ 2008-06-02 9:54 ` Jakub Narebski
2008-06-02 17:02 ` Gerrit Pape
1 sibling, 1 reply; 5+ messages in thread
From: Jakub Narebski @ 2008-06-02 9:54 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Gerrit Pape, Kai Blin
Fix search form generation to not modify $cgi->param(...)'s.
In git_header_html() we used to use $cgi->hidden(-name => "a") etc. to
generate hidden fields; unfortunately to use this form it is required
to modify $cgi->param("a") etc., which makes href(-replay,...) use
wrong replay values. This for example made the "next" link on the
bottom of the page has a=search instead of a=$action, and thus fails to
get you to the next page.
Because in CGI the value of a hidden field is "sticky", there is no
way to modify it short of modifying $cgi->param(...). Therefore it
got replaced by generating <input type="hidden" ...> element [semi]
directly.
Alternate solution would be for href(-replay,...) to use values saved
in global variables, such as $action etc., instead of (re)reading them
from $cgi->param($symbol).
The bad link was reported by Kai Blin through
http://bugs.debian.org/481902
Reported-by: Kai Blin <kai.blin@gmail.com>
Noticed-by: Gerrit Pape <pape@smarden.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Jakub Narebski <jnareb@gmail.com> wrote:
> Gerrit Pape <pape@smarden.org> writes:
>
>> When viewing a gitweb repository commit log, the "next" link at the top
>> of the page works as expected, the "next" link on the bottom of the page
>> has a=search instead of a=log and thus fails to get you to the next
>> page. This commit replaces the bottom "next" link with the same links
>> as shown at the top of the page.
>
>> - print $cgi->a({-href => href(-replay=>1, page=>$page+1),
>> - -accesskey => "n", -title => "Alt-n"}, "next");
>
> Should not happen: href(-replay=>1, ...) should have the same value
> of 'a' parameter as the page it is in, so it should be 'log' not 'search'.
This bug was caused by the fact that git_header_html() modified
parameters in $cgi->param(...) when generating search form, and
href(-replay, ...) uses paramemeters values from $cgi, not saved in
variables (although it could).
This fixes mentioned bug, not only in the case of 'log' view, but in
all cases (although it is possible that this bug doesn't occur for
other pages).
[I'm sorry if I have send this patch twice.]
gitweb/gitweb.perl | 13 +++++--------
1 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dd0f0ac..50cde3b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2622,7 +2622,7 @@ EOF
print "</div>\n";
my ($have_search) = gitweb_check_feature('search');
- if ((defined $project) && ($have_search)) {
+ if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
}
@@ -2638,16 +2638,13 @@ EOF
my ($use_pathinfo) = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
- } else {
- $cgi->param("p", $project);
}
- $cgi->param("a", "search");
- $cgi->param("h", $search_hash);
print $cgi->startform(-method => "get", -action => $action) .
"<div class=\"search\">\n" .
- (!$use_pathinfo && $cgi->hidden(-name => "p") . "\n") .
- $cgi->hidden(-name => "a") . "\n" .
- $cgi->hidden(-name => "h") . "\n" .
+ (!$use_pathinfo &&
+ $cgi->input({-name=>"p", -value=>$project, -type=>"hidden"}) . "\n") .
+ $cgi->input({-name=>"a", -value=>"search", -type=>"hidden"}) . "\n" .
+ $cgi->input({-name=>"h", -value=>$search_hash, -type=>"hidden"}) . "\n" .
$cgi->popup_menu(-name => 'st', -default => 'commit',
-values => ['commit', 'grep', 'author', 'committer', 'pickaxe']) .
$cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gitweb: Fix "next" link on bottom of page
2008-06-02 9:54 ` [PATCH] gitweb: Fix "next" link on bottom of page Jakub Narebski
@ 2008-06-02 17:02 ` Gerrit Pape
0 siblings, 0 replies; 5+ messages in thread
From: Gerrit Pape @ 2008-06-02 17:02 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Kai Blin
On Mon, Jun 02, 2008 at 11:54:41AM +0200, Jakub Narebski wrote:
> This bug was caused by the fact that git_header_html() modified
> parameters in $cgi->param(...) when generating search form, and
> href(-replay, ...) uses paramemeters values from $cgi, not saved in
> variables (although it could).
>
> This fixes mentioned bug, not only in the case of 'log' view, but in
> all cases (although it is possible that this bug doesn't occur for
> other pages).
> gitweb/gitweb.perl | 13 +++++--------
> 1 files changed, 5 insertions(+), 8 deletions(-)
Hi, this fixes the bottom 'next' link in the log view for me, so
Tested-by: Gerrit Pape <pape@smarden.org>
Thanks, Gerrit.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-06-02 17:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-05-29 18:10 [PATCH] gitweb: fix "next" link on bottom of commit log page Gerrit Pape
2008-05-30 8:12 ` Jakub Narebski
2008-06-02 8:52 ` Jakub Narebski
2008-06-02 9:54 ` [PATCH] gitweb: Fix "next" link on bottom of page Jakub Narebski
2008-06-02 17:02 ` Gerrit Pape
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).