Cannot view dependent bugs when there are many

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Dependency Views
--
minor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Simon Green, Assigned: Simon Green)

Tracking

Bugzilla 4.2
Bug Flags:
approval +
approval4.2 +
blocking4.2.2 -

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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.
Assignee: dependency.views → sgreen+mozilla
Attachment #631273 - Flags: review?
(Assignee)

Comment 2

5 years ago
Patch is against trunk, but will work with 4.2 as well.
(Assignee)

Updated

5 years ago
Summary: Dependency tree links gibe 414 error when too many bugs → Cannot view dependent bugs when there are many
(Assignee)

Comment 3

5 years ago
Created attachment 631283 [details] [diff] [review]
v2 patch

Uses CGI_URI_LIMIT and fixes up some indentation issues.
Attachment #631273 - Attachment is obsolete: true
Attachment #631273 - Flags: review?
Attachment #631283 - Flags: review?
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.
Attachment #631283 - Flags: review? → review+
Flags: approval?
(Assignee)

Updated

5 years ago
Flags: blocking4.2.2?
Flags: approval4.2?

Comment 5

5 years ago
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.
Severity: normal → minor
Status: NEW → ASSIGNED
Flags: blocking4.2.2? → blocking4.2.2-

Comment 6

5 years ago
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

5 years ago
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.
Flags: approval?
Flags: approval4.2?
(Assignee)

Comment 8

5 years ago
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
Attachment #631283 - Attachment is obsolete: true
Attachment #632054 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #632054 - Flags: review? → review?(glob)
Comment on attachment 632054 [details] [diff] [review]
v3 patch

r=glob
Attachment #632054 - Flags: review?(glob) → review+
Flags: approval?
(Assignee)

Comment 10

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

  -- simon
Flags: blocking4.2.2- → blocking4.2.2?

Comment 11

5 years ago
(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.
Flags: blocking4.2.2?
Flags: blocking4.2.2-
Flags: approval?
Flags: approval4.2+
Flags: approval+
Target Milestone: --- → Bugzilla 4.2
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.