jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [1433-UIInputRequiredTrue] PROPOSAL

From: Edward Burns <edward.burns_at_oracle.com>
Date: Wed, 14 Dec 2016 14:39:23 -0800

>>>>> On Wed, 30 Nov 2016 08:21:12 +0100, Bauke Scholtz <balusc_at_gmail.com> said:

BalusC> Proposal itself looks good

>>>>> On Wed, 30 Nov 2016 10:47:14 +0100, arjan tijms <arjan.tijms_at_gmail.com> said:

AT> An "always run validator when required is true" seems okay to me. I did
AT> spot a small typo in the text, as it currently says:

AT> " If the value of "required" is false, the required attribute is not set
AT> not set"

I've fixed that by editing the JIRA. Thanks.

>>>>> On Wed, 30 Nov 2016 17:32:44 -0500, Neil Griffin <neil.griffin_at_portletfaces.org> said:

NG> Q: Would it cause a backward compatibility problem if we simply made
NG> the following change?

NG> if ((submittedValue == null) && !isRequired()) {
NG> return;
NG> }

NG> In other words, is it necessary to introduce the context-param along
NG> with the fix?

Those of you who have worked with me over the years know that I don't
like to forcefully advocate for a particular approach, but in this case,
I feel that such a fundamental change must be guarded by an opt-in.

>>>>> On Wed, 30 Nov 2016 23:50:22 -0500, Leonardo Uribe <leonardo.uribe_at_irian.at> said:

LU> Is the change backward compatible with JSF 2.0/2.2 third party
LU> libraries? Yes, I think so, unless you have a component that does
LU> not extend from UIInput and implements EditableValueHolder. I prefer
LU> that compromise to another strange global web config param that will
LU> break later.

I see your point, but the key word here is "another". We already have
so many of these that one more isn't going to make a difference. I know
that's not the best argument, but I'm asking you to accept it.

>>>>> On Thu, 01 Dec 2016 07:30:34 +0000, Bauke Scholtz <balusc_at_gmail.com> said:

BalusC> Yes, but this indicaties a bad model. The normal solution would
BalusC> be to put a @NotNull constraint on the entity or a NOT NULL
BalusC> constraint on the column.

Please let's constrain the discussion to the original issue, which is
UIInput field validation.

>>>>> On Fri, 2 Dec 2016 13:28:38 -0500, Neil Griffin <neil.griffin_at_portletfaces.org> said:

NG> @Leonardo: +1 I think that you have identified an oversight in the
NG> Spec language. Also, I agree with you that we should avoid a global
NG> web config param if possible.

Yes, it is an oversight in the spec language. I judge this oversight is
adequately addressed by this change:

  Modify PDF section 3.5.4 to read:

    Spec> *The render-independent property required is a shorthand for the
    Spec> function of a required validator. If the value of this property is
    Spec> true, **there is an entry in the request payload corresponding to
    Spec> this component**, and the component has no value, the component is
    Spec> marked invalid and a message is added to the FacesContext
    Spec> instance.*

NG> @Leonardo: +1 I think that you are exactly right. If
NG> required="true", shouldn't the JSF developer be able to rely on the
NG> PROCESS_VALIDATIONS phase to enforce that? The plain meaning would
NG> be that the value is *always* required, without qualification or
NG> exception.

Think about that statement. Now think about all the JSF apps that are
already out there. We have always maintained that updating to a new JSF
version will not cause any breaking changes. We need to own that such a
change is a very low level change and it's quite possible it would have
unintended consequences.

>>>>> On Mon, 5 Dec 2016 23:33:51 -0500, Leonardo Uribe <leonardo.uribe_at_irian.at> said:

LU> So the answer is the change does not cause a backward compatibility
LU> problem, and based on that the web config parameter is not really
LU> necessary. Does it work in all cases? I'm not 100% sure, because we need
LU> to check what happens for menu/radio/checkbox, but if that so, apply a
LU> simple patch with just these lines could be a valid solution.

I'm not so confident in our test suite that knowing that it passes with
the change in place is enough to know that it will always pass with all
possible apps. As you said, you're not 100% sure, but if we make the
change an "opt in" change, we can be 100% sure.

>>>>> On Tue, 06 Dec 2016 07:08:51 +0000, Bauke Scholtz <balusc_at_gmail.com> said:

BalusC> For menus, radios and manycheckboxes, nothing will change as
BalusC> well. For booleancheckbox, it will fix the unintuitive behavior
BalusC> of a required=true to not have any effect.

Unintuitive, yes, but very long established. I mean, like, twelve years
ago established.

>>>>> On Wed, 7 Dec 2016 10:24:00 +0100, Bauke Scholtz <balusc_at_gmail.com> said:

BalusC> I can understand that, but IMO it should be done the other way
BalusC> round: use the flag to disable it. The default behavior should
BalusC> be the leading behavior in all cases (the same applies to other
BalusC> 2.3 things like enabling CDI resolver chain, by the way).

>>>>> On Wed, 7 Dec 2016 10:28:35 +0100, Bauke Scholtz <balusc_at_gmail.com> said:

BalusC> Coming back to enabling/disabling JSF 2.3 specific features, the
BalusC> <faces-config version> attribute should be used as a global
BalusC> flag. Developers who'd like to use JSF 2.3 in JSF 2.2 backwards
BalusC> compatibility mode could simply keep it to version="2.2". That's
BalusC> at least how it's supposed to be used (and has worked for all
BalusC> previous JSF versions and in all other Java EE APIs).

This is a larger issue for a separate thread. I'll start that thread now.

BalusC> As to the <h:selectBooleanCheckbox required="true"> misbehavior,
BalusC> this solution would fix
BalusC> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1289.

I would think the original proposal for 1433-UIInputRequiredTrue would
also fix this, right?

>>>>> On Wed, 7 Dec 2016 22:05:06 -0500, Leonardo Uribe <leonardo.uribe_at_irian.at> said:

LU> +1 to make it the default behavior, because the expected impact will
LU> be minimal in my opinion. The flag should be activated (disable) by
LU> the developer.

>>>>> On Thu, 8 Dec 2016 10:00:40 -0500, Neil Griffin <neil.griffin_at_portletfaces.org> said:

NG> +1 to make it the default behavior in 2.3 with a global config param
NG> to disable it.

>>>>> On Fri, 9 Dec 2016 23:19:29 -0600, Josh Juneau <juneau001_at_gmail.com> said:

JJ> +1, I agree that it should become the default behavior, with an
JJ> opportunity to opt out. I also agree with disabling by default if
JJ> it is back ported.

You're making it hard guys, but I still have to say -1. It's too
fundamental to be the default behavior. I'm surprised to see you
advocating so vociferously for this Neil.

LU> I do not like to add another param like
LU> javax.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL , but I
LU> agree it is the most practical solution in this moment.

I'm glad we agree at least on that point!

NG> I still recommend that this be backported to the 2.2 / 2.1 API
NG> source. In the backport case, it could be disabled by default, with
NG> a global com.sun.faces/org.apache.myfaces config param to enable it.

>>>>> On Tue, 6 Dec 2016 15:43:27 -0500, Neil Griffin <neil.griffin_at_portletfaces.org> said:

NG> If JAVASERVERFACES_SPEC_PUBLIC-1433 is adopted, then would anyone
NG> object to backporting this to the JSF 2.2 and 2.1 API source? My
NG> best guess is that it would be necessary to enable it with a
NG> com.sun.faces/org.apache.myfaces global web config param in 2.2/2.1
NG> unless we did an MR.

Therefore, I propose we simply provide the same fix I've proposed for
JSF 2.3 in the existing Mojarra releases, but without introducing the
symbolic constant ALWAYS_PERFORM_VALIDATION_WHEN_REQUIRED_IS_TRUE. In
other words, the context-param is there, it has the same name and
meaning, it's just not in the signatures.

Thanks,

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 37 business days until DevNexus 2017
| 62 business days until JavaLand 2017