762783 - Cannot view dependent bugs when there are many

Status: RESOLVED FIXED

(Bugzilla :: Dependency Views, defect)

Description

[Simon Green]

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

[Simon Green]

Attachment: 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

[Simon Green]

Patch is against trunk, but will work with 4.2 as well.

Comment 3

[Simon Green]

Attachment: v2 patch

Uses CGI_URI_LIMIT and fixes up some indentation issues.

Comment 4

[:glob ✱]

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]

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]

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]

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

[Simon Green]

Attachment: 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

[:glob ✱]

Comment on attachment 632054 [details] [diff] [review]
v3 patch

r=glob

Comment 10

[Simon Green]

As per comment #5, requesting approval for v4.2 release too.

  -- simon

Comment 11

[Frédéric Buclin]

(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

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified template/en/default/bug/dependency-tree.html.tmpl
Committed revision 8100.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/bug/dependency-tree.html.tmpl
Committed revision 8280.