On enter_bug.cgi, show component description when it is selected

RESOLVED FIXED in Bugzilla 3.2

Status

()

P3
enhancement
RESOLVED FIXED
18 years ago
10 years ago

People

(Reporter: n_hibma, Assigned: LpSolit)

Tracking

3.1.2
Bugzilla 3.2
Bug Flags:
approval +

Details

Attachments

(1 attachment, 8 obsolete attachments)

(Reporter)

Description

18 years ago
When selecting a module to report a bug for, it is convenient to be able to 
select the program as well.

This incurs an extra database query though. Maybe it should be an option.

Patch against today's bugzilla in URL.
Created attachment 40610 [details] [diff] [review]
The patch found at the URL in the URL field, just so we don't lose it.
Keywords: patch, review
Target Milestone: --- → Bugzilla 2.16

Comment 3

17 years ago
-> Bugzilla product, Creating Bugs component, reassigning.
Assignee: tara → myk
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: other → unspecified

Updated

17 years ago
Whiteboard: enter_bug
As I understand this bug, it doesn't scale the way it's done. However, having
Component selection, with descriptions, as a separate page might cut down the
number of misfiled bugs. That's certainly worth considering.

Also, this patch is obsoleted by templatisation. If we still want this, we need
to write it again. Removing patch keyword.

Gerv
Keywords: patch, review
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18

Comment 6

17 years ago
Created attachment 69780 [details] [diff] [review]
templatised version
Comment on attachment 69780 [details] [diff] [review]
templatised version

>Index: enter_bug.cgi
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v
>retrieving revision 1.60
>diff -u -r1.60 enter_bug.cgi
>--- enter_bug.cgi	2002/02/04 21:17:10	1.60
>+++ enter_bug.cgi	2002/02/16 00:13:41
>@@ -59,7 +60,7 @@
> # user is right from the start. 
> confirm_login() if (Param("usebuggroupsentry"));
> 
>-if (!defined $::FORM{'product'}) {
>+if (!defined $::FORM{'component'} || !defined $::FORM{'component'}) {

That bit is wrong....

>Index: template/default/global/choose_product.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/default/global/choose_product.tmpl,v
>retrieving revision 1.1
>diff -u -r1.1 choose_product.tmpl
>--- template/default/global/choose_product.tmpl	2002/02/04 21:17:17	1.1
>+++ template/default/global/choose_product.tmpl	2002/02/16 00:13:42
>@@ -23,19 +23,40 @@
> 
> <table>
> 
>-[% FOREACH p = proddesc.keys.sort %]
>-  <tr>
>-    <th align="right" valign="top">
>-      <a href="[% target %]?product=[% p FILTER uri %]">
>-        [% p FILTER html %]</a>:
>-    </th>
>+[% IF product %]
>+  [% FOREACH c = components.keys.sort %]
>+    <tr>
>+      <td></td>
>+      <th align="right" valign="top">
>+        <a href="[% target %]?product=[% product FILTER uri %]&component=[% c FILTER uri %]">
>+          [% c FILTER html %]</a>:
>+      </th>

And this bit is less general, and more complicated - the product is fixed, so
you could just rename
the proddesc key in the template to something more general, and hardcode the
product, removing the
need for an if statement.

However, I disagree with this change anyway - it would be a pain on bmo which
has a very large number
of components, which you would have to page though.
Maybe do this as part of an "easy bug entry" type thing, which I'm sure we have
bugs on?
Attachment #69780 - Flags: review-

Comment 8

17 years ago
Created attachment 69839 [details] [diff] [review]
v2: review fixes + param

Thanks for the review and templating tip.

That b.m.o. has a lot of components is precisely the reason why this should be
used, to ensure the person entering the bug is provided with enough information
to choose correctly, otherwise the first reasonable option is choosen,
resulting in bugs getting moved around needlessly.

This is not about making bug entry 'easier', but rather more difficult and with
a little more process, to reduce load on developers.  I have no doubt that
b.m.o. has a large number of users who would find this an annoying feature, but
of that same group I am sure you will find many that bitch about ppl entering
bugs in the wrong components.  This could be solved with a pref.

Updated

17 years ago
Attachment #69839 - Flags: review-
Comment on attachment 69839 [details] [diff] [review]
v2: review fixes + param

I'm not sure about the param name. I haven't tested this, but won't this code
not get reached if there is only one product? There $prodsize will be == 1, so
the elsif branch won't be taken. You should also short circuit this logic for
the one-component case, and move this test to below this area, bascially
duplicating it.

Re the template, what I meant was keeping the existing template as is, and just
rename proddesc to something like description (remember to change the other
user of this, describecomponents.cgi). Change the targeturi so that it has to
include the ?, and then just poke it with the product in enter_bug.

Actually, thinking about this a bit more, why not just use the
describe-components template? OTOH, that gives the qa contact and asignee,
which you probably don't want.

Comment 10

17 years ago
Created attachment 69896 [details] [diff] [review]
v3

Sorry it took twice to sink in.  The template would read better if I used the
URL plugin for TT, but I am guessing that would create more problems that it
solves, and besides, I need sleep.

Comment 11

17 years ago
Created attachment 70278 [details] [diff] [review]
v4: small fixup

Updated

16 years ago
Attachment #69896 - Attachment is obsolete: true
Comment on attachment 70278 [details] [diff] [review]
v4: small fixup

This is basically done with gerv's simple enter_bug page if you have js
enabled; do we still want this?

Updated

16 years ago
Attachment #69839 - Attachment is obsolete: true

Updated

16 years ago
Attachment #69780 - Attachment is obsolete: true

Updated

16 years ago
Attachment #40610 - Attachment is obsolete: true

Updated

16 years ago
Attachment #70278 - Flags: review?

Comment 13

15 years ago
Comment on attachment 70278 [details] [diff] [review]
v4: small fixup

At least it's bitrotten.
Attachment #70278 - Flags: review? → review-
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22

Comment 15

13 years ago
(In reply to comment #12)
> (From update of attachment 70278 [details] [diff] [review] [edit])
> This is basically done with gerv's simple enter_bug page if you have js
> enabled; do we still want this?

close?
(Assignee)

Comment 16

13 years ago
The trunk is now frozen to prepare Bugzilla 2.22. Only bug fixes are accepted, no enhancement bugs. As this bug has a pretty low activity (especially from the assignee), it's retargetted to ---. If you want to work on it and you think you can have it fixed for 2.24, please retarget it accordingly (to 2.24).
Target Milestone: Bugzilla 2.22 → ---

Updated

13 years ago
QA Contact: mattyt-bugzilla → default-qa

Comment 17

12 years ago
*** Bug 329788 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Summary: make enter_bug.cgi list the programs next to the modules. → On enter_bug.cgi, show component description when it is selected
(Assignee)

Updated

12 years ago
Assignee: myk → create-and-change
(Assignee)

Updated

12 years ago
Depends on: 376673
(Assignee)

Comment 18

11 years ago
Created attachment 283958 [details] [diff] [review]
patch, v5

I reorganize fields in a way which I think 1) makes more sense, 2) allows me to display the description of components and 3) will let us easily implement bug 376673 as the expert_fields class is already in place (a good example on how bug 376673 will look like is to add .expert_fields { display: none; } into global.css).
Assignee: create-and-change → LpSolit
Attachment #70278 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #283958 - Flags: review?(mkanat)
Attachment #283958 - Flags: review?(bugzilla-mozilla)
(Assignee)

Updated

11 years ago
Blocks: 376673
No longer depends on: 376673
Whiteboard: enter_bug
Target Milestone: --- → Bugzilla 3.2

Comment 19

11 years ago
Comment on attachment 283958 [details] [diff] [review]
patch, v5

I'm pretty sure you can't have two <tbodys> in a <table>, even if that works.

I'm also not sure that all browsers will respect the existence of the <tbody> and be able to hide it. If you can, test Firefox, IE, and Safari.

Could I see this in action on a public test install?
(Assignee)

Comment 20

11 years ago
(In reply to comment #19)
> (From update of attachment 283958 [details] [diff] [review])
> I'm pretty sure you can't have two <tbodys> in a <table>, even if that works.

Of course you can: http://www.w3.org/TR/html4/struct/tables.html#h-11.2.3

"Table rows may be grouped into a table head, table foot, and one or more table body sections, using the THEAD, TFOOT and TBODY elements, respectively."

And the example there shows two <tbody> in the same table.


> I'm also not sure that all browsers will respect the existence of the <tbody>
> and be able to hide it. If you can, test Firefox, IE, and Safari.

I tested with Firefox, IE and Opera. All three respect <tbody> and the ability to hide the one with the .expert_fields class.

Comment 21

11 years ago
Okay. The code looks OK by my brief inspection, but it'd be nice to have a test installation that this was working on so I could see it.
(Assignee)

Comment 22

11 years ago
(In reply to comment #21)
> Okay. The code looks OK by my brief inspection, but it'd be nice to have a test
> installation that this was working on so I could see it.

Use https://landfill.bugzilla.org/bz57842. I added:

[% style = Bugzilla.cgi.param("hide") ? ".expert_fields {display: none;}" : "" %]

at the top of the template (not part of the patch) so that you can easily test to hide fields having the .expert_fields class. All you have to do is to append &hide=1 to the URL.
(Assignee)

Comment 23

11 years ago
Created attachment 283982 [details] [diff] [review]
patch, v5.1

For the record, the patch on landfill has a slight change compared to v5:

   [% IF !Param('defaultplatform') || !Param('defaultopsys') %]
     <tr>
-      <th colspan="2">&nbsp;</th>
-      <td class="comment">
+      <th>&nbsp;</th>
+      <td colspan="3" class="comment">
         We've made a guess at your
         [% IF Param('defaultplatform') %]
           operating system. Please check it

This patch includes this change.
Attachment #283958 - Attachment is obsolete: true
Attachment #283982 - Flags: review?(mkanat)
Attachment #283958 - Flags: review?(mkanat)
Attachment #283958 - Flags: review?(bugzilla-mozilla)

Comment 24

11 years ago
Comment on attachment 283982 [details] [diff] [review]
patch, v5.1

Great, thank you.

The "Information" box appears too far away from the "Component" box for me to understand that it's describing the Component.

Also, it should have a different name than "Information," perhaps "Component Description".

Ideally it would appear right next to or right underneath the Component box. Perhaps some colspans would do it.
Attachment #283982 - Flags: review?(mkanat) → review-
(Assignee)

Comment 25

11 years ago
Created attachment 284151 [details] [diff] [review]
patch, v5.2

I use a <fieldset> to display the component description. Do you like it better?
Attachment #283982 - Attachment is obsolete: true
Attachment #284151 - Flags: review?(mkanat)

Comment 26

11 years ago
Comment on attachment 284151 [details] [diff] [review]
patch, v5.2

Wrap the fieldset in <table><tr><td> </td></tr></table>, which will make it auto-size to its content, which looks a lot nicer.

Maybe you could even fit the whole fieldset inside the Componen column, and move up the other fields to make the page even smaller.
Attachment #284151 - Flags: review?(mkanat) → review-
(Assignee)

Comment 27

11 years ago
Created attachment 284231 [details] [diff] [review]
patch, v6

Let's hope that's the final one. I fixed the incorrect filtering of the component description, as mkanat noted on IRC.
Attachment #284151 - Attachment is obsolete: true
Attachment #284231 - Flags: review?(mkanat)

Comment 28

11 years ago
Comment on attachment 284231 [details] [diff] [review]
patch, v6

Yeah, looks good.

My only nit is that I prefer 'selected="selected"' instead of "selected=\"selected\"".
Attachment #284231 - Flags: review?(mkanat) → review+

Updated

11 years ago
Flags: approval+
Version: unspecified → 3.1.2
(Assignee)

Comment 29

11 years ago
Checking in skins/standard/global.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v  <--  global.css
new revision: 1.41; previous revision: 1.40
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.79; previous revision: 1.78
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: relnote
Resolution: --- → FIXED

Comment 30

10 years ago
Added to the Bugzilla 3.2 release notes in bug 432331.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.