orbeon / orbeon-forms

Orbeon Forms is an open source web forms solution. It includes an XForms engine, the Form Builder web-based form editor, and the Form Runner runtime.
http://www.orbeon.com/
GNU Lesser General Public License v2.1
518 stars 220 forks source link

Empty, non-required field must be valid, even it is has unmet constraints #3156

Open avernet opened 7 years ago

avernet commented 7 years ago

Create a form in Form Builder, make a field not required, and either:

  1. Say that field must be an integer; run the form, leave the field empty ⇒ it is valid; good.
  2. Say that the field minimum length is 5; run the form, leave the field empty ⇒ it is invalid; mmmh.

This calls for changing the behavior in no 2 to make an optional and empty fields consistently valid in all cases. Since this might change the behavior of existing forms, we might want to have this new behavior only for new forms created after we've made this change.

avernet commented 7 years ago

+1 from community

ebruchez commented 7 years ago

+1 from customer

ebruchez commented 7 years ago

XForms says this:

An instance node is valid if and only if the following conditions hold:

  • the constraint model item property is true
  • the value is non-empty if the required model item property is true
  • the node satisfies all applicable types (including those associated by the type model item property, by an external or an inline XML Schema, or by xsi:type).

There is no indication that an optional-but-empty field would be considered valid. This explains why we implemented validation this way.

Yet we think it would be reasonable to say that an optional field is valid when blank, independently from other constraints and types.

This would not be backward compatible, so would have to be expressed with a flag. This flag could be:

For our needs, it is enough to make it per form. New forms created by Form Builder would have the flag set, while older forms would not, and so remain compatible. Possibly, the Form Builder UI could be used to toggle that flag.

ebruchez commented 7 years ago

How hard is it to implement this?

ValidationBindOps.applyValidationBinds() and then validateTypeAndRequired() set the list of invalid instances with:

if (! typeValidity || ! requiredValidity)

This would have to be changed as the instances would be valid.

validateConstraint runs in any case in order to set common constraints properties and in order to handle the switch of validity of datatype.

Also check ValidationBindOps.validateConstraint which sets invalid instances.

InstanceData.getValid tests for:

That would have to be changed as well.

Check also uses of the following:

ebruchez commented 7 years ago

Form-level attribute name suggestion: xxf:empty-optional-valid="true". The default would be false for backward compatibility.

ebruchez commented 7 years ago

Suggested to the XForms Working Group that maybe we could introduce an optional attribute which would trigger the new behavior.

ebruchez commented 7 years ago

Example:

<xf:bind
    ref       ="quantity"
    required  ="false()    (: or just missing as `false()` is the default :)"
    type      ="xf:integer (: so the field remains valid when empty       :)"
    constraint="normalize-space(string(.)) = '' or . gt 0"/>

becomes:

<xf:bind
    ref       ="quantity"
    required  ="false()    (: or just missing as `false()` is the default :)"
    type      ="xs:integer (: keep the standard type                      :)"
    constraint=". gt 0     (: just focus on the constraint logic          :)"/>

or:

<xf:bind
    ref       ="quantity"
    optional  ="true()     (: triggers new behavior                       :)"
    type      ="xs:integer (: keep the standard type                      :)"
    constraint=". gt 0     (: just focus on the constraint logic          :)"/>
ebruchez commented 7 years ago

As an aside, with XPath 2, the data(.) function is more explicit to retrieve the typed value of the node instead of relying on atomization:

data(.) gt 0

And with XPath 3, you can use simply use data():

data() gt 0
ebruchez commented 7 years ago
ebruchez commented 7 years ago

We would like to have this fixed in XForms. Initial impression (see minutes) is good: we should do this. optional attribute might be confusing, so probably a no-go. Possibly XForms might use the XForms version="2.0" attribute to enable this.

ebruchez commented 7 years ago

2017-07-05: XForms deciding that the behavior of required will change in 2.0. In our case, we will introduce a flag to enable the new behavior, for example xxf:empty-optional-valid="true" as suggested above.

New forms created with Form Builder would have the flag set to true. Old forms could opt in via form setting.

avernet commented 7 years ago

+1 from user

ebruchez commented 6 years ago

Some in progress work on 2017-05-09-3156-wip.

ebruchez commented 6 years ago

A really important one to do, but unlikely to make it for 2018.1

ebruchez commented 5 years ago

Possible +1 from customer.

ebruchez commented 5 years ago

It would be really great to do this!

ebruchez commented 4 years ago

+1 from customer

ebruchez commented 4 years ago

As a first step, for common constraints, we could have a separate fix. For example, they could all be valid if the node is optional.

If we do that, we might want a backward compatibility mode?

ebruchez commented 4 years ago

+1 from customer

avernet commented 3 years ago

+1 from user

ebruchez commented 1 year ago

+1 from customer

avernet commented 2 months ago

+1 from customer