Closed Bug 1183398 Opened 5 years ago Closed 5 years ago

Mandatory custom fields block form submission if they are hidden and have no value

Categories

(Bugzilla :: Creating/Changing Bugs, defect, major)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 5.0

People

(Reporter: bvandermerwe, Assigned: LpSolit)

References

Details

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.132 Safari/537.36

Steps to reproduce:

We have a custom field named: cf_fix_in
Description: Build(s)\Version Fixed In
Type: Free Text
Is Mandatory: check
Field only appears when: Resolution (resolution)
Is set to any of: Fixed


Actual results:

If you look at a new bug where the status is NEW, enter a text comment, and click "Save changes", then nothing happens.

The browser output window complains about: An invalid form control with name='cf_fix_in' is not focusable.

If I change the custom field to always be visible, then it does not allow a change to be saved, showing a popup on this field asking for a value.


Expected results:

Since the field is not visible, it should not matter what the value is.

So it seems that it correctly decides to not show the field, but then incorrectly decides that it needs a value for it even though it is not visible.
This bug is preventing us from upgrading to bugzilla 5.0. If a fix or patch or work around could be made available, that would greatly help, please.
This also happens if you log a new bug. One work around is to set the resolution to fixed, then supply something for Build(s)\Version Fixed in, and change the resolution back to new. You can then enter comments and save them.

But the user interface is hiding this field from you correctly, but incorrectly wants a value for it anyway.
This was working fine in bugzilla 4.x. So it broken in the 5.0 release.
Bugzilla 5.0 adds the 'required' HTML5 attribute to mandatory custom fields, even if they are hidden. So the browser will refuse to submit the form if such fields have no value.

We could certainly fix this by removing the 'required' attribute on hidden fields when submitting the form, assuming events related to form submission are executed before the browser checks input data.
Assignee: general → create-and-change
Status: UNCONFIRMED → NEW
Component: Bugzilla-General → Creating/Changing Bugs
Ever confirmed: true
Target Milestone: --- → Bugzilla 5.0
Summary: Mandatory custom filed with conditional visibility prevent any bug change → Mandatory custom fields block form submission if they are hidden and have no value
Flags: blocking5.0.1?
Attached patch patch, v1 (obsolete) — Splinter Review
I use data-required to store the information about mandatory fields so that the 'required' attribute can be restored when mandatory fields are visible again. data-* are element attributes which can be used freely to store extra data, and are valid with HTML5.
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attachment #8633634 - Flags: review?(dkl)
Attached patch patch, v1.1 (obsolete) — Splinter Review
I forgot to fix the case where the field was initially hidden and never displayed.
Attachment #8633634 - Attachment is obsolete: true
Attachment #8633634 - Flags: review?(dkl)
Attachment #8633652 - Flags: review?(dkl)
Comment on attachment 8633652 [details] [diff] [review]
patch, v1.1

Thank you, the last patch appears to resolve the issue successfully and allows us to upgrade.
Duplicate of this bug: 1188915
Duplicate of this bug: 1190875
Hi Frederic,

I applied this patch to my sandbox "Clean" 5.0 install and it worked --> Great!!!

Then I went to my Sandbox "4.4.9" install and ran the 5.0 upgrade via git. Then ran the patch. My results were not so great.

I am still getting 11 of the "An invalid form control with name='cf_*' is not focusable" Errors

On the upside I was getting 79 of these errors.

Thanks,
Dean Price
Hi Frederic,

After doing some detective work in the DB... I have found that the errors are coming from the only custom fields that are of "Type 4" (Large Text Box)

I hope that helps you out

Thanks
Dean Price
This doesn't help me, no. :)

IMO, this is not specific to this field type. If you detected which custom field causes trouble, then look at the HTML source code of the form which your browser refuses to submit, check attributes of this (hidden?) field, and look at conditions under which this field is displayed.

When you say that your clean 5.0 installation works, do you mean you first recreated all the custom fields you had in 4.4.9 before doing testing? Else it's pretty hard to compare the data.
Hi Frederic,

What I found this AM is, of the 11 remaining errors I am getting,

Three of them have "showFieldWhen" statements in the html and those are dependant on other custom fields.

The rest of them do not have the "showFieldWhen" statement

Here is the section of code of one of the fields that do not have the "showFieldWhen" statement:
this is the only reference to this field in the html.

    <tr ><th class="field_label  bz_hidden_field required"
    id="field_label_cf_paareareq">

    <label for="cf_paareareq">

  <a 
      title="A description of why the request is being made"
      class="field_help_link"
      href="page.cgi?id=fields.html#cf_paareareq"
  >Reason for Request:</a>
</label>
</th>
  <td class="field_value  bz_hidden_field"
      id="field_container_cf_paareareq"  colspan="3">
       <div id="cf_paareareq_edit_container" class="bz_default_hidden">
         <div>
             (<a href="#" id="cf_paareareq_edit_action">edit</a>)
         </div>
       </div>
       <div id="cf_paareareq_input"><textarea name="cf_paareareq" id="cf_paareareq"
            rows="4"
          cols="60"
            onFocus="this.rows=8"
            aria-required="true" required></textarea>
       </div>
       <script type="text/javascript">
         hideEditableField('cf_paareareq_edit_container',
                           'cf_paareareq_input',
                           'cf_paareareq_edit_action',
                           'cf_paareareq',
                           '',
                           '',
                           true);
       </script>

</td>
    </tr>

let me know if I can provide you with anything else...
(In reply to Dean Price from comment #13)
> Here is the section of code of one of the fields that do not have the
> "showFieldWhen" statement:
> this is the only reference to this field in the html.
> 
>     <tr ><th class="field_label  bz_hidden_field required"
>     id="field_label_cf_paareareq">


Why does this field have both bz_hidden_field and required classes?? You say this field has no showFieldWhen condition attached to it, so why is it there at all? :) Can you tell me more about this custom field?
(In reply to Frédéric Buclin from comment #14)
> Why does this field have both bz_hidden_field and required classes??

Ah, I know why! :) New patch coming!
(In reply to Frédéric Buclin from comment #15)
> (In reply to Frédéric Buclin from comment #14)
> > Why does this field have both bz_hidden_field and required classes??
> 
> Ah, I know why! :) New patch coming!

Thank you ...
Attached patch patch, v2Splinter Review
Textarea fields are generated from a separate template, which is why I missed it in my previous patch.
Attachment #8633652 - Attachment is obsolete: true
Attachment #8633652 - Flags: review?(dkl)
Attachment #8643683 - Flags: review?(dkl)
> patch, v2
> 
> Textarea fields are generated from a separate template, which is why I
> missed it in my previous patch.

It seemed to have worked ... I will continue to test my sandbox upgrade 

Thank you much.
Duplicate of this bug: 1191668
Comment on attachment 8643683 [details] [diff] [review]
patch, v2

gerv, are you faster than dkl to review this? :)
Attachment #8643683 - Flags: review?(dkl) → review?(gerv)
Comment on attachment 8643683 [details] [diff] [review]
patch, v2

Review of attachment 8643683 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like hiddenness is toggleable in JS (see js/field.js) - is that like the "Advanced fields" twisty on the enter_bug page? Have I missed something, or does this mean that a user can avoid a field's mandatory-ness simply by hiding it? 

Gerv
No, a user cannot hide mandatory (custom) fields. But a mandatory custom field can be hidden if it's configured to be only displayed on some condition (for instance: custom field 'cf_foo' is only displayed if severity = critical). You configure these conditions from editfields.cgi.
Then what's the JS for?

Gerv
(In reply to Gervase Markham [:gerv] from comment #23)
> Then what's the JS for?

If a user set a field to a value which triggers displaying/hidding a custom field, then you need JS to trigger this change. In my example above, if the user select severity = critical, then JS must display the cf_foo custom field.
Comment on attachment 8643683 [details] [diff] [review]
patch, v2

Review of attachment 8643683 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like hiddenness is toggleable in JS (see js/field.js) - is that like the "Advanced fields" twisty on the enter_bug page? Have I missed something, or does this mean that a user can avoid a field's mandatory-ness simply by hiding it? 

Gerv
Attachment #8643683 - Flags: review?(gerv) → review+
Oops, ignore the comment there. Bugzilla "restored it from draft"...

Gerv
Thanks for the quick review. :)
Severity: normal → major
Flags: approval?
Flags: approval5.0?
Flags: blocking5.0.1?
Flags: blocking5.0.1+
Flags: approval?
Flags: approval5.0?
Flags: approval5.0+
Flags: approval+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   baed571..684bee4  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   a2ae154..6f616e1  5.0 -> 5.0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1057017
You need to log in before you can comment on or make changes to this bug.