search

javaee / javaserverfaces-spec

JavaServer(TM) Faces Specification web site
https://javaee.github.io/javaserverfaces-spec/
Other
43 stars 19 forks source link

Modify the requirements of the jsf.ajax.response JavaScript documentation to enable portlet compatibility #1421

Closed glassfishrobot closed 7 years ago

glassfishrobot commented 8 years ago

The jsf.ajax.response JavaScript documentation contains requirements that are not compatible with portlet environments.

Portlet Incompatibility #1

If an element is found in the response with the identifier javax.faces.ViewRoot:

<update id="javax.faces.ViewRoot">
   <![CDATA[...]]>
</update>

Update the entire DOM replacing the appropriate head and/or body sections with the content from the response

Portlet Incompatibility #2

If an element is found in the response with the identifier javax.faces.ViewBody:

<update id="javax.faces.ViewBody">
   <![CDATA[...]]>
</update>

update the document's body section with the CDATA contents from the response.

These requirements assume that the RENDER_RESPONSE phase of the JSF lifecycle was responsible for generating the entire HTML document including the following tags:

<html>
    <head>
        ...
    </head>
    <body>
        ...
    </body>
</html>

The requirements further assume that the renderer associated with the h:body component tag renders the starting tag and end tag, and that the (UIViewRoot) corresponds to the body section of the document.

In a portlet environment, the JSF Portlet Bridge ensures that object returned by FacesContext.getCurrentInstance().getViewRoot() to be an instance of javax.portlet.faces.component.PortletNamingContainerUIViewRoot. Since it extends javax.faces.component.NamingContainer, the UIViewRoot becomes "namespaced" with the portlet namespace.

The JSF Portlet Bridge also overrides the renderer associated with the h:body component and renders a

...
section instead of a ... section. In addition, the div has an id attribute with the namespaced clientId of the view root.

The JSF Portlet Bridge also overrides the renderer associated with the h:head component so that it can inform the portlet container of required JSF resources. The renderer does not render a ... section in a portlet environment.

For example, given the following JSF view:

<f:view>
    <h:head />
    <h:body>
        <h:form id="f1">
            <h:inputText id="foo" value="#{backingBean.foo}" />
            <h:commandButton />
        </h:form>
    </h:body>
</f:view>

On Apache Pluto the portlet div section might look something like this:

<div id="Pluto_myPortlet_1__24955534_0_">
    ...
    <form id="Pluto_myPortlet_1__24955534_0_:f1">
        <input type="hidden" name="Pluto_myPortlet_1__24955534_0_:f1" value="Pluto_myPortlet_1__24955534_0_:f1">
        <input id="Pluto_myPortlet_1__24955534_0_:f1:foo" type="text" name="Pluto_myPortlet_1__24955534_0_:f1:foo">
        <input type="submit">
        <input type="hidden" name="Pluto_myPortlet_1__24955534_0_javax.faces.ViewState" id="Pluto_myPortlet_1__24955534_0_:javax.faces.ViewState:0" value="-7034168008597714894:-3199096175767081201" autocomplete="off">
    </form>
</div>

The requirements need to be updated so that they enable portlet compatibility by replacing the

...
section associated with the UIViewRoot rather than the ... section of the document when running in a portlet environment.

Environment

Portlets

Affected Versions

[2.2]

glassfishrobot commented 8 years ago

Reported by ngriffin7a

glassfishrobot commented 8 years ago

@BalusC said: This seems to be already implemented since Mojarra 2.2.0. It only uses getClientId() instead of getContainerClientId(), but it shouldn't make any difference on the root component.

In other words, when tt>render="@all"</tt is used, it will already not anymore write out . It's only left unspecified in the spec. All in all, when also considering issue 790, I think it makes sense to propose the following change to be as transparent as possible (i.e. do not mention Portlet specifics and do not change JavaScript):

Update PartialViewContext#processPartial() to specify that during render response phase, when the for javax.faces.ViewRoot will be written, and the UIViewRoot is an instance of UINamingContainer, then write id attribute with value of UIViewRoot#getContainerClientId(), else write value of PartialResponseWriter#RENDER_ALL_MARKER.

This way, we can also remove the Util.isPortletRequest() check from the current implementation.

The javax.faces.ViewBody is nowhere considered in JSF API nor in Mojarra impl, it's only mentioned in JS API. It's internally treated exactly the same way as javax.faces.ViewRoot. I guess it's a leftover, so we can ignore it for now. Perhaps it should better be removed from JS API, but that's a different issue.

glassfishrobot commented 8 years ago

ngriffin7a said: @balusc: Please forgive, but I simply didn't remember that I had created #1069 back in 2012. This resulted in the following paragraph being modified in section 2.2.6.1 of the Spec titled "Render Response Partial Processing":

If isRenderAll() returns true and this is not a portlet request, call startUpdate(PartialResponseWriter.RENDER_ALL_MARKER) on the PartialResponseWriter. For each child of the UIViewRoot, call encodeAll(). Call endUpdate() on the PartialResponseWriter. Render the state using the algorithm described below in Section “Partial State Rendering”, call endDocument() on the PartialResponseWriter and return. If isRenderAll() returns true and this is a portlet request, treat this as a case where isRenderAll() returned false, but use the UIViewRoot itself as the one and only component from which the tree visit must start.

Thanks for pointing out the code in com.sun.faces.context.PartialViewContextImpl – This functionality was introduced into Mojarra 2.2 by Manfred via commit e25abcae and 0e42795e.

I did some research and the reason why ResponseWriterBridgeImpl.java still contains a workaround is because the MyFaces PartialViewContextImpl. processRenderAll(UIViewRoot,PartialResponseWriter) method does not yet implement the requirements outlined in section 2.2.6.1 of the Spec. I just created MYFACES-4053 and will try to contribute a fix soon.

I really like your idea of using language like "if UIViewRoot is an instance of UINamingContainer" rather than "if this is a portlet request" and would recommend that section 2.2.6.1 of the Spec be updated.

Finally, I agree with your assessment regarding javax.faces.ViewBody. After studying the JavaScript documentation more today, I also noticed javax.faces.ViewHead. I think that the Bridge Spec can simply state that bridge implementations should ignore these features since they are inherently incompatible with portlet environments.

glassfishrobot commented 7 years ago

stiemannkj1 said: Neil Griffin, balusc, It seems that the consensus is that JSDoc should not be changed and the Bridge spec should state that the javax.faces.ViewRoot, javax.faces.ViewBody, and javax.faces.ViewHead features are incompatible with portlets. It seems that JSF 2.2 already supports the portlet case thanks to #1069: if PartialViewContext.isRenderAll() is true in a portlet evironment, then 's id will be the UIViewRoot's id, and the the UIViewRoot will be rendered. It seems like the only changes needed are to the spec and implementation.

Here's the proposed changes to spec section 2.2.6.1 (removed text appears as strikethrough, added text is bold):

If isRenderAll() returns true and this is not a portlet request the view root is not an instance of NamingContainer, call startUpdate(PartialResponseWriter.RENDER_ALL_MARKER) on the PartialResponseWriter. For each child of the UIViewRoot, call encodeAll(). Call endUpdate() on the PartialResponseWriter. Render the state using the algorithm described below in Section “Partial State Rendering”, call endDocument() on the PartialResponseWriter and return. If isRenderAll() returns true and this is a portlet request the view root is an instance of NamingContainer, treat this as a case where isRenderAll() returned false, but use the UIViewRoot itself as the one and only component from which the tree visit must start.

Here's the proposed implementation changes: https://github.com/stiemannkj1/jsf-spec-mojarra/commit/611f5f07104234a75390a75633067f887ec27a0a. I haven't been able to test the changes yet though.

@Neil Griffin, should this issue be closed, and a new issue or email be opened about changing the spec?

glassfishrobot commented 7 years ago

ngriffin7a said: stiemannkj1: The changes you mentioned for Spec section 2.2.6.1 and the Pull Request look good to me. Thanks.

glassfishrobot commented 7 years ago

stiemannkj1 said: Neil Griffin, vsingleton, balusc, Ed Burns,

Since the spec has been changed according to my comments, I've created a PR to vsingleton to change the implementation to be consistent with the spec: https://github.com/vsingleton/mojarra_read-only_mirror/pull/15

I've tested these changes against the following modules (and they passed):

glassfishrobot commented 7 years ago

vsingleton said: merged branch 'modify-PartialViewContext-renderAll-requirements-to-remove-references-to-portlets-JSF-SPEC-1421' of https://github.com/stiemannkj1/mojarra

https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1421 modify PartialViewContext renderAll requirements to remove references to portlets

modified: jsf-ri/src/main/java/com/sun/faces/context/PartialViewContextImpl.java

glassfishrobot commented 8 years ago

Issue-Links: is related to JAVASERVERFACES_SPEC_PUBLIC-790

glassfishrobot commented 8 years ago

Was assigned to ngriffin7a

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA JAVASERVERFACES_SPEC_PUBLIC-1421

glassfishrobot commented 7 years ago

Marked as fixed on Thursday, January 12th 2017, 4:10:58 pm