{"thread":{"id":"44489","subject":"gitweb html validation","startedAt":"2016-11-15T15:52:01Z","lastAt":"2016-11-18T20:06:09Z","messageCount":5,"participants":["Raphaël Gertz","Ralf Thielow","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"305960","messageId":"13c9b4a74d82a1f0ed3f626406a43e92@rapsys.eu","threadId":"44489","inReplyTo":null,"subject":"gitweb html validation","fromName":"Raphaël Gertz","fromEmail":"mageia@rapsys.eu","sentAt":"2016-11-15T15:13:23Z","receivedAt":"2016-11-15T15:52:01Z","isPatch":false,"sender":{"key":"mageia@rapsys.eu","avatar":null},"body":"Hi,\n\nThere a small bug in gitweb html validation, you need the following \npatch to pass w3c check with searchbox enabled.\n\nThe problem lies in the input directly embed inside a form without a \nwrapper which is not valid.\n\nBest regards\n\nThe following patch fix the issue for git-2.10.2 :\n--- /usr/share/gitweb/gitweb.cgi.orig   2016-11-15 15:37:21.149805026 \n+0100\n+++ /usr/share/gitweb/gitweb.cgi        2016-11-15 15:37:48.579240429 \n+0100\n@@ -5518,6 +5518,7 @@ sub git_project_search_form {\n\n         print \"<div class=\\\"projsearch\\\">\\n\";\n         print $cgi->start_form(-method => 'get', -action => $my_uri) .\n+             '<div>'.\n               $cgi->hidden(-name => 'a', -value => 'project_list')  . \n\"\\n\";\n         print $cgi->hidden(-name => 'pf', -value => $project_filter). \n\"\\n\"\n                 if (defined $project_filter);\n@@ -5529,6 +5530,7 @@ sub git_project_search_form {\n                              -checked => $search_use_regexp) .\n               \"</span>\\n\" .\n               $cgi->submit(-name => 'btnS', -value => 'Search') .\n+             '</div>'.\n               $cgi->end_form() . \"\\n\" .\n               $cgi->a({-href => href(project => undef, searchtext => \nundef,\n                                      project_filter => \n$project_filter)},\n"},{"id":"305973","messageId":"20161115182632.GA17539@gmail.com","threadId":"44489","inReplyTo":"13c9b4a74d82a1f0ed3f626406a43e92@rapsys.eu","subject":"Re: gitweb html validation","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-11-15T18:26:32Z","receivedAt":"2016-11-15T18:26:43Z","isPatch":false,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Raphaël Gertz <mageia@rapsys.eu> wrote:\n> Hi,\n> \n> There a small bug in gitweb html validation, you need the following patch to\n> pass w3c check with searchbox enabled.\n> \n> The problem lies in the input directly embed inside a form without a wrapper\n> which is not valid.\n>\n\nI agree this is a small bug. Only block level elements are\nallowed to be inside form tags, according to\nhttps://www.w3.org/2010/04/xhtml10-strict.html#elem_form\n\n> Best regards\n> \n> The following patch fix the issue for git-2.10.2 :\n> --- /usr/share/gitweb/gitweb.cgi.orig   2016-11-15 15:37:21.149805026 +0100\n> +++ /usr/share/gitweb/gitweb.cgi        2016-11-15 15:37:48.579240429 +0100\n> @@ -5518,6 +5518,7 @@ sub git_project_search_form {\n> \n>         print \"<div class=\\\"projsearch\\\">\\n\";\n>         print $cgi->start_form(-method => 'get', -action => $my_uri) .\n> +             '<div>'.\n>               $cgi->hidden(-name => 'a', -value => 'project_list')  . \"\\n\";\n>         print $cgi->hidden(-name => 'pf', -value => $project_filter). \"\\n\"\n>                 if (defined $project_filter);\n> @@ -5529,6 +5530,7 @@ sub git_project_search_form {\n>                              -checked => $search_use_regexp) .\n>               \"</span>\\n\" .\n>               $cgi->submit(-name => 'btnS', -value => 'Search') .\n> +             '</div>'.\n>               $cgi->end_form() . \"\\n\" .\n>               $cgi->a({-href => href(project => undef, searchtext => undef,\n>                                      project_filter => $project_filter)},\n\nI think it's better to just move the <form>-Tag outside of the\nsurrounding div?\nSomething like this perhaps, I didn't test it myself yet.\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7cf68f07b..33d7c154f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5531,8 +5531,8 @@ sub git_project_search_form {\n \t\t$limit = \" in '$project_filter/'\";\n \t}\n \n-\tprint \"<div class=\\\"projsearch\\\">\\n\";\n \tprint $cgi->start_form(-method => 'get', -action => $my_uri) .\n+\t      \"<div class=\\\"projsearch\\\">\\n\" .\n \t      $cgi->hidden(-name => 'a', -value => 'project_list')  . \"\\n\";\n \tprint $cgi->hidden(-name => 'pf', -value => $project_filter). \"\\n\"\n \t\tif (defined $project_filter);\n@@ -5544,11 +5544,11 @@ sub git_project_search_form {\n \t                     -checked => $search_use_regexp) .\n \t      \"</span>\\n\" .\n \t      $cgi->submit(-name => 'btnS', -value => 'Search') .\n-\t      $cgi->end_form() . \"\\n\" .\n \t      $cgi->a({-href => href(project => undef, searchtext => undef,\n \t                             project_filter => $project_filter)},\n \t              esc_html(\"List all projects$limit\")) . \"<br />\\n\";\n-\tprint \"</div>\\n\";\n+\tprint \"</div>\\n\" .\n+\t      $cgi->end_form() . \"\\n\";\n }\n \n # entry for given @keys needs filling if at least one of keys in list\ndiff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\nindex 321260103..507740b6a 100644\n--- a/gitweb/static/gitweb.css\n+++ b/gitweb/static/gitweb.css\n@@ -539,7 +539,7 @@ div.projsearch {\n \tmargin: 20px 0px;\n }\n \n-div.projsearch form {\n+form div.projsearch {\n \tmargin-bottom: 2px;\n }\n \n\n"},{"id":"306017","messageId":"xmqqy40ks301.fsf@gitster.mtv.corp.google.com","threadId":"44489","inReplyTo":"20161115182632.GA17539@gmail.com","subject":"Re: gitweb html validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T00:05:34Z","receivedAt":"2016-11-16T00:05:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow <ralf.thielow@gmail.com> writes:\n\n> Only block level elements are\n> allowed to be inside form tags, according to\n> https://www.w3.org/2010/04/xhtml10-strict.html#elem_form\n> ...\n> I think it's better to just move the <form>-Tag outside of the\n> surrounding div?\n> Something like this perhaps, I didn't test it myself yet.\n\nThat sounds like a sensible update to me (no, I do not run gitweb\nmyself).  Is this the only <form> we have in the UI, or is it the\nonly one that is problematic?\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7cf68f07b..33d7c154f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -5531,8 +5531,8 @@ sub git_project_search_form {\n>  \t\t$limit = \" in '$project_filter/'\";\n>  \t}\n>  \n> -\tprint \"<div class=\\\"projsearch\\\">\\n\";\n>  \tprint $cgi->start_form(-method => 'get', -action => $my_uri) .\n> +\t      \"<div class=\\\"projsearch\\\">\\n\" .\n>  \t      $cgi->hidden(-name => 'a', -value => 'project_list')  . \"\\n\";\n>  \tprint $cgi->hidden(-name => 'pf', -value => $project_filter). \"\\n\"\n>  \t\tif (defined $project_filter);\n> @@ -5544,11 +5544,11 @@ sub git_project_search_form {\n>  \t                     -checked => $search_use_regexp) .\n>  \t      \"</span>\\n\" .\n>  \t      $cgi->submit(-name => 'btnS', -value => 'Search') .\n> -\t      $cgi->end_form() . \"\\n\" .\n>  \t      $cgi->a({-href => href(project => undef, searchtext => undef,\n>  \t                             project_filter => $project_filter)},\n>  \t              esc_html(\"List all projects$limit\")) . \"<br />\\n\";\n> -\tprint \"</div>\\n\";\n> +\tprint \"</div>\\n\" .\n> +\t      $cgi->end_form() . \"\\n\";\n>  }\n>  \n>  # entry for given @keys needs filling if at least one of keys in list\n> diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\n> index 321260103..507740b6a 100644\n> --- a/gitweb/static/gitweb.css\n> +++ b/gitweb/static/gitweb.css\n> @@ -539,7 +539,7 @@ div.projsearch {\n>  \tmargin: 20px 0px;\n>  }\n>  \n> -div.projsearch form {\n> +form div.projsearch {\n>  \tmargin-bottom: 2px;\n>  }\n>  \n"},{"id":"306037","messageId":"add4ba7fced68430c421c6b0ec388a13@rapsys.eu","threadId":"44489","inReplyTo":"xmqqy40ks301.fsf@gitster.mtv.corp.google.com","subject":"Re: gitweb html validation","fromName":"Raphaël Gertz","fromEmail":"mageia@rapsys.eu","sentAt":"2016-11-16T09:25:44Z","receivedAt":"2016-11-16T09:41:03Z","isPatch":false,"sender":{"key":"mageia@rapsys.eu","avatar":null},"body":"Le 16.11.2016 01:05, Junio C Hamano a écrit :\n> Ralf Thielow <ralf.thielow@gmail.com> writes:\n> \n>> Only block level elements are\n>> allowed to be inside form tags, according to\n>> https://www.w3.org/2010/04/xhtml10-strict.html#elem_form\n>> ...\n>> I think it's better to just move the <form>-Tag outside of the\n>> surrounding div?\n>> Something like this perhaps, I didn't test it myself yet.\n> \n> That sounds like a sensible update to me (no, I do not run gitweb\n> myself).  Is this the only <form> we have in the UI, or is it the\n> only one that is problematic?\n> \nThere is an other form in the cgi line 4110 :\n         print $cgi->start_form(-method => \"get\", -action => $action) .\n               \"<div class=\\\"search\\\">\\n\" .\n\nBut this one has a <div class=\"search\"> inside.\n\nThe problem with projsearch I want to change is that the div is around \nthe form without a container inside.\n\nI agree with moving the <div class=\"projsearch\"> inside the form if it's \na better option.\n\nBest regards\n"},{"id":"306162","messageId":"CAN0XMOLwZ9mpnCwO1JWMSYb42SU_T--fZ=aH+M0j8wfp15EGcw@mail.gmail.com","threadId":"44489","inReplyTo":"20161115182632.GA17539@gmail.com","subject":"Re: gitweb html validation","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-11-18T20:06:02Z","receivedAt":"2016-11-18T20:06:09Z","isPatch":false,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-11-15 19:26 GMT+01:00 Ralf Thielow <ralf.thielow@gmail.com>:\n\nFinally I've found the time to actually try this out and there\nare some problems with it.\n\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7cf68f07b..33d7c154f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -5531,8 +5531,8 @@ sub git_project_search_form {\n>                 $limit = \" in '$project_filter/'\";\n>         }\n>\n> -       print \"<div class=\\\"projsearch\\\">\\n\";\n>         print $cgi->start_form(-method => 'get', -action => $my_uri) .\n> +             \"<div class=\\\"projsearch\\\">\\n\" .\n>               $cgi->hidden(-name => 'a', -value => 'project_list')  . \"\\n\";\n>         print $cgi->hidden(-name => 'pf', -value => $project_filter). \"\\n\"\n>                 if (defined $project_filter);\n> @@ -5544,11 +5544,11 @@ sub git_project_search_form {\n>                              -checked => $search_use_regexp) .\n>               \"</span>\\n\" .\n>               $cgi->submit(-name => 'btnS', -value => 'Search') .\n> -             $cgi->end_form() . \"\\n\" .\n>               $cgi->a({-href => href(project => undef, searchtext => undef,\n>                                      project_filter => $project_filter)},\n>                       esc_html(\"List all projects$limit\")) . \"<br />\\n\";\n> -       print \"</div>\\n\";\n> +       print \"</div>\\n\" .\n> +             $cgi->end_form() . \"\\n\";\n>  }\n>\n\nThe anchor is now inside the form-tag, which means there is no\nvisual line-break anymore that comes automatically after </form>\nas the form-tag is a block level element.  Could be solved by adding a\n\"<br />\", which is not very nice, but OK.\n\n>  # entry for given @keys needs filling if at least one of keys in list\n> diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css\n> index 321260103..507740b6a 100644\n> --- a/gitweb/static/gitweb.css\n> +++ b/gitweb/static/gitweb.css\n> @@ -539,7 +539,7 @@ div.projsearch {\n>         margin: 20px 0px;\n>  }\n>\n> -div.projsearch form {\n> +form div.projsearch {\n>         margin-bottom: 2px;\n>  }\n>\n\nThis is wrong as it overwrites the setting above, 20px at the bottom\nwent to 2px.\n\nThe problem is how to apply the 2px now. Before this, we had the\n<form>, a block element, which we can give the 2px margin at the\nbottom.  Now this element is gone and we have a set of inline\nelements where we use \"<br />\" to emulate the line break.  There\nare two css rules which can solve this, but I'm not really happy with\nboth of them.\n1) As we know we need the two pixel below an input element, we\ncan say\ndiv.projsearch input {\n  margin-bottom: 2px;\n}\n2) Make the a-Tag inside div.projsearch being displayed as a block\nelement to make a margin-top setting working.  This has the benefit\nthat we don't care about the element above.\ndiv.projsearch a {\n        display: inline-block;\n        margin-top: 2px;\n}\nIf I have to choose I'd prefer the second one.\n\nSo I can't think of a way to solve this nicely with this change.\n"}]}