Closed Bug 308253 Opened 19 years ago Closed 16 years ago

Ability to add select (enum) fields to a bug whose list of values depends on the value of another field

Categories

(Bugzilla :: Bugzilla-General, enhancement)

2.21
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.4

People

(Reporter: mkanat, Assigned: mkanat)

References

(Blocks 2 open bugs)

Details

(Keywords: selenium)

Attachments

(3 files, 6 obsolete files)

It would be nice to have a custom field type of a <select> box whose <option> values depend on another field. That is, "I want to make a per-product field," or "I want to make a per-component" field, or "I want to make a per-version field." This is all handled by having "a field that depends on another field." We don't need to get too complex with this, but we will need to genericize the JavaScript that we have on query.cgi that changes value lists depending on what product you choose.
This functionality would be most useful and we would use it as soon as it becomes available. The user should be able to change the default value of the field if they so choose to however.
Attached patch Monolithic Patch (obsolete) (deleted) — Splinter Review
Here's a monolithic patch that does it. It needs to be split into several separate bugs and reviewed in those.
Assignee: general → mkanat
Status: NEW → ASSIGNED
Blocks: 26940
Blocks: 79964
Could the select box also support multi-select rather than just a single-select?
(In reply to comment #3) > Could the select box also support multi-select rather than just a > single-select? We could do that in a separate bug. Do you mean the child being a multi-select, or the parent being a multi-select? I have code somewhere for the child being a multi-select, but not for the parent--that's way more complex.
(In reply to comment #4) > (In reply to comment #3) > > Could the select box also support multi-select rather than just a > > single-select? > > We could do that in a separate bug. Do you mean the child being a > multi-select, or the parent being a multi-select? I have code somewhere for the > child being a multi-select, but not for the parent--that's way more complex. I mean the child being multi-select. My main interest is I want the list of values to change based on the product selected (or the component selected)
(In reply to comment #5) > I mean the child being multi-select. My main interest is I want the list of > values to change based on the product selected (or the component selected) Yeah, that's possible. Like I said, I already have code that does it, so maybe I'll even include it in this patch. That would be fixing two things in one patch, though, so I might separate it out into two patches.
(In reply to comment #6) > (In reply to comment #5) > > I mean the child being multi-select. My main interest is I want the list of > > values to change based on the product selected (or the component selected) > > Yeah, that's possible. Like I said, I already have code that does it, so > maybe I'll even include it in this patch. That would be fixing two things in > one patch, though, so I might separate it out into two patches. Cool, this means we could migrate Versions and Milestones to use the new custom fields.
Attached patch Work In Progress (obsolete) (deleted) — Splinter Review
Okay, here's what I have so far. I thought about implementing it so that each value could be controlled by a different field, but that got too complex. So we just have one field that controls one field's values, and each value sets which value shows it.
Attachment #257195 - Attachment is obsolete: true
Eventually we'll probably want to add the ability to say "this field shows up when the value is X or Y or Z", but that's in the future, if it's needed by enough people.
Attached patch v1 (works in non-IE browsers) (obsolete) (deleted) — Splinter Review
Here's a totally complete version of this patch...that doesn't work in IE. It works everywhere else, as far as I know. Apparently IE doesn't support "display: none" on <option> tags, and it also doesn't support the disabled attribute (these facts are even true in IE7). The solution that I have now is elegant. It just doesn't work in IE. There are hacky workarounds for IE7 to enable the disabled attribute, but they are...well, bad from a user perspective (they let you select the item and then revert it after you've selected it). And anyhow, I want to *hide* the item, not just disable it. I don't want to have to carry around a data structure like we do for classifications, products, and components, because then I have to add JavaScript all over the place for fields. :-( I may be forced to do that, though, to make this stuff work in IE.
Attachment #342551 - Attachment is obsolete: true
Target Milestone: --- → Bugzilla 4.0
One option may be to programatically replace the <option> tag with an empty <optgroup> that contains the name of the option, when the option is disabled, and swap it out again for an <option> when it's enabled. I'd have to probably use DOM methods to do it, but it could be done...
Attached patch Work In Progress 3 (obsolete) (deleted) — Splinter Review
This one works pretty much on IE, but apparently still needs some bug fixes.
Attachment #344596 - Attachment is obsolete: true
Attached patch v2 (obsolete) (deleted) — Splinter Review
OK! This works in IE! And I didn't have to re-work everything. :-) Basically, what I do in IE is I replace hidden <option> tags with a comment node in the DOM, which works in IE6 and above. This is also necessary in Safai/Chrome, apparently (though at least WebKit supports the "disabled" option on <option> tags). You'll notice that I had to make some changes in Install::DB about some old status_workflow code--that code was using a Bugzilla::Status function, and Install::DB can't use objects. In this case Bugzilla::Status now expects a visibility_value_id field to be in the bug_status table, but it's not there yet at that point of Install::DB, so I had to use manual code instead of using a Bugzilla::Status code.
Attachment #345255 - Attachment is obsolete: true
Attachment #345900 - Flags: review?(bbaetz)
Comment on attachment 345900 [details] [diff] [review] v2 As discussed on IRC, doesn't seem to work when the page loads initially - 'hidden' fields are hidden Can't add a new cf (Can't use string ("Bugzilla::Field") as a HASH ref while "strict refs" in use at Bugzilla/Field.pm line 373.) but that may not be this patch - can't trivially test due to the schema change. Since I can't create new cfs, can't do additional testing. The code itself looks fine, though
Attachment #345900 - Flags: review?(bbaetz) → review-
Attached patch v3 (obsolete) (deleted) — Splinter Review
Ah, okay, I fixed all the problems you found.
Attachment #345900 - Attachment is obsolete: true
Attachment #346859 - Flags: review?(bbaetz)
Attached patch v3 (bzr) (deleted) — Splinter Review
Here's v3 diffed with bzr instead of cvs, so you can interdiff it with the previous patches (at least, I hope!).
Attachment #346859 - Attachment is obsolete: true
Attachment #346860 - Flags: review?(bbaetz)
Attachment #346859 - Flags: review?(bbaetz)
Blocks: 463598
Comment on attachment 346860 [details] [diff] [review] v3 (bzr) >=== modified file 'Bugzilla/Field.pm' >--- Bugzilla/Field.pm 2008-10-25 04:14:56 +0000 >+++ Bugzilla/Field.pm 2008-11-07 10:28:50 +0000 > >+ my $type = $params->{type}; >+ $params->{value_field_id} = >+ $class->_check_value_field_id($params->{value_field_id}, >+ ($type == FIELD_TYPE_SINGLE_SELECT >+ || $type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0); Use is_select >=== modified file 'js/field.js' >--- js/field.js 2008-10-25 04:11:30 +0000 >+++ js/field.js 2008-11-07 10:28:50 +0000 >@@ -359,3 +359,125 @@ >+function showOptionInIE(aNode, aSelect) { >+ if (browserCanHideOptions(aSelect)) return; >+ // If aNode is an Option >+ alert(aNode.value + " type: " + typeof(aNode.value)); and remove this Tested on FF and IE, and it seems to work. r=bbaetz with that
Attachment #346860 - Flags: review?(bbaetz) → review+
Flags: approval+
I can't use is_select there, there's no object in existence to call it on. I fixed the alert. Checking in editfields.cgi; /cvsroot/mozilla/webtools/bugzilla/editfields.cgi,v <-- editfields.cgi new revision: 1.11; previous revision: 1.10 done Checking in editvalues.cgi; /cvsroot/mozilla/webtools/bugzilla/editvalues.cgi,v <-- editvalues.cgi new revision: 1.38; previous revision: 1.37 done Checking in Bugzilla/Field.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field.pm,v <-- Field.pm new revision: 1.38; previous revision: 1.37 done Checking in Bugzilla/Status.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Status.pm,v <-- Status.pm new revision: 1.11; previous revision: 1.10 done Checking in Bugzilla/DB/Schema.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v <-- Schema.pm new revision: 1.108; previous revision: 1.107 done Checking in Bugzilla/Field/Choice.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field/Choice.pm,v <-- Choice.pm new revision: 1.7; previous revision: 1.6 done Checking in Bugzilla/Install/DB.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v <-- DB.pm new revision: 1.58; previous revision: 1.57 done Checking in js/field.js; /cvsroot/mozilla/webtools/bugzilla/js/field.js,v <-- field.js new revision: 1.12; previous revision: 1.11 done Checking in js/util.js; /cvsroot/mozilla/webtools/bugzilla/js/util.js,v <-- util.js new revision: 1.6; previous revision: 1.5 done Checking in skins/standard/global.css; /cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v <-- global.css new revision: 1.55; previous revision: 1.54 done Checking in template/en/default/admin/custom_fields/cf-js.js.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/custom_fields/cf-js.js.tmpl,v <-- cf-js.js.tmpl new revision: 1.3; previous revision: 1.2 done Checking in template/en/default/admin/custom_fields/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/custom_fields/create.html.tmpl,v <-- create.html.tmpl new revision: 1.12; previous revision: 1.11 done Checking in template/en/default/admin/custom_fields/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/custom_fields/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.12; previous revision: 1.11 done Checking in template/en/default/admin/fieldvalues/confirm-delete.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl,v <-- confirm-delete.html.tmpl new revision: 1.13; previous revision: 1.12 done Checking in template/en/default/admin/fieldvalues/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/create.html.tmpl,v <-- create.html.tmpl new revision: 1.12; previous revision: 1.11 done Checking in template/en/default/admin/fieldvalues/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.14; previous revision: 1.13 done RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field-events.js.tmpl,v done Checking in template/en/default/bug/field-events.js.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field-events.js.tmpl,v <-- field-events.js.tmpl initial revision: 1.1 done Checking in template/en/default/bug/field.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v <-- field.html.tmpl new revision: 1.21; previous revision: 1.20 done Checking in template/en/default/bug/knob.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/knob.html.tmpl,v <-- knob.html.tmpl new revision: 1.39; previous revision: 1.38 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.90; previous revision: 1.89 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.79; previous revision: 1.78 done Checking in template/en/default/global/user-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v <-- user-error.html.tmpl new revision: 1.266; previous revision: 1.265 done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: relnote
Resolution: --- → FIXED
Target Milestone: Bugzilla 4.0 → Bugzilla 3.4
We need to document this. docs/en/html/custom-fields.html says nothing about this feature, and when I selected a field, I wondered "So what? The UI remains the same; no new form appears", till I realized I had to go to editvalues.cgi to see the new UI. Also, I second what Albert says. Not being able to say "this value should be displayed only when the other field has values value1, or value2, or value3, ..." is annoying (e.g. display only when Product = Core or Firefox). Could someone file a separate bug for this (if not filed yet)?
Flags: documentation?
Yes, please do file a separate bug for that. It should be a relatively easy modification now that we have the basic framework in place.
Thanks Max for all the legwork! Does this patch handle multi-select or just single select enum fields?
(In reply to comment #22) > Thanks Max for all the legwork! Does this patch handle multi-select or just > single select enum fields? It handles both.
Flags: testcase?
Added to the release notes for Bugzilla 3.4 in bug 494037.
Keywords: relnote
Blocks: 516692
Blocks: 522971
Attached patch Selenium script for 4.2 (deleted) — Splinter Review
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.2/ modified t/test_custom_fields.t Committed revision 216.
Attached patch Selenium script for 4.0 (deleted) — Splinter Review
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.0/ modified t/test_custom_fields.t modified t/lib/QA/Util.pm Committed revision 203.
Documentation added to bug 707170. While writing tests for this feature, I found a bug related to custom field values which depend on the value of another field: when you select such a value and commit your changes, this value is not always selected correctly when you visit the bug again. The source code has selected="selected" for this value, but it's not the one being displayed in the web browser. If I turn off JS in my web browser, then this value is correctly displayed. More information in the regression bug I will file in a minute.
Flags: testcase?
Flags: testcase+
Flags: documentation?
Flags: documentation+
Keywords: selenium
Blocks: 707428
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: