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)
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)
(deleted),
patch
|
bbaetz
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•19 years ago
|
||
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.
Assignee | ||
Comment 2•18 years ago
|
||
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
Comment 3•17 years ago
|
||
Could the select box also support multi-select rather than just a single-select?
Assignee | ||
Comment 4•17 years ago
|
||
(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.
Comment 5•17 years ago
|
||
(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)
Assignee | ||
Comment 6•17 years ago
|
||
(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.
Comment 7•17 years ago
|
||
(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.
Assignee | ||
Comment 9•16 years ago
|
||
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
Assignee | ||
Comment 10•16 years ago
|
||
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.
Assignee | ||
Comment 11•16 years ago
|
||
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
Assignee | ||
Updated•16 years ago
|
Target Milestone: --- → Bugzilla 4.0
Assignee | ||
Comment 12•16 years ago
|
||
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...
Assignee | ||
Comment 13•16 years ago
|
||
This one works pretty much on IE, but apparently still needs some bug fixes.
Attachment #344596 -
Attachment is obsolete: true
Assignee | ||
Comment 14•16 years ago
|
||
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 15•16 years ago
|
||
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-
Assignee | ||
Comment 16•16 years ago
|
||
Ah, okay, I fixed all the problems you found.
Attachment #345900 -
Attachment is obsolete: true
Attachment #346859 -
Flags: review?(bbaetz)
Assignee | ||
Comment 17•16 years ago
|
||
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)
Comment 18•16 years ago
|
||
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+
Assignee | ||
Updated•16 years ago
|
Flags: approval+
Assignee | ||
Comment 19•16 years ago
|
||
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
Comment 20•16 years ago
|
||
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?
Assignee | ||
Comment 21•16 years ago
|
||
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.
Comment 22•16 years ago
|
||
Thanks Max for all the legwork! Does this patch handle multi-select or just single select enum fields?
Assignee | ||
Comment 23•16 years ago
|
||
(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.
Updated•16 years ago
|
Flags: testcase?
Assignee | ||
Comment 24•15 years ago
|
||
Added to the release notes for Bugzilla 3.4 in bug 494037.
Keywords: relnote
Comment 25•13 years ago
|
||
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.2/
modified t/test_custom_fields.t
Committed revision 216.
Comment 26•13 years ago
|
||
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.
Comment 27•13 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•