dev@javaserverfaces.java.net

Re: REVIEW - Issue 26 - Tightly Coupled Renderer Dependencies

From: Craig R. McClanahan <Craig.McClanahan_at_Sun.COM>
Date: Mon, 02 Aug 2004 09:47:12 -0700

Roger Kitain wrote:

> I would especially like to get Craig's input on this change bundle,
> since he
> was bitten by this dependency when working on struts-faces integration.
>
Thanks for working on the Roger. Unfortunately, it does not address the
real problem. The RI's version of the renderer for a Command Link is
presuming on *behavior* in the RI's version of the Form renderer -- i.e.
writing out the hidden fields -- and the patch only addresses how
CommandLinkRenderer *stores* its request for rendering to be done later.

The required behavior is not specified clearly enough for alternative
implementations to portably duplicate (the renderkit docco says "Render
all the necessary hidden fields for all commandLinki instances in the
page just before the close of the "form" element). Therefore, even with
the proposed patch, the RI's command link component will *still* only
work inside the RI's form component. That is not sufficient to address
the problem reported, which is based on the requirement in Struts-Faces
to use a custom form renderer in order to trigger some additional
Struts-specific behavior).

A couple of additional comments interspersed below, in case the ultimate
solution is still along these lines.

Craig McClanahan

> Thanks, Roger.
>
> Summary:
> https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=26
> The original tightly coupled renderer dependency was introduced as a
> solution
> for bugster bug: 5055795 (command link/multiple forms/back button
> problem).
>
> M src/com/sun/faces/RIConstants.java
> - introduce new constant to identify stored component attribute data
> M src/com/sun/faces/renderkit/html_basic/CommandLinkRenderer.java
> - stash a hidden field (identifying this command link) in a map;
> - stash the map as attribute data in this command link's form
> component;
> - store param data associated with this link using the param's "value"
> ** not "name* - because "name is not required - and if no name is
> specified,
> HtmlResponseWriter will throw NPE (per spec)..
> M src/com/sun/faces/renderkit/html_basic/FormRenderer.java
> - render any hidden field information from stored map in Form
> component;
> - make addNeededHiddenField/addRenderedHiddenField/getHiddenFieldMap
> private non static helper methods (not sure if these are still
> needed)..
> M src/com/sun/faces/renderkit/html_basic/HiddenRenderer.java
> - remove FormRenderer.addRenderedHiddenField call
> M systest/web/golden/taglib/commandLink_test.txt
> - new golden file
> M web/test/RenderResponse_correct
> - new golden file
>
> Index: RIConstants.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/RIConstants.java,v
> retrieving revision 1.67
> diff -u -r1.67 RIConstants.java
> --- RIConstants.java 26 Jul 2004 21:12:43 -0000 1.67
> +++ RIConstants.java 2 Aug 2004 12:36:35 -0000
> @@ -98,6 +98,11 @@
>
> public static boolean IS_UNIT_TEST_MODE = false;
>
> + /**
> + * <p>Can be used as the name of stored component attribute data</p>
> + */
> + public static final String COMPONENT_DATA = "component-data";
> +

Because we are going to use this string as the name of an attribute in
someone's form component, the value should be something less likely to
clash with existing attributes that the form component is already
using. For example, qualify it with "com.sun.faces" because it's
specific to the RI's implementation of CommandLinkRenderer.

> /*
> * <p>TLV Resource Bundle Location </p>
> */
>
>
> Index: FormRenderer.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/renderkit/html_basic/FormRenderer.java,v
>
> retrieving revision 1.78
> diff -u -r1.78 FormRenderer.java
> --- FormRenderer.java 28 Apr 2004 20:59:41 -0000 1.78
> +++ FormRenderer.java 2 Aug 2004 12:28:28 -0000
> @@ -212,22 +212,17 @@
> throws IOException {
>
> ResponseWriter writer = context.getResponseWriter();
> - Map map = getHiddenFieldMap(context, false);
> +
> + Map map =
> (Map)component.getAttributes().get(RIConstants.COMPONENT_DATA);
> if (map != null) {
> Iterator entries = map.entrySet().iterator();
> while (entries.hasNext()) {
> Map.Entry entry = (Map.Entry) entries.next();
> - if (Boolean.TRUE.equals(entry.getValue())) {
> - writer.startElement("input", component);
> - writer.writeAttribute("type", "hidden", null);
> - writer.writeAttribute("name", entry.getKey(), null);
> - writer.endElement("input");
> - }
> + writer.startElement("input", component);
> + writer.writeAttribute("type", "hidden", null);
> + writer.writeAttribute("name", entry.getKey(), null);
> + writer.endElement("input");
> }
> -
> - // Clear the hidden field map
> - Map requestMap =
> context.getExternalContext().getRequestMap();
> - requestMap.put(HIDDEN_FIELD_KEY, null);
> }
> }
>
> @@ -235,7 +230,7 @@
> /**
> * <p>Remember that we will need a new hidden field.</p>
> */
> - public static void addNeededHiddenField(FacesContext context,
> + private void addNeededHiddenField(FacesContext context,
> String clientId) {
> Map map = getHiddenFieldMap(context, true);
> if (!map.containsKey(clientId)) {
> @@ -243,18 +238,17 @@
> }
> }
>
> -
> /**
> * <p>Note that a hidden field has already been rendered.</p>
> */
> - public static void addRenderedHiddenField(FacesContext context,
> + private void addRenderedHiddenField(FacesContext context,
> String clientId) {
> Map map = getHiddenFieldMap(context, true);
> map.put(clientId, Boolean.FALSE);
> }
>
>
> - private static Map getHiddenFieldMap(FacesContext context,
> + private Map getHiddenFieldMap(FacesContext context,
> boolean createIfNew) {
> Map requestMap = context.getExternalContext().getRequestMap();
> Map map = (Map) requestMap.get(HIDDEN_FIELD_KEY);
> rogerk_at_bruins:~/jsf/javaserverfaces-sources/jsf-ri/src/com/sun/faces/renderkit/html_basic>
> cvs diff -u *.java
> Index: CommandLinkRenderer.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/renderkit/html_basic/CommandLinkRenderer.java,v
>
> retrieving revision 1.22
> diff -u -r1.22 CommandLinkRenderer.java
> --- CommandLinkRenderer.java 8 Jun 2004 13:47:28 -0000 1.22
> +++ CommandLinkRenderer.java 2 Aug 2004 12:35:25 -0000
> @@ -26,6 +26,7 @@
> import javax.faces.event.ActionEvent;
>
> import java.io.IOException;
> +import java.util.HashMap;
> import java.util.Iterator;
> import java.util.Map;
>
> @@ -213,7 +214,7 @@
> sb.append(formClientId);
> sb.append("'");
> sb.append("]['");
> - sb.append(paramList[i].getName());
> + sb.append(paramList[i].getValue());


Is this change actually intended? It seems like this would generate an
incorrect reference in the rendered JavaScript code.

> sb.append("'].value='");
> sb.append(paramList[i].getValue());
> sb.append("';");
> @@ -313,10 +314,10 @@
> writer.endElement("a");
>
> //Handle hidden fields
> -
> //Only need one hidden field for the link itself per form.
> - FormRenderer.addNeededHiddenField(context,
> - getHiddenFieldName(context, command));
> + String hiddenFieldName = getHiddenFieldName(context, command);
> + Map map = new HashMap();
> + map.put(hiddenFieldName, hiddenFieldName);
>
> // PENDING(edburns): not sure if the JSFA59 back button problem
> // manifests itself with param children as well...
> @@ -324,8 +325,10 @@
> // get UIParameter children...
> Param paramList[] = getParamList(context, command);
> for (int i = 0; i < paramList.length; i++) {
> - FormRenderer.addNeededHiddenField(context,
> paramList[i].getName());
> + map.put(paramList[i].getValue(), paramList[i].getValue());
> }
> + UIForm form = getMyForm(context, command);
> + form.getAttributes().put(RIConstants.COMPONENT_DATA, map);
> if (log.isTraceEnabled()) {
> log.trace("End encoding component " + component.getId());
> }
> Index: FormRenderer.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/renderkit/html_basic/FormRenderer.java,v
>
> retrieving revision 1.78
> diff -u -r1.78 FormRenderer.java
> --- FormRenderer.java 28 Apr 2004 20:59:41 -0000 1.78
> +++ FormRenderer.java 2 Aug 2004 12:35:25 -0000
> @@ -212,22 +212,17 @@
> throws IOException {
>
> ResponseWriter writer = context.getResponseWriter();
> - Map map = getHiddenFieldMap(context, false);
> +
> + Map map =
> (Map)component.getAttributes().get(RIConstants.COMPONENT_DATA);
> if (map != null) {
> Iterator entries = map.entrySet().iterator();
> while (entries.hasNext()) {
> Map.Entry entry = (Map.Entry) entries.next();
> - if (Boolean.TRUE.equals(entry.getValue())) {
> - writer.startElement("input", component);
> - writer.writeAttribute("type", "hidden", null);
> - writer.writeAttribute("name", entry.getKey(), null);
> - writer.endElement("input");
> - }
> + writer.startElement("input", component);
> + writer.writeAttribute("type", "hidden", null);
> + writer.writeAttribute("name", entry.getKey(), null);
> + writer.endElement("input");
> }
> -
> - // Clear the hidden field map
> - Map requestMap =
> context.getExternalContext().getRequestMap();
> - requestMap.put(HIDDEN_FIELD_KEY, null);
> }
> }
>
> @@ -235,7 +230,7 @@
> /**
> * <p>Remember that we will need a new hidden field.</p>
> */
> - public static void addNeededHiddenField(FacesContext context,
> + private void addNeededHiddenField(FacesContext context,
> String clientId) {
> Map map = getHiddenFieldMap(context, true);
> if (!map.containsKey(clientId)) {
> @@ -243,18 +238,17 @@
> }
> }
>
> -
> /**
> * <p>Note that a hidden field has already been rendered.</p>
> */
> - public static void addRenderedHiddenField(FacesContext context,
> + private void addRenderedHiddenField(FacesContext context,
> String clientId) {
> Map map = getHiddenFieldMap(context, true);
> map.put(clientId, Boolean.FALSE);
> }
>
>
> - private static Map getHiddenFieldMap(FacesContext context,
> + private Map getHiddenFieldMap(FacesContext context,
> boolean createIfNew) {
> Map requestMap = context.getExternalContext().getRequestMap();
> Map map = (Map) requestMap.get(HIDDEN_FIELD_KEY);
> Index: HiddenRenderer.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/renderkit/html_basic/HiddenRenderer.java,v
>
> retrieving revision 1.20
> diff -u -r1.20 HiddenRenderer.java
> --- HiddenRenderer.java 31 Mar 2004 18:48:35 -0000 1.20
> +++ HiddenRenderer.java 2 Aug 2004 12:35:25 -0000
> @@ -97,8 +97,6 @@
> writer.writeAttribute("value", currentValue, "value");
> }
> writer.endElement("input");
> -
> - FormRenderer.addRenderedHiddenField(context, clientId);
> }
> // The testcase for this class is TestRenderers_3.java
>
> Index: commandLink_test.txt
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/systest/web/golden/taglib/commandLink_test.txt,v
>
> retrieving revision 1.5
> diff -u -r1.5 commandLink_test.txt
> --- commandLink_test.txt 12 May 2004 04:35:07 -0000 1.5
> +++ commandLink_test.txt 2 Aug 2004 12:37:52 -0000
> @@ -11,17 +11,17 @@
> <body>
> - <form id="form01" method="post"
> action="/jsf-systest/faces/taglib/commandLink_test.jsp;jsessionid=1F42887EA431E608414DB7159B9B67C8"
> enctype="application/x-www-form-urlencoded">
> + <form id="form01" method="post"
> action="/jsf-systest/faces/taglib/commandLink_test.jsp;jsessionid=71102211E1D9689C845183191F9EE958"
> enctype="application/x-www-form-urlencoded">
>
> <a id="form01:hyperlink01" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink01';
> document.forms['form01'].submit(); return false;">My
> Link</a>
> <a id="form01:hyperlink02" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink02';
> document.forms['form01'].submit(); return false;">This is a String
> property</a>
> <a id="form01:hyperlink03" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink03';
> document.forms['form01'].submit(); return false;">RES-BUNDLE LINK</a>
> <a id="form01:hyperlink04" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink04';
> document.forms['form01'].submit(); return false;"><img src="duke.gif"
> /></a>
> - <a id="form01:hyperlink05" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink05';
> document.forms['form01'].submit(); return false;"><img
> src="resbundle_image.gif;jsessionid=1F42887EA431E608414DB7159B9B67C8"
> alt="" /></a>
> + <a id="form01:hyperlink05" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink05';
> document.forms['form01'].submit(); return false;"><img
> src="resbundle_image.gif;jsessionid=71102211E1D9689C845183191F9EE958"
> alt="" /></a>
> - <a id="form01:hyperlink06" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink06';document.forms['form01']['param1'].value='value1';
> document.forms['form01'].submit(); return false;">Paramter Link</a>
> - <input type="hidden" name="form01" value="form01" /><input
> type="hidden" name="param1" /><input type="hidden" name="form01:_idcl"
> /></form>
> + <a id="form01:hyperlink06" href="#"
> onclick="document.forms['form01']['form01:_idcl'].value='form01:hyperlink06';document.forms['form01']['value1'].value='value1';
> document.forms['form01'].submit(); return false;">Paramter Link</a>
> + <input type="hidden" name="form01" value="form01" /><input
> type="hidden" name="value1" /><input type="hidden" name="form01:_idcl"
> /></form>
> </body>
> </html>
>
> Index: RenderResponse_correct
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/web/test/RenderResponse_correct,v
> retrieving revision 1.109
> diff -u -r1.109 RenderResponse_correct
> --- RenderResponse_correct 28 Jul 2004 20:36:17 -0000 1.109
> +++ RenderResponse_correct 2 Aug 2004 12:38:39 -0000
> @@ -15,7 +15,7 @@
>
>
> -<form id="basicForm" method="post"
> action="/test/faces/TestRenderResponsePhase.jsp;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"
> class="formClass" accept-charset="some-charset" accept="html,wml"
> enctype="application/x-www-form-urlencoded" title="basicForm">
> +<form id="basicForm" method="post"
> action="/test/faces/TestRenderResponsePhase.jsp;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"
> class="formClass" accept-charset="some-charset" accept="html,wml"
> enctype="application/x-www-form-urlencoded" title="basicForm">
>
>
> <TABLE BORDER="1">
> @@ -130,12 +130,12 @@
>
> - <a
> id="basicForm:imageLink" href="#" style="someStyle"
> onclick="document.forms['basicForm']['basicForm:_idcl'].value='basicForm:imageLink';
> document.forms['basicForm'].submit(); return false;"><img
> src="duke.gif;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF" alt="" /></a>
> + <a id="basicForm:imageLink" href="#" style="someStyle"
> onclick="document.forms['basicForm']['basicForm:_idcl'].value='basicForm:imageLink';
> document.forms['basicForm'].submit(); return false;"><img
> src="duke.gif;jsessionid=F24CE7A21FF23E25526AC4B8004D6097" alt="" /></a>
>
> </TD>
>
> <TD>
> - <img id="basicForm:graphicImage"
> src="/test/duke.gif;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"
> style="someStyle" usemap="#map1" ismap="ismap" alt="" />
> + <img id="basicForm:graphicImage"
> src="/test/duke.gif;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"
> style="someStyle" usemap="#map1" ismap="ismap" alt="" />
> </TD>
>
> </TR>
> @@ -153,7 +153,7 @@
> - <a
> id="basicForm:commandParamLink" href="#"
> onclick="document.forms['basicForm']['basicForm:_idcl'].value='basicForm:commandParamLink';document.forms['basicForm']['name'].value='horwat';document.forms['basicForm']['value'].value='password';
> document.forms['basicForm'].submit(); return false;"
> class="hyperlinkClass">link text</a>
> + <a id="basicForm:commandParamLink" href="#"
> onclick="document.forms['basicForm']['basicForm:_idcl'].value='basicForm:commandParamLink';document.forms['basicForm']['horwat'].value='horwat';document.forms['basicForm']['password'].value='password';
> document.forms['basicForm'].submit(); return false;"
> class="hyperlinkClass">link text</a>
> </TD>
> </TR>
>
> @@ -169,7 +169,7 @@
> - <a
> id="basicForm:hrefParamLink" href="#"
> onclick="document.forms['basicForm']['basicForm:_idcl'].value='basicForm:hrefParamLink';document.forms['basicForm']['name'].value='horwat';document.forms['basicForm']['value'].value='password';
> document.forms['basicForm'].submit(); return false;"><img
> src="duke.gif;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF" alt="" /></a>
> + <a id="basicForm:hrefParamLink" href="#"
> onclick="document.forms['basicForm']['basicForm:_idcl'].value='basicForm:hrefParamLink';document.forms['basicForm']['horwat'].value='horwat';document.forms['basicForm']['password'].value='password';
> document.forms['basicForm'].submit(); return false;"><img
> src="duke.gif;jsessionid=F24CE7A21FF23E25526AC4B8004D6097" alt="" /></a>
> </TD>
> </TR>
>
> @@ -177,7 +177,7 @@
>
> <TD>
>
> - <a id="basicForm:outputLink"
> href="test.html;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"
> class="hyperlinkClass">output link text</a>
> + <a id="basicForm:outputLink"
> href="test.html;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"
> class="hyperlinkClass">output link text</a>
>
> </TD>
>
> @@ -189,12 +189,12 @@
>
> - <a
> id="basicForm:output_imageLink"
> href="test.html;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"
> style="position: absolute; left: 96px; top: 168px"><img
> src="duke.gif;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF" alt="" /></a>
> + <a id="basicForm:output_imageLink"
> href="test.html;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"
> style="position: absolute; left: 96px; top: 168px"><img
> src="duke.gif;jsessionid=F24CE7A21FF23E25526AC4B8004D6097" alt="" /></a>
>
> </TD>
>
> <TD>
> - <img id="basicForm:output_graphicImage"
> src="/test/duke.gif;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"
> usemap="#map1" ismap="ismap" alt="" />
> + <img id="basicForm:output_graphicImage"
> src="/test/duke.gif;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"
> usemap="#map1" ismap="ismap" alt="" />
> </TD>
>
> </TR>
> @@ -202,7 +202,7 @@
> <TR>
> <TD>
> - <a id="basicForm:output_commandLink"
> href="test.html;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"
> style="position: absolute; left: 96px; top: 168px"
> class="hyperlinkClass">link text</a>
> + <a id="basicForm:output_commandLink"
> href="test.html;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"
> style="position: absolute; left: 96px; top: 168px"
> class="hyperlinkClass">link text</a>
> </TD>
> </TR>
>
> @@ -212,13 +212,13 @@
> - <a
> id="basicForm:output_commandParamLink"
> href="test.html;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF?name=horwat&value=password"
> class="hyperlinkClass">link text</a>
> + <a id="basicForm:output_commandParamLink"
> href="test.html;jsessionid=F24CE7A21FF23E25526AC4B8004D6097?name=horwat&value=password"
> class="hyperlinkClass">link text</a>
> </TD>
> </TR>
>
> <TR>
> <TD>
> - <a id="basicForm:output_hrefLink"
> href="test.html;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF"><img
> src="duke.gif"></a>
> + <a id="basicForm:output_hrefLink"
> href="test.html;jsessionid=F24CE7A21FF23E25526AC4B8004D6097"><img
> src="duke.gif"></a>
> </TD>
> </TR>
>
> @@ -228,7 +228,7 @@
> - <a
> id="basicForm:output_hrefParamLink"
> href="test.html;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF?name=horwat&value=password"><img
> src="duke.gif;jsessionid=6E6598E4B463B472E7F9A6A3054DD3DF" alt="" /></a>
> + <a id="basicForm:output_hrefParamLink"
> href="test.html;jsessionid=F24CE7A21FF23E25526AC4B8004D6097?name=horwat&value=password"><img
> src="duke.gif;jsessionid=F24CE7A21FF23E25526AC4B8004D6097" alt="" /></a>
> </TD>
> </TR>
>
> @@ -846,7 +846,7 @@
> <span id="basicForm:userMsg">Param 0: my param</span>
>
>
> -<input type="hidden" name="basicForm" value="basicForm" /><input
> type="hidden" name="value" /><input type="hidden"
> name="basicForm:_idcl" /><input type="hidden" name="name" /></form>
> +<input type="hidden" name="basicForm" value="basicForm" /><input
> type="hidden" name="password" /><input type="hidden" name="horwat"
> /><input type="hidden" name="basicForm:_idcl" /></form>
>
>
> </BODY>
>
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net