Gang -
A likely pointless review of ClientWindow, given how late this is
arriving. Nonetheless, here goes:
Index: javax/faces/lifecycle/ClientWindow.java
===================================================================
--- javax/faces/lifecycle/ClientWindow.java (revision 0)
+++ javax/faces/lifecycle/ClientWindow.java (revision 11719)
+ * <p>To accomadate the widest possible range of implementation choices
+ * to support this feature, explicit names for modes other than "none"
+ * are not specified.
Below we state that an "url" mode is supported. Which is correct?
+ * <p>In addition to the hidden field already described. The runtime
+ * must ensure that any component that renders a hyperlink that causes
+ * the user agent to send a GET request to the Faces server when it is
+ * clicked has a query parameter with a name and value specified in
+ * {_at_link ResponseStateManager#CLIENT_WINDOW_URL_PARAM}. This
+ * requirement is met by several of the "encode" methods on {_at_link
+ * javax.faces.context.ExternalContext}. See {_at_link
+ * javax.faces.context.ExternalContext#encodeActionURL(java.lang.String)
+ * } for details.</p>
What is the expected behavior when the end user creates a second browser
tab/window with an existing window id? For example, this can happen by:
- Bookmarking an url with a window id and opening that URL in multiple tabs.
- Copying and pasting an url from one tab into a second tab.
- (On some browsers) Using the browser's "New Window" action.
For each of the above cases, each window/tab should be assigned its own
unique id, even though it would require additional work to determine
when a client window id has been copied across windows/tabs. Will JSF's
client window implementation handle this? If not, we run the risk of
severely hosing our end users.
+ /**
+ * <p class="changed_added_2_2">The context-param that controls the
operation
+ * of the <code>ClientWindow</code> feature. The runtime must support
+ * the values "none" and "url", without the quotes, but other values
+ * are possible. If not specified, "none" is assumed.</p>
+ *
+ * @since 2.2
+ */
+ public static final String CLIENT_WINDOW_MODE_PARAM_NAME =
What should the JSF implementation do with a value that it does not
recognize?
+ /**
+ * <p class="changed_added_2_2">This method will be called whenever
a URL
+ * is generated by the runtime where client window related
parameters need
+ * to be inserted into the URL. This guarantees custom {_at_code
ClientWindow} implementations
+ * that they will have the opportunity to insert any additional
client window specific
+ * information in any case where a URL is generated, such as the
rendering
+ * of hyperlinks. The default implementation of this method returns
+ * the {_at_code null}.</p>
+ *
+ *
+ * @since 2.2
+ * @param context the {_at_code FacesContext} for this request.
+ * @return {_at_code null} or a map of parameters to insert into the
URL query string.
+ */
+
+ public abstract Map<String, String>
getQueryURLParameters(FacesContext context);
Is the returned Map mutable? My thinking is that:
- The returned Map should be immutable. And...
- The method should be spec'ed to return a non-null value.
An alternate approach would be to avoid returning a Map altogether and
instead ask the ClientWindow to tack its parameters onto a provided base
URL, similar to our other encode*URL APIs. This would simplify life for
client code (eg. no need to create an Iterator, iterate over parameter
names, check for already existing parameters, etc...)
+ /**
+ * <p class="changed_added_2_2">The implementation is responsible
+ * for examining the incoming request and extracting the value that
must
+ * be returned from the {_at_link #getId} method. If {_at_link
#CLIENT_WINDOW_MODE_PARAM_NAME}
+ * is "none" this method must not be invoked. If {_at_link
#CLIENT_WINDOW_MODE_PARAM_NAME}
+ * is "url" the implementation must first look for a request parameter
+ * under the name given by the value of {_at_link
javax.faces.render.ResponseStateManager#CLIENT_WINDOW_PARAM}.
+ * If no value is found, look for a request parameter under the
name given
+ * by the value of {_at_link
javax.faces.render.ResponseStateManager#CLIENT_WINDOW_URL_PARAM}.
+ * If no value is found, fabricate an id that uniquely identifies this
+ * <code>ClientWindow</code> within the scope of the current
session. This
+ * value must be encrypted with a key stored in the http session
and made
+ * available to return from the {_at_link #getId} method. The value
must be
+ * suitable for inclusion as a hidden field or query parameter.
+ * If a value is found, decrypt it using the key from the session and
+ * make it available for return from {_at_link #getId}.</p>
+ *
+ * @param context the {_at_link FacesContext} for this request.
+ *
+ * @since 2.2
+ */
+
+ public abstract void decode(FacesContext context);
Currently ClientWindow creation/initialization is buried in
LifecycleImpl.attachWindow(). This makes it difficult to provide custom
ClientWindow implementations, as it requires that you replace or wrap
the Lifecycle. It also assumes that you are only creating ClientWindows
from within the JSF lifecycle. Based on our experience with Trinidad's
window manager, we are likely going to want to create the ClientWindow
earlier (eg. from a filter).
Fortunately, there is a simple solution, which is to follow the typical
JSF factory pattern. We should introduce a ClientWindowFactory that is
responsible for vending ClientWindow instances. The factory can return
the appropriate type of ClientWindow implementation based on the client
window mode (eg. return a stub instance for "none", an url-based
instance for "url", or a 3rd party instance, eg. in the case of Trinidad.)
This would allow us to kill off some unneeded complexity - eg. we could
kill ClientWindow.decode() - the factory would handle this instead.
While we're at it, we could kill Lifecycle.attachWindow(), and instead
spec that the ClientWindow is set up during FacesContext creation - eg.
the FacesContextFacotry could set up the ClientWindow on the
ExternalContext before creating the FacesContext instance.
+ /**
+ * <p class="changed_added_2_2">Components that permit per-use
disabling
+ * of the appending of the ClientWindow in generated URLs must call
this method
+ * first before rendering those URLs. The caller must call {_at_link
#enableClientWindowRenderMode(javax.faces.context.FacesContext)}
+ * from a <code>finally</code> block after rendering the URL. If
+ * {_at_link #CLIENT_WINDOW_MODE_PARAM_NAME} is "url" without the
quotes, all generated
+ * URLs that cause a GET request must append the ClientWindow by
default.
+ * This is specified as a static method because callsites need to
access it
+ * without having access to an actual {_at_code ClientWindow}
instance.</p>
+ *
+ * @param context the {_at_link FacesContext} for this request.
+ *
+ * @since 2.2
+ */
+
+ public static void disableClientWindowRenderMode(FacesContext
context) {
I don't understand why these are static. This explanation:
+ * This is specified as a static method because callsites need to
access it
+ * without having access to an actual {_at_code ClientWindow}
instance.</p>
Is confusing.
The ClientWindow is currently initialized in FacesServlet.service():
> if (handler.isResourceRequest(context)) {
> handler.handleResourceRequest(context);
> } else {
> lifecycle.attachWindow(context);
> lifecycle.execute(context);
> lifecycle.render(context);
> }
As such, if you've got access to a FacesContext (as is the case when
disableClientWindowRenderMode is called), you've got access to a
ClientWindow.
If for some reason the ClientWindow is not available (eg. if
facesContext.getExternalContext().getClientWindow() reutrns null), well,
then... there is nothing to disable anyway, right?
At the moment the only place that Mojarra calls
ClientWindow.disableClientWindowRenderMode() is in
OutcomeTargetRenderer.getEncodedTargetURL(), which is called during
encoding. We clearly have access to a ClientWindow here.
What's the deal with making these methods static? From what I can tell
they should be instance methods.
Also, should we spec that when disabled,
ClientWindow.getQueryURLParameters() returns the empty collection? If
we did that, do we even need an isClientWindowRenderModeEnabled() API?
Andy