Last Comment Bug 762783 - Cannot view dependent bugs when there are many
: Cannot view dependent bugs when there are many
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Dependency Views (show other bugs)
: 3.6
: All All
: -- minor (vote)
: Bugzilla 4.2
Assigned To: mail
: default-qa
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-07 22:33 PDT by mail
Modified: 2012-06-28 21:52 PDT (History)
2 users (show)
LpSolit: approval+
LpSolit: approval4.2+
LpSolit: blocking4.2.2-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 patch (1.82 KB, patch)
2012-06-07 22:37 PDT, mail
no flags Details | Diff | Splinter Review
v2 patch (1.93 KB, patch)
2012-06-07 23:29 PDT, mail
glob: review+
Details | Diff | Splinter Review
v3 patch (1.62 KB, patch)
2012-06-11 16:16 PDT, mail
glob: review+
Details | Diff | Splinter Review

Description mail 2012-06-07 22:33:42 PDT
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.
Comment 1 mail 2012-06-07 22:37:28 PDT
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.
Comment 2 mail 2012-06-07 22:38:15 PDT
Patch is against trunk, but will work with 4.2 as well.
Comment 3 mail 2012-06-07 23:29:31 PDT
Created attachment 631283 [details] [diff] [review]
v2 patch

Uses CGI_URI_LIMIT and fixes up some indentation issues.
Comment 4 Byron Jones ‹:glob› [PTO until 2017-01-09] 2012-06-07 23:59:16 PDT
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.
Comment 5 Frédéric Buclin 2012-06-08 05:59:58 PDT
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 6 Frédéric Buclin 2012-06-08 09:03:03 PDT
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.
Comment 7 Frédéric Buclin 2012-06-08 09:04:18 PDT
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.
Comment 8 mail 2012-06-11 16:16:22 PDT
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 9 Byron Jones ‹:glob› [PTO until 2017-01-09] 2012-06-28 01:30:24 PDT
Comment on attachment 632054 [details] [diff] [review]
v3 patch

r=glob
Comment 10 mail 2012-06-28 02:30:19 PDT
As per comment #5, requesting approval for v4.2 release too.

  -- simon
Comment 11 Frédéric Buclin 2012-06-28 03:59:44 PDT
(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.
Comment 12 Byron Jones ‹:glob› [PTO until 2017-01-09] 2012-06-28 21:52:54 PDT
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/4.2/
modified template/en/default/bug/dependency-tree.html.tmpl
Committed revision 8100.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/bug/dependency-tree.html.tmpl
Committed revision 8280.

Note You need to log in before you can comment on or make changes to this bug.