On Sun, 2007-10-28 at 18:19 +0100, Mario Ivankovits wrote: > Hi Simon, > > > I cannot get this patch to apply cleanly unfortunately. And it's > > difficult to review the patch file directly as it has a lot of > > whtespace-only changes. > Sorry for that, I'll see if I can clean it up before committing, not > sure about it. > > > There are now two phases to rendering for JSP: buildView then encodeAll. > > As long as no component changes the tree during the encodeAll pass then > > it seems to me that it is possible to just compute the serialized view > > state at the start of the encodeAll phase [1] and write it out directly > > when it is needed. > Do you talk about client-side-state-saving?
I'm talking about both cases. Just to clarify, I think that your proposed change is fine, but it *might* be possible to clean up the existing code much further..making your change redundant. > This patch is just about server-side-state-saving and with this, during > the encodeAll pase just an "ID" gets rendered, no state should have been > built until then. > The state is build after encodeAll (at least with Facelets, will work > with JSP too but I think there is a bug I realized when there was no > time to fix it). > This means, even a modified tree structure will be correctly saved. Yes, that's how it currently works. Right now, the saved state is not computed until after encodeAll has completed. But why? I'm wondering if this is just a leftover from the old JSF1.1 days where the state *had* to be computed at the end of rendering because building the component tree was a "side effect" of rendering. And if the state can be computed before encodeAll is invoked, then the "server side" and "client side" logic can effectively be unified. The JspStateManager.saveState method can serialize the tree, then: * for server-side, allocate a seq#, store the state in the session using that key and return the seq# as a string. * for client-side, return the state as a byte-array (which the renderkit will base64-encode). The writeState method then simply has to do what its name implies - write the value returned by saveState to the response. For server-side that will happen to be an integer, and for client-side that will happen to be a bytearray. There will be no if-statements needed in the JspViewHandler though.. > > > There might be a couple of flaws in my reasoning though: can backing bean > > code invoked during render alter the tree state (eg insert data into the > > attributes map of a component). > > > Yep, is still possible, not sure how many people will use stuff like > this. But we do - includeFacelets - you know :-) > > > Is it necessary to support this kind of thing by backing beans in > > *rendering* phase? (obviously, it is quite reasonable in invokeApplication > > etc). I think not. In this case, state writing would become a whole lot > > simpler. > > > It is possible so I think it needs to be supported, though, i slowly > move to the side which states that this should not be the common style. In what situation *does* this need to be supported? Shouldn't a facelets dynamic-include tag instead modify the tree during the buildView, rather than the encodeAll? That then solves the problem; the tree then is not modified during encodeAll so state can be computed at the start. The orchestra dynaForm should do the same, I think - modify the tree during buildView. It means that some logic needs to be driven by the taghandler rather than the component, but that seems reasonable to me. Taghandlers are responsible for building component trees, components are not. I'm struggling at the moment with getting myfaces1.2 to build, but if I can solve that then I'll experiment with dynamic include and dynaForm in this manner. Cheers, Simon
