User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20100101 Firefox/12.0 Build ID: 20120424151700 Steps to reproduce: In the dependency tree view, there are two links (if you have the editbugs privilege, otherwise one link) that links to buglist.cgi. The URL has a comma separated list of bug ids. Actual results: If the URL is bigger than 2,048 characters, you get a "414 Request URI too long" HTTP error Expected results: The bug list should show.
Created attachment 631273 [details] [diff] [review] v1 patch This patch will force the URL to post if the URL would be too long. I am using a table so that we can keep the submit buttons aligned.
Patch is against trunk, but will work with 4.2 as well.
Created attachment 631283 [details] [diff] [review] v2 patch Uses CGI_URI_LIMIT and fixes up some indentation issues.
Comment on attachment 631283 [details] [diff] [review] v2 patch r=glob this is fine; although lines 87-91 and 93 needs to be indented one more level, which can be fixed when committing.
Not a blocker as this problem has always been here. And to trigger this problem, you would need several hundreds of bugs in the dependency list, which is not that common.
Comment on attachment 631283 [details] [diff] [review] v2 patch >=== modified file 'template/en/default/bug/dependency-tree.html.tmpl' >+ [% use_post = (ids.join(",").length > constants.CGI_URI_LIMIT - 32 ) ? 1 : 0 %] What's the rationale for removing 32 bits from the length? To take the length of the domain name into account? I wouldn't worry about that. >+ [% IF use_post %] >+ <form action="buglist.cgi" method="post"> >+ <input type="hidden" name="bug_id" value="[% ids.join(",") %]"> You don't need two separate forms. This would avoid duplicating these lines. Instead of using <input> for buttons, simply use <button>; they will be inlined automatically and are more powerful than <input>. For instance, you can replace: >+ <input type="hidden" name="tweak" value="1"> >+ <input type="submit" value="change several"> by: <button type="submit" name="tweak" value="1">Change several</button> This way, if you click this button, tweak=1 will be automatically passed to the CGI script, which is the reason why you don't need two forms. This would also skip the need to use a table. Also, I think we don't need parens to be displayed when using the form, which will make your code even smaller. Please update your patch accordingly.
Please re-request approval once the patch has been updated and reviewed. If the patch is small enough, we could take it for 4.2.2.
Created attachment 632054 [details] [diff] [review] v3 patch Patch with improvements suggested by LpSolit. The top part of the the <form has to appear before the "Ip to X levels", otherwise the line splits on the browser. -- simon
Comment on attachment 632054 [details] [diff] [review] v3 patch r=glob
As per comment #5, requesting approval for v4.2 release too. -- simon
(In reply to Simon Green from comment #10) > As per comment #5, requesting approval for v4.2 release too. In comment 5, I said this was not a blocker.
Committing to: bzr+ssh://firstname.lastname@example.org/bugzilla/4.2/ modified template/en/default/bug/dependency-tree.html.tmpl Committed revision 8100. Committing to: bzr+ssh://email@example.com/bugzilla/trunk/ modified template/en/default/bug/dependency-tree.html.tmpl Committed revision 8280.