{"thread":{"id":"13724","subject":"[PATCH] gitweb: fix \"next\" link on bottom of commit log page","startedAt":"2008-05-29T18:10:03Z","lastAt":"2008-06-02T17:02:19Z","messageCount":5,"participants":["Gerrit Pape","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"78071","messageId":"20080529181003.1249.qmail@b35f4f9e60eb05.315fe32.mid.smarden.org","threadId":"13724","inReplyTo":null,"subject":"[PATCH] gitweb: fix \"next\" link on bottom of commit log page","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-05-29T18:10:03Z","receivedAt":"2008-05-29T18:10:03Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"When viewing a gitweb repository commit log, the \"next\" link at the top\nof the page works as expected, the \"next\" link on the bottom of the page\nhas a=search instead of a=log and thus fails to get you to the next\npage.  This commit replaces the bottom \"next\" link with the same links\nas shown at the top of the page.\n\nThe bad link was reported by Kai Blin through\n http://bugs.debian.org/481902\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n gitweb/gitweb.perl |    7 +------\n 1 files changed, 1 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 57a1905..f7b9ac2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4703,12 +4703,7 @@ sub git_log {\n \t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n \t\tprint \"</div>\\n\";\n \t}\n-\tif ($#commitlist >= 100) {\n-\t\tprint \"<div class=\\\"page_nav\\\">\\n\";\n-\t\tprint $cgi->a({-href => href(-replay=>1, page=>$page+1),\n-\t\t\t       -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n-\t\tprint \"</div>\\n\";\n-\t}\n+\tgit_print_page_nav('log','', $hash,undef,undef, $paging_nav);\n \tgit_footer_html();\n }\n \n-- \n1.5.5.2\n"},{"id":"78118","messageId":"m3y75s1a9v.fsf@localhost.localdomain","threadId":"13724","inReplyTo":"20080529181003.1249.qmail@b35f4f9e60eb05.315fe32.mid.smarden.org","subject":"Re: [PATCH] gitweb: fix \"next\" link on bottom of commit log page","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-30T08:12:49Z","receivedAt":"2008-05-30T08:12:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Gerrit Pape <pape@smarden.org> writes:\n\n> When viewing a gitweb repository commit log, the \"next\" link at the top\n> of the page works as expected, the \"next\" link on the bottom of the page\n> has a=search instead of a=log and thus fails to get you to the next\n> page.  This commit replaces the bottom \"next\" link with the same links\n> as shown at the top of the page.\n\n> -\t\tprint $cgi->a({-href => href(-replay=>1, page=>$page+1),\n> -\t\t\t       -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n\nShould not happen: href(-replay=>1, ...) should have the same value\nof 'a' parameter as the page it is in, so it should be 'log' not 'search'.\n\nWill investigate.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"78344","messageId":"m3d4n01anr.fsf@localhost.localdomain","threadId":"13724","inReplyTo":"m3y75s1a9v.fsf@localhost.localdomain","subject":"Re: [PATCH] gitweb: fix \"next\" link on bottom of commit log page","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-02T08:52:31Z","receivedAt":"2008-06-02T08:52:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Gerrit Pape <pape@smarden.org> writes:\n> \n> > When viewing a gitweb repository commit log, the \"next\" link at the top\n> > of the page works as expected, the \"next\" link on the bottom of the page\n> > has a=search instead of a=log and thus fails to get you to the next\n> > page.  This commit replaces the bottom \"next\" link with the same links\n> > as shown at the top of the page.\n> \n> > -\t\tprint $cgi->a({-href => href(-replay=>1, page=>$page+1),\n> > -\t\t\t       -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n> \n> Should not happen: href(-replay=>1, ...) should have the same value\n> of 'a' parameter as the page it is in, so it should be 'log' not 'search'.\n> \n> Will investigate.\n\nNow I know what is happening.  git_header_html() modifies parameters\nin $cgi, and href(-replay, ...) uses paramemeters values from $cgi,\nnot saved in variables (although it could).\n\nSo the correct solution would be to change the part which generates\nsearch form in git_header_html() to _not_ modify $cgi->params().\n\nPatch will follow...\n\n\nP.S. I wonder if using Test::WWW::Mechanize (from CPAN) to test\ngitweb's HTML output would be a good idea...\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"78346","messageId":"20080602095348.16843.24351.stgit@localhost.localdomain","threadId":"13724","inReplyTo":"m3y75s1a9v.fsf@localhost.localdomain","subject":"[PATCH] gitweb: Fix \"next\" link on bottom of page","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-02T09:54:41Z","receivedAt":"2008-06-02T09:54:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Fix search form generation to not modify $cgi->param(...)'s.\n\nIn git_header_html() we used to use $cgi->hidden(-name => \"a\") etc. to\ngenerate hidden fields; unfortunately to use this form it is required\nto modify $cgi->param(\"a\") etc., which makes href(-replay,...) use\nwrong replay values.  This for example made the \"next\" link on the\nbottom of the page has a=search instead of a=$action, and thus fails to\nget you to the next page.\n\nBecause in CGI the value of a hidden field is \"sticky\", there is no\nway to modify it short of modifying $cgi->param(...).  Therefore it\ngot replaced by generating <input type=\"hidden\" ...> element [semi]\ndirectly.\n\nAlternate solution would be for href(-replay,...) to use values saved\nin global variables, such as $action etc., instead of (re)reading them\nfrom $cgi->param($symbol).\n\nThe bad link was reported by Kai Blin through\n  http://bugs.debian.org/481902\n\nReported-by: Kai Blin <kai.blin@gmail.com>\nNoticed-by: Gerrit Pape <pape@smarden.org>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nJakub Narebski <jnareb@gmail.com> wrote:\n> Gerrit Pape <pape@smarden.org> writes:\n> \n>> When viewing a gitweb repository commit log, the \"next\" link at the top\n>> of the page works as expected, the \"next\" link on the bottom of the page\n>> has a=search instead of a=log and thus fails to get you to the next\n>> page.  This commit replaces the bottom \"next\" link with the same links\n>> as shown at the top of the page.\n> \n>> -\t\tprint $cgi->a({-href => href(-replay=>1, page=>$page+1),\n>> -\t\t\t       -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n> \n> Should not happen: href(-replay=>1, ...) should have the same value\n> of 'a' parameter as the page it is in, so it should be 'log' not 'search'.\n\nThis bug was caused by the fact that git_header_html() modified\nparameters in $cgi->param(...) when generating search form, and\nhref(-replay, ...) uses paramemeters values from $cgi, not saved in\nvariables (although it could).\n\nThis fixes mentioned bug, not only in the case of 'log' view, but in\nall cases (although it is possible that this bug doesn't occur for\nother pages).\n\n[I'm sorry if I have send this patch twice.]\n\n gitweb/gitweb.perl |   13 +++++--------\n 1 files changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex dd0f0ac..50cde3b 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2622,7 +2622,7 @@ EOF\n \tprint \"</div>\\n\";\n \n \tmy ($have_search) = gitweb_check_feature('search');\n-\tif ((defined $project) && ($have_search)) {\n+\tif (defined $project && $have_search) {\n \t\tif (!defined $searchtext) {\n \t\t\t$searchtext = \"\";\n \t\t}\n@@ -2638,16 +2638,13 @@ EOF\n \t\tmy ($use_pathinfo) = gitweb_check_feature('pathinfo');\n \t\tif ($use_pathinfo) {\n \t\t\t$action .= \"/\".esc_url($project);\n-\t\t} else {\n-\t\t\t$cgi->param(\"p\", $project);\n \t\t}\n-\t\t$cgi->param(\"a\", \"search\");\n-\t\t$cgi->param(\"h\", $search_hash);\n \t\tprint $cgi->startform(-method => \"get\", -action => $action) .\n \t\t      \"<div class=\\\"search\\\">\\n\" .\n-\t\t      (!$use_pathinfo && $cgi->hidden(-name => \"p\") . \"\\n\") .\n-\t\t      $cgi->hidden(-name => \"a\") . \"\\n\" .\n-\t\t      $cgi->hidden(-name => \"h\") . \"\\n\" .\n+\t\t      (!$use_pathinfo &&\n+\t\t      $cgi->input({-name=>\"p\", -value=>$project, -type=>\"hidden\"}) . \"\\n\") .\n+\t\t      $cgi->input({-name=>\"a\", -value=>\"search\", -type=>\"hidden\"}) . \"\\n\" .\n+\t\t      $cgi->input({-name=>\"h\", -value=>$search_hash, -type=>\"hidden\"}) . \"\\n\" .\n \t\t      $cgi->popup_menu(-name => 'st', -default => 'commit',\n \t\t                       -values => ['commit', 'grep', 'author', 'committer', 'pickaxe']) .\n \t\t      $cgi->sup($cgi->a({-href => href(action=>\"search_help\")}, \"?\")) .\n"},{"id":"78384","messageId":"20080602170219.8487.qmail@86b125e51c7491.315fe32.mid.smarden.org","threadId":"13724","inReplyTo":"20080602095348.16843.24351.stgit@localhost.localdomain","subject":"Re: [PATCH] gitweb: Fix \"next\" link on bottom of page","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-06-02T17:02:19Z","receivedAt":"2008-06-02T17:02:19Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Mon, Jun 02, 2008 at 11:54:41AM +0200, Jakub Narebski wrote:\n> This bug was caused by the fact that git_header_html() modified\n> parameters in $cgi->param(...) when generating search form, and\n> href(-replay, ...) uses paramemeters values from $cgi, not saved in\n> variables (although it could).\n> \n> This fixes mentioned bug, not only in the case of 'log' view, but in\n> all cases (although it is possible that this bug doesn't occur for\n> other pages).\n\n>  gitweb/gitweb.perl |   13 +++++--------\n>  1 files changed, 5 insertions(+), 8 deletions(-)\n\nHi, this fixes the bottom 'next' link in the log view for me, so\n\nTested-by: Gerrit Pape <pape@smarden.org>\n\nThanks, Gerrit.\n"}]}