Hi
2013/3/13 Edward Burns <edward.burns_at_oracle.com>:
>>>>>> On Sun, 10 Mar 2013 17:44:32 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
>
> LU> In my opinion, if we can solve this list and the issues related to
> LU> faces flow (OutcomeTarget navigation, nested flow return, ... ), I
> LU> can finally give my +1 (non binding) to the spec.
>
> Thank you for spelling out so clearly your conditions for your +1. I
> will try my best to meet all of them.
>
> LU> MISCELANEOUS
>
> LU> - Some events were added in JSF 2.2:
>
> LU> PostKeepFlashValueEvent
> LU> PostPutFlashValueEvent
> LU> PreClearFlashEvent
> LU> PreRemoveFlashValueEvent
>
> LU> But section 3.4 doesn't mention them. In theory it should be an
> LU> small mention in section 3.4.3.1
>
> Fixed.
>
> LU> - JSF 2.2 section 7.1.1 has a spelling bug just before the end:
>
> LU> "... See the Javadocs for getActionListener() for important
> LU> backwards compatability information. ..."
>
> LU> It should be "compatibility".
>
> Fixed.
>
> LU> - JSF 2.2 section 7.6.2.3 says that getActionURL() requires to add
> LU> the CSRF token as query parameter, but is really that necessary? In
> LU> theory, getActionURL is used in forms, that are executed through a
> LU> POST request, protected by javax.faces.ViewState parameter (note
> LU> this is not true if stateless views uses another generic token as
> LU> hidden value for javax.faces.ViewState). Only if getActionURL is
> LU> used by something else it has sense. Instead, getBookmarkableURL()
> LU> should do that.
>
> We need it to be the case that every URL rendered by JSF is brought
> under the control of view protection. This is why I put it in
> getActionURL().
>
Ok, In theory there is no problem, just that according to the spec,
on POST request the param will be ignored, but that's ok.
> LU> - JSF 2.2 section 7.7.2.5 maybe should include UIViewAction description,
> LU> in the same way UIViewParameter is described.
>
> Fixed.
>
> LU> - JSF 2.2 section 11.1.3 In javax.faces.FACELETS_DECORATORS it must be
> LU> clear what happen when this param is not set. The javadoc of TagDecorator
> LU> says "... The runtime must provide a default implementation of this
> LU> interface that performs the following actions ..." ,but if a custom
> LU> TagDecorator is defined with this parameter, what happens? it is possible
> LU> to create a wrapper for TagDecorator?
>
> LU> Note a previous email was sent related to this problem:
>
> LU> [jsr344-experts] javax.faces.view.facelets.TagDecorator
>
> Fixed, per earlier email.
Yes, it was already discussed, but the spec requires to define how the
param works.
I remember this description:
FC>> In Mojarra you can't apply multiple TagDecorator implementations to one
FC>> tag (which is stated in the javadoc of TagDecorator). Once a TagDecorator
FC>> successfully decorated a tag, the process is ended. The only exception
FC>> is the DefaultTagDecorator, which handles the new jsf: syntax. It runs
FC>> first and then the users tag decorators will be applied.
Note the default TagDecorator is special, and that should be mentioned. Can
the default TagDecorator be overriden?.
>
> LU> - JSF 2.2 section 11.4.5 : new factories were added like FlashFactory and
> LU> FlowHandlerFactory. This part should be updated to reflect these changes.
>
> Assuming you meant 11.4.6, the section where we list what each kind of
> factory is supposed to do, fixed.
>
> LU> - JSF 2.2 section 11.4.7 Delegating Implementation Support
>
> LU> Is FlowHandlerFactory decoratable? if that so, it should be included in the
> LU> list.
>
> Yes it is. Fixed.
>
> LU> - JSF 2.2 section 7.7.2.1 has an spelling bug on "... and store the results
> LU> as the values of the locale and renderKitId, proeprties ..." It should say
> LU> "properties".
>
> Fixed.
>
> LU> RESOURCE LIBRARY CONTRACTS
>
> LU> - JSF 2.2 section 5.6.2.5 Resource ELResolver
>
> LU> In getValue description it says something like this:
>
> EB> "... If property contains a single colon character ':', treat the
> EB> content before the ':' as the libraryOrContractName and the content
> EB> after the ':' as the resourceName and pass both to
> EB> ResourceHandler.createResource(resourceName,
> EB> libraryOrContractName). If the value of libraryOrContractName is the
> EB> literal string "this" (without the quotes), discover the library
> EB> name of the current resource (or the contract name of the current
> EB> resource, the two are mutually exclusive) and replace "this" with
> EB> that library name (or contract name) before calling
> EB> ResourceHandler.createResource( ). If property contains more than
> EB> one colon character ':', throw a localized ELException, including
> EB> property ..."
>
> LU> The problem here is the contractName is something that is implicit derived
> LU> from the view itself (using the viewId and then calling
> LU> VDL.calculateResourceLibraryContracts(FacesContext context, String viewId) )
> LU> and it does not have any use on the implicit notation, because the idea
> LU> of a resource library contract is to be on top of the resource resolution.
>
> I have found that, in practice, this was necessary to allow refering to
> resources from CSS files from within a contract.
>
Suppose we have something this:
contractA
logo.png
contractB
logo.png
and we try to resolve #{resource['logo.png']}
and both contracts are active, which one is selected?
In theory if two contracts are used both should be mutually exclusive and
situations like this should be avoided. In that sense it is better to
organize it like this:
contractA
mylibA
logo.png
contractB
mylibB
logo.png
And reference each logo as #{resource['mylibA:logo.png']} and
#{resource['mylibB:logo.png']}.
Ok, maybe the syntax without libraryName has sense, but if that so,
shouldn't be valid to use this syntax too?
.someclass {
background-image:url("#{resource['contractA:images:logo.png']}");
}
because you can define a library inside a contract, right?. I think
for consistency
this should be allowed. If only one ':' is found try with
libraryName/resourceName
then with contractName/resourceName.
> [...]
>
> LU> In conclusion, the description on JSF 2.2 section 5.6.2.5 should be
> LU> removed.
>
> Well, we can't remove it because the ResourceELResolver is something we
> added in JSF 2.0. I have put the text back to its state from the most
> recent published JCP final document, the JSF 2.1 MR release from
> 20101108. I have also added this text at the end of that paragraph (the
> one that starts with "if property contains a single colon...":
>
> In the case of resource library contracts, libraryName will actually be
> the contract name.
>
Ok, but note that a "resource library contract" is not the same as a
"resource library". Maybe it is missing a ResourceHandler.createResource()
that can point to an specific "contract".
> LU> - Javadoc of ResourceHandler.createViewResource() still has a pending part.
> LU> In theory, it should scan META-INF/contracts/[contract-name],
> LU> META-INF/flows/[flow-name] and webapp context. It should say something like
> LU> "... the default implementation must ....".
>
> I have fixed this and will commit it with the rest of the changes in
> response to this email.
>
Ok.
> LU> - UIViewRoot does not have a method to store the associated resource library
> LU> contracts once are calculated. This is valid if and only if the result of
> LU> VDL.calculateResourceLibraryContracts() is the same over the application
> LU> lifetime, otherwise there is a risk of make some views unstable. Note
> LU> ResourceHandler requires call this method to derive the contract in the
> LU> same way it is done for the locale, so at the end the preferred way will be
> LU> store the result of the call inside UIViewRoot using a custom attribute,
> LU> so if the attribute is present, ResourceHandler will get the value and avoid
> LU> the calculation.
>
> We specifically discussed this and decided to make the FacesContext be
> the place where this information is stored.
>
But note state saving algorithm requires to save it and restore it.
Also, when navigation
is performed, it requires to be calculated and stored with the view.
I still feel weird to have something that is related to the view and
that needs to be
saved/restored with the view outside it.
> LU> JSF 2.2 section 10.1.3.2 says this:
>
> LU> "... The specification of the tag handler for <f:view> is the one other place
> LU> where the resourceLibraryContracts property may be set. This behavior is
> LU> normatively specified in the tag handler for <f:view>. ..."
>
> LU> The facelets javadoc of f:view defines an attribute called contracts:
>
> LU> "... A comma separated list of resource library contracts that may be used from
> LU> within the Facelets chain. If this attribute is present, it must only be on the
> LU> outer-most file in the chain of files that started ultimately with a call to
> LU> ViewDeclarationLanguage.createView(). Any use of this attribute on a
> LU> non-outer-most file must be silently ignored. ..."
>
> LU> If in f:view there exists the attribute "contracts" and the spec says that a
> LU> contract may be set in that location, the missing part is the description that
> LU> should say "copy the result of calculateResourceLibraryContracts() in some
> LU> UIViewRoot attribute". JSF 2.2 section 7.7.2.1 says that it is necessary
> LU> to call calculateResourceLibraryContracts() but it doesn't say what to do with
> LU> the result. According to the intention written into the spec, one question
> LU> arise,
>
> LU> * Does the result of the assigment of f:view "contracts" attribute override
> LU> or add the contract valids for an specific view against
> LU> calculateResourceLibraryContracts()? .
>
> Yes, it does.
>
> LU> In my opinion, f:view "contracts" should override the result of
> LU> calculateResourceLibraryContracts(), just like f:view locale does.
>
> LU> In conclusion. UIViewRoot should provide a property for
> LU> resourceLibraryContracts that can be called from ResourceHandler and JSF 2.2
> LU> section 7.7.2.1 needs to be updated to reflect the fact that the result of
> LU> calculateResourceLibraryContracts() needs to be copied in that attribute of
> LU> UIViewRoot, so f:view tag can later override it.
>
> I'm going to modify the TLDDoc for f:view contracts to clarify that it
> overrides whatever else is there.
>
> LU> STATELESS JSF
>
> LU> - If a stateless view is protected, according to
> LU> ResponseStateManager.writeState() javadoc:
>
> LU> "... Call FacesContext.getViewRoot(). Call StateHolder.isTransient()
> LU> returns true, take implementation specific action so that the following
> LU> call to isStateless(javax.faces.context.FacesContext, java.lang.String)
> LU> returns true and return. Otherwise, proceed as follows. ..."
>
> LU> This creates a conflict with CSRF protection, because one assumption
> LU> already made is that javax.faces.ViewState hidden field is always
> LU> generated and included and in that way it protects POST request. Note
> LU> also that a postback is detected with the presence of
> LU> javax.faces.ViewState in the request (javadoc of
> LU> ResponseStateManager.isPostback() )
>
> LU> "... The implementation of this method for the Standard HTML RenderKit
> LU> must consult the ExternalContext's requestParameterMap and return true
> LU> if and only if there is a key equal to the value of the symbolic
> LU> constant VIEW_STATE_PARAM. ..."
>
> LU> In conclusion:
>
> LU> 1. Stateless views must still write a dummy javax.faces.ViewState field
> LU> to detect postback.
>
> Yes, the stateless view already does that. We use the existing
> ViewState field for stateless as well as non-stateless.
>
> LU> 2. Stateless views that needs to be protected against CSRF attack, must
> LU> include its token in javax.faces.ViewState field as if it was the view
> LU> state, but only contains the info related to the secret stored in session
> LU> or encoded/encrypted when client side state saving is used. In few words
> LU> if the view is protected, javax.faces.ViewState generation may requires
> LU> additional steps.
>
> I'm glad you brought up this interaction. I have filed
> <http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1171>.
>
> LU> FACES FLOW
>
> LU> - javadoc of FlowHandler.clientWindowTransition() still mentions
> LU> Lifecycle.attachWindow(javax.faces.context.FacesContext), but now
> LU> it is called from Restore View Phase directly.
>
> Fixed.
>
> LU> - JSF 2.2 section 7.1.3 FlowHandler Property
>
> LU> It says this about Application.getFlowHandler() and setFlowHandler(...):
>
> LU> "... [P1-start flowHandler called after startup] This method may only be
> LU> called at application startup, before any Faces requests have been processed.
> LU> [P1-end] This is a limitation of the current Specification, and may be lifted
> LU> in a future release ..."
>
> LU> In theory setFlowHandler() should be called at application startup but
> LU> getFlowHandler() should be called after application startup. setFlowHandler()
> LU> should not be called after application startup.
>
> Clarified.
>
> LU> - JSF 2.2 section 11.4.3.2 and 11.4.3.3
>
> LU> If every flow should be declared in a faces-config.xml file, it means there is
> LU> no need to scan the classpath or directories to found every available flow in
> LU> a webapp, because with the list of the flow names it is possible to derive the
> LU> locations where the views related to a flow really are. That's ok.
>
> LU> The problem is, if the views related to the flow are handled by
> LU> ResourceHandler.createViewResource(), FlowHandler doesn't expose that list,
> LU> so in this point the default algorithm should depend on implementation
> LU> details to get the list and know the possible valid paths where it is possible
> LU> to look for view files and other resources.
>
> Fixed.
>
> Committed as r11731.
>
Thanks Ed for take a look into these issues.
regards
Leonardo Uribe
> Ed
>
> --