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

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

From: Leonardo Uribe <leonardo.uribe_at_irian.at>
Date: Thu, 15 Dec 2016 00:41:02 -0500

Hi

+1

Thanks Ed for reviewing this.

regards,

Leonardo Uribe

2016-12-14 17:39 GMT-05:00 Edward Burns <edward.burns_at_oracle.com>:

> >>>>> 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
>