users@javaserverfaces-spec-public.java.net

[jsr344-experts mirror] [jsr344-experts] Re: Re: [ADMIN] Spec snapshot 20120924

From: Werner Punz <werner.punz_at_gmail.com>
Date: Wed, 24 Oct 2012 11:16:07 +0200

Hello after rereading the entire section I have to apologize, I was
mislead but, i am somewhat clearer,
sorry for my first mail which was caused a little bit by my
misunderstanding probably also because
I was already thinking this fixes the multiforms issue we have had since
2.0 which it does not.

I personally think for xml correctnes this thing works, but it does not
work out for multiple form scenarii:
Lets rereview some parts

We have the change form <update id="javax.faces.ViewState"> to

|<update id="<VIEW_ROOT_CONTAINER_CLIENT_ID><SEP>javax.faces.ViewState<SEP><UNIQUE_PER_VIEW_NUMBER>">
    <![CDATA[...]]>
</update>

For XML correctness this is ok.
|

>locate and update the submitting form's |javax.faces.ViewState| value
with the |CDATA| contents from
> the response. <SEP>: is the currently configured (sidenote also the
render targets are updated as > fix for the multiple forms scenarii)

So far so good this is the old behavior, as we have it, which is broken
in itself, because the multiple forms scenarii break here.
(only the issuing form is updated which causes an old viewstate in case
of a second form)

Then we have the section:

Locate and update the |javax.faces.ViewState| value for all forms
specified in the |render| target list.

This fixes the multiple forms scenario in a way that you can add other
forms to the render targets,
this is quite unsatisfying because you have to specify render targets
for every form you want to update.
(I introduced in myfaces a config param no_portlet_environment (aka
update_all_forms)
to bypass the issue, because I got a lot of angry user mails regarding
this problem
who felt that the render target solution was unsatisfying)
The reason for this was that we did not have any viewroot information at
that stage in the viewstate part
of the spec, this has changed.

Now we have already the viewroot information and thus a dedicated
information which section of the page
is affected by the viewstate change, wouldnt it be simpler to update
simply all the forms within the viewroot
and maybe optionally the render targets? Or is there a reason why we
still stick with the old broken
behavior of not simply updating all forms under viewroot but just the
issuing form?
The question then also is do we still need the render target update as
well?

My personal preference to this section would be simply:

|/<update id="<VIEW_ROOT_CONTAINER_CLIENT_ID><SEP>javax.faces.ViewState<SEP><UNIQUE_PER_VIEW_NUMBER>">
    <![CDATA[...]]>
</update>//
/
/locate and update the issuing form and all forms under/|||/VIEW_ROOT_CONTAINER_CLIENT_ID.//
/
||

and maybe


/||Locate and update the |javax.faces.ViewState| value for all forms
specified in the |render| target list.
||/
For backwards compatibility reasons.
I am not sure whether this should be discussed in here or later after
the first public draft, I just wanted
to add this comment as a sidenote because I see still no fix for the
multiform scenario here as it
was introduced in 2.0 and then later somewhat a little bit fixed but not
satisfyingly in 2.0a/2.1.
And now that we have the viewroot information in place I guess it would
be time to fix this as well.


Werner


Am 23.10.12 22:55, schrieb Edward Burns:
>>>>>> On Mon, 01 Oct 2012 12:33:33 +0200, Werner Punz <werner.punz_at_gmail.com> said:
> WP> Ok I found another issue jsdoc section ViewState handling:
>>> If an |update| element is found in the response with an identifier
>>> containing |javax.faces.ViewState|:
>>> |<update id="<VIEW_ROOT_CONTAINER_CLIENT_ID><SEP>javax.faces.ViewState<SEP><UNIQUE_PER_VIEW_NUMBER>">
>>> <![CDATA[...]]>
>>> </update>|
>>> locate and update the submitting form's |javax.faces.ViewState| value
>>> with the |CDATA| contents from the response. <SEP>: is the currently
>>> configured |UINamingContainer.getSeparatorChar()|.
>>> <VIEW_ROOT_CONTAINER_CLIENT_ID> is the return from
>>> |UIViewRoot.getContainerClientId()| on the view from whence this state
>>> originated. <UNIQUE_PER_VIEW_NUMBER> is a number that must be unique
>>> within this view, but must not be included in the view state. This
>>> requirement is simply to satisfy XML correctness in parity with what
>>> is done in the corresponding non-partial JSF view. Locate and update
>>> the |javax.faces.ViewState| value for all forms specified in the
>>> |render| target list
> WP> Now if we introduce a viewroot container client id, as far as I
> WP> understood the original discussion, we do not need that anymore, we do
> WP> only update the forms within the given viewroot (this would solve the
> WP> multiple forms problem as well), maybe I misunderstood the discussion
>
> But we *have* introduced the VIEW_ROOT_CONTAINER_CLIENT_ID. There is no
> "if".
>
> "We do not need that anymore", I'm sorry, what do you mean by "that"?
>
> WP> From what I can gather here, the Viewroot is not really used and we
> WP> default back to the original broken implementation of using render
> WP> targets and origin forms in a multiple form scenario. The question is,
> WP> given the original discussion of introducing this mechanism to be xml
> WP> correct for multiple forms, do we still need the render target mechanism
> WP> here, wouldnt it be simpler just to update all forms within a given
> WP> ViewRoot?
>
> At this point, I'm not sure. Given where we are in the schedule, unless
> you feel that the current draft specification is broken, I'd really like
> to keep it as is.
>
> Ed
>