|
Created an attachment (id=337) Hi Ed. Finally I found some time to develop a patch for this. It works fine with CS JSF. regards taking ownership to develop patch for review and commit. Hello. This is a reworked patch to restore ViewScope before the templates are processed in buildView(). Until now the full state including ViewScope is restored in UIViewRoot.restoreState(). But this method must only be called after the templates have been processed. To allow restore of ViewScope before processing the templates, I introduced a new method called UIViewRoot.restoreViewScopeState(FacesContext fc, Object state) to restore ViewScope only. In UIViewRoot.restoreState(..) viewScope will only be restored if not done before. I am aware that it seems weird that ViewScope is state saved in the normal UIViewRoot.saveState(..) method and restored with restoreViewScopeState(..). But for saving the state there was no need to change the behaviour unless we want to redesign this in more depth which would involve a pair of method to save/restore ViewScope. We use this solution in CS JSF since about 1/2 year and it worked with partial- and full state saving. regards This is a nice and simple patch. Naturally, I wonder if there is a root problem (no pun intended) not specific to just the UIViewRoot, where a component needs to have some state restored to it after the tree has been built from the template, but before the partial state restoration traversal. If the UIViewRoot has this problem, then perhaps other components may have it as well? Do we need to generalize this or is it sufficient to handle the view scope alone? Arguments in favor of accepting the special case approach directly from 1. The manner in which the pre-restoration-traversal state is obtained 2. An other use-case for pre-restoration-traversal state is dynamic 3. The UIViewRoot is unique among all other components in that you don't Arguments in favor of denying Hanspeter's 20110607 patch in favor of a 1. The additional API required would not be that large. Here is a sketch. Hanspeter, can you please cook up a version of the patch that uses the Hi Ed. Interesting considerations and I really like more generalized solutions, but I think UIViewRoot is unique for this matter: 1. UIViewRoot is not created from templates and there is also no possibility to do special work in a UIViewRootTagHandler. 2. The state in question is the ViewScope - all other components don't have theyr own scope to restore. 3. PreRestoreStateEvent on the component would be about the same time as PostAddToViewEvent on the component. It's after component was constructed and added to the view but before restore state is processed - restore state is processed after the full component tree was built. 4. In my opinion there is no way to restore some component state before the component is created - and during component creation TagHandler may be used for such action or directly after component construction PostAddToView component event may be used. 5. PreRestoreStateEvent processing would probably cause another tree walk, which some of the EG members will not like. Ed, I don't see the need for that elaboration. Regards final changebundle before commit Committed to trunk. Sending jsf-api/src/main/java/javax/faces/component/UIViewRoot.java (had to reproduce this manually - next time I know) Sending jsf-api/src/main/java/javax/faces/component/UIViewRoot.java Hi Matt, we used that on our custom built JSF RI 2.0 before 2.0.4 without any problem. So you should be save doing the same. regards Actually, this change does appear to have a problem on 2.0.4b9. After applying the patch, we do have the viewscope at the correct time, but for some reason we are seeing two PostAddToViewEvents being fired during RENDER_RESPONSE but only during the very first time the page is requested. The second PostAddToViewEvent has the component in a bad state, with no children. If I refresh the page, everything works as normal. The only way to cause the problem again is to bounce the server, and again on the first page refresh the additional PostAddToViewEvent is fired. Matt Any thoughts on this? I've tried looking through the source code, but I'm currently really too ignorant of the details of the system to figure out why this might happen... I think it must be related to how the system deals with the templates for the first time, like when it compiles/caches(correct?) them or something its throwing that additional PostAddToViewEvent. I've confirmed that if I remove the code, the additional event goes away as expected. To work around this, I'm serializing my object graph into a hidden input field and restoring on the decode() of my custom composite component (manually grabbing the value from request). This seems to work, and allows us to then dynamically create our component tree during PostAddToView based on the data serialized in that field from request to request (which can change). Thanks for any input on this! Regards, Matt Hi Matt, The changes from this issue only affect restoreState in UIViewRoot and restoreView in StateManagementStrategyImpl - so I can hardly believe it affects the first time a page is rendered. I assume the double delivered PostAddToViewEvent is due to something else. Debug into your PostAddToViewEvent handling will show where the event comes from. Make sure the second one is not from a resource request - which would explain why the component delivered by the event does not have any children or parent. regards I did check this, and there are no resources being requested.. It is simply a custom composite component in an otherwise empty page. Now, I am registering for the PostAddToViewEvent inside my components constructor, not in any page backing bean. It is strange, that when I revert the changes and re-compile the extra event goes away with the exact same page. As noted above I'm still very new to this technology having come across from ASP.NET, but it does have some similarities and I am working hard to get a better understanding. Thanks, Matt This patch breaks a fundamental dynamic components test. Here it is (and the exception): Test Case: Simple View <h:form prependId="false" id="dynamicForm"> <h:panelGroup id="group"> </h:form> The action in the managed bean dynamically created an HTMLOutputText component and adds it as a child to the panelGroup ("group"): public void addComponent() { FacesContext ctx = FacesContext.getCurrentInstance(); UIComponent group = ctx.getViewRoot().findComponent("dynamicForm" + UINamingContainer.getSeparatorChar(ctx) + "group"); HtmlOutputText output = new HtmlOutputText(); output.setValue("OUTPUT"); group.getChildren().add(output); } The first button click adds it fine (after the button). The next button click produces the following exception: java.lang.ClassCastException: com.sun.faces.application.view.StateHolderSaver cannot be cast to [Ljava.lang.Object; The problem is that dynamic components are saved as StateHolderSaver objects. Hi Roger, thanks for that update - but I think the workaround is really just a workaround. I wonder why the dynamically added component changes the state structure. In UIViewRoot.saveState() - the saved state is an Object array with size 2 - element 0 is assigned with super.saveState() and element 1 is storing the viewScope as attachedState. But in case of the dynamically added component that seems no longer true - so that would mean the dynamically added component corrupts the state structure on UIViewRoot level. I'll try to reproduce that problem and analyse it - hope I find time for it. regards I spoke incorrectly when I said dynamic components were saved as StateHolderSaver objects - obviously that is not true. StateHolderSaver is a helper class in Mojarra. During saveView, dynamic components are added to the state map as StateHolderSaver objects. You can see this in StateManagementStrategyImpl.saveView. Hi Ed and Roger. I tried to reproduce the error with the example Roger has given - but was not able to reproduce the exact same problem. But I have realized that h:form prependId="false" may lead to duplicate id's - see http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1032 Looking at what tree visitor does in StateManagementStrategyImpl.saveView I somewhat understood the problem - in case a certain component was added dynamically, it's state is packed into a StateHolderSaver. But to cause the problem that Roger has pointed out it would be necessary to dynamically add UIViewRoot - is that possible at all or are we hunting a phantom problem? I think the problem Roger ran into was based on the problem that UIForm with prependId="false" generated an id for the dynamically added component that was the same as the UIViewRoot's id. Then of course the saveView() treeVisitor assumes UIViewRoot was added dynamically and wraps it's state within a StateHolderSaver - and voila - we have the problem. However, I was not able to reproduce that. Big question: is it possible to have UIViewRoot as a dynamically added component? I was not able to reproduce the problem - if Roger can reproduce it, please provide me exact information of how to reproduce. Unfortunatley on my windows machine I cannot run test.with.container.refresh anymore ... Testcase I tried to reproduce the problem - no success so far. Finally with some help from Roger I was abler to reproduce the problem - but only after removing the patch for http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1032 The dynamically added component is actually added to a panelGroup - and get's the clientId "j_id1" - which is the same as teh UIViewRoot's clientId. Now StateManagementStrategyImpl gets confused and handles UIViewRoot as a dynamically added component - and wraps it's state into a StateSaveHolder. And then next restoreState fails. resolved as of Sending jsf-api/src/main/java/javax/faces/component/UIViewParameter.java http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1063 SECTION: Modified Files
M jsf-ri/src/main/java/com/sun/faces/config/processor/FaceletTaglibConfigProcessor.java
M jsf-api/src/main/java/javax/faces/component/UIViewParameter.java
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
I talked to Martin and Hanspeter about this. I agree with restoring the ViewMap first, but I don't know
if it will be possible in practice. Marking as P2 to make sure we get to it.