>>>>> On Thu, 14 Mar 2013 14:14:13 -0400, Andy Schwartz <andy.schwartz_at_oracle.com> said:
AS> I've been discussing the ClientWindow API with two of my colleagues
AS> here: Blake Sullivan and Max Starets. Both were involved in
AS> implementing the Trinidad + ADF Faces window management API.
AS> Interestingly, they both independently made the same recommendation:
AS> that we should follow the typical JSF decode/encode model. That is,
AS> they would like to see methods on ClientWindow to:
AS> 1. Decode parameters from the incoming request. And...
For what it's worth, that's what we had already, but you suggested I
drop decode(), so I did.
AS> 2. Encode parameters into urls (as necessary).
Yes, we have that.
AS> Sticking with the advice that I gave Ed last night,
... and on which I promptly acted.
AS> I would like to recommend that we go with this simple approach
AS> rather than gearing our API to address a performance problem that
AS> might not be a problem.
AS> Leonardo - I know that you are happy with #1 above. Would you be
AS> willing to live with #2 as well?
AS> If not, some more comments on the Map-based below...
>> In that sense, the default implementation of ExternalContext.encodeXXX
>> check if client window API is enabled and if that so it scan this map
>> for query params and add them.
I'm not going to entertain more methods on ExternalContext for this.
After extensive discussion at CONFESS last year, we arrived at the
ClientWindow API. Last night I applied Andy's simplifications because I
felt they wore worthwhile. I need to leave it at that.
AS> I think I figured out one reason why we weren't seeing eye to eye on
AS> whether these two calls:
AS> - ClientWindow.isClientWindowRenderModeEnabled()
AS> - ClientWindow.getQueryURLParameters().isEmpty()
AS> Are redundant.
AS> I missed one important detail, described in the
AS> ExternalContext.encodeActionURL() API doc:
>> * <p class="changed_added_2_2">Call {_at_link
>> javax.faces.lifecycle.ClientWindow#isClientWindowRenderModeEnabled(javax.faces.context.FacesContext)
>> }.
>> * If the result is <code>false</code> take no further action and
>> return
>> * the rewritten URL. If the result is <code>true</code>, call
>> {_at_link #getClientWindow()}.
>> * If the result is non-<code>null</code>, call {_at_link
>> javax.faces.lifecycle.ClientWindow#getId()}
>> * and append the id to the query string of the URL, making the
>> necessary
>> * allowances for a pre-existing query string or no query-string.</p>
>> *
>> * <p>Call {_at_link
>> javax.faces.lifecycle.ClientWindow#getQueryURLParameters}.
>> * If the result is non-{_at_code null}, for each parameter in the map,
>> * unconditionally add that parameter to the URL.</p>
AS> I had been assuming that the ClientWindow's parameter map would include
AS> the parameter corresponding to the ClientWindow id - ie. that if
AS> ClientWindow.getQueryURLParameters() returned the empty map, the url
AS> would not be modified.
AS> Based on the above documentation, this is not the case.
AS> I am not happy with this.
AS> The ClientWindow should have complete control over what state gets
AS> encoded into the url - eg. a ClientWindow implementation should be able
AS> to pick its own query parameter name, and
AS> ExternalContext.encodeActionURL() should not care. This is not the case
AS> with the current design.
Yes, that is not the case. The ClientWindow gets to add things to the
URL, not have complete control over it.
AS> My preference would be to just let the ClientWindow completely handle
AS> encoding (and decoding) its state - ie. add an encode method (and
AS> restore the decode method).
AS> However, if we are not willing to do this, let's at least spec that
AS> getQueryURLParameters() is responsible for returning a Map that includes
AS> all state that is needed by decode(), including the window id
parameter.
You are effectively making that method into an encode(). That's not
what it was intended for.
[...]
AS> Even if we leave Lifecycle.attachWindow(), I would still prefer that
AS> this use a factory for instantiating the ClientWindow object.
I have removed Lifecycle.attachWindow() and moved its responsibilities
into ClientWindowFactory.getClientWindow() and required that it be
called from the FacesContextFactory.getFacesContext().
AS> Fortunately, there is a simple solution, which is to follow the typical
AS> JSF factory pattern. We should introduce a ClientWindowFactory that is
AS> responsible for vending ClientWindow instances. The factory can return
AS> the appropriate type of ClientWindow implementation based on the client
AS> window mode (eg. return a stub instance for "none", an url-based instance
AS> for "url", or a 3rd party instance, eg. in the case of Trinidad.)
>> The same can be done with LifecycleWrapper/LifecycleFactory. The difference
>> is the logic related to enable/disable an specific client window mode can
>> be overriden using LifecycleWrapper. For example, you could have a
>> "client-url-mixed" mode that works different according to the browser
>> capabilities and so on.
AS> Sure. This could also be easily implemented inside of a
AS> ClientWindowFactory.
AS> This would allow us to kill off some unneeded complexity - eg. we could
AS> kill ClientWindow.decode() - the factory would handle this instead.
>>
>> I feel Client Window.decode() straightforward. This part is the central point
>> each ClientWindow implementation should override, so the factory cannot
>> really do it.
>>
AS> The feedback that I got from my colleagues here is that they agree.
AS> Though they would also like to see the corresponding encode() method. :-)
Andy, it would seem your recommendations from
http://java.net/projects/javaserverfaces-spec-public/lists/jsr344-experts/archive/2013-03/message/25
run counter to your colleagues.
AS> While we're at it, we could kill Lifecycle.attachWindow(), and instead
AS> spec that the ClientWindow is set up during FacesContext creation -
AS> eg. the FacesContextFacotry could set up the ClientWindow on the
AS> ExternalContext before creating the FacesContext instance.
>>
>> It is important to have a full FacesContext/ExternalContext instance before
>> attachWindow(). These two steps are different, because in a resource request,
>> you have FacesContext/ExternalContext, but you don't need to call
>> attachWindow(), because the served resource "usually" does not have state.
>>
AS> Ah, I see. So if we init the ClientWindow in the FacesContextFactory,
AS> we'll be doing this for all requests, not just for view-related
AS> requests. Good point.
So you want me to back it out again?
At this point, I'm sorry to say that I'm lost about what Leonardo and
Andy want to do here. ACTION: Andy, please confer with Leonardo *off
list* and come back with your recommendations.
In the meantime, the most current state of the API is
<
http://jsf-spec.java.net/SNAPSHOT/javadoc/>.
Ed
--