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

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement, P3)

Description

[Nick Hibma]

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.

Comment 1

[Nick Hibma]

Make the URL a FQHN

Comment 2

[Dave Miller [:justdave]]

Attachment: The patch found at the URL in the URL field, just so we don't lose it.

Comment 3

[Andreas Franke (gone)]

-> Bugzilla product, Creating Bugs component, reassigning.

Comment 4

[Gervase Markham [:gerv]]

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

Comment 5

[Dave Miller [:justdave]]

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.

Comment 6

[John Vandenberg]

Attachment: templatised version

Comment 7

[Bradley Baetz (:bbaetz)]

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?

Comment 8

[John Vandenberg]

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

Comment 9

[Bradley Baetz (:bbaetz)]

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

[John Vandenberg]

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

[John Vandenberg]

Attachment: v4: small fixup

Comment 12

[Bradley Baetz (:bbaetz)]

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?

Comment 13

[Jouni Heikniemi]

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

At least it's bitrotten.

Comment 14

[Dave Miller [:justdave]]

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.

Comment 15

[Wayne Mery (:wsmwk)]

(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?

Comment 16

[Frédéric Buclin]

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).

Comment 17

[Olav Vitters]

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

Comment 18

[Frédéric Buclin]

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

Comment 19

[Max Kanat-Alexander]

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?

Comment 20

[Frédéric Buclin]

(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

[Max Kanat-Alexander]

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.

Comment 22

[Frédéric Buclin]

(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.

Comment 23

[Frédéric Buclin]

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

Comment 24

[Max Kanat-Alexander]

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.

Comment 25

[Frédéric Buclin]

Attachment: patch, v5.2

I use a <fieldset> to display the component description. Do you like it better?

Comment 26

[Max Kanat-Alexander]

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.

Comment 27

[Frédéric Buclin]

Attachment: patch, v6

Let's hope that's the final one. I fixed the incorrect filtering of the component description, as mkanat noted on IRC.

Comment 28

[Max Kanat-Alexander]

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\"".

Comment 29

[Frédéric Buclin]

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

Comment 30

[Max Kanat-Alexander]

Added to the Bugzilla 3.2 release notes in bug 432331.