13 messages in net.java.dev.javaserverfaces.devRe: REVIEW - Issue 26 - Tightly Coupl...| From | Sent On | Attachments |
|---|---|---|
| Roger Kitain | Aug 2, 2004 5:41 am | |
| Craig R. McClanahan | Aug 2, 2004 9:46 am | |
| Roger Kitain | Aug 2, 2004 11:42 am | |
| Craig R. McClanahan | Aug 2, 2004 12:05 pm | |
| Roger Kitain | Aug 2, 2004 3:05 pm | |
| Craig R. McClanahan | Aug 2, 2004 3:10 pm | |
| Roger Kitain | Aug 2, 2004 3:26 pm | |
| Craig R. McClanahan | Aug 2, 2004 3:57 pm | |
| Roger Kitain | Aug 3, 2004 5:15 am | |
| Roger Kitain | Aug 4, 2004 7:11 am | |
| Adam Winer | Aug 4, 2004 9:16 am | |
| Adam Winer | Aug 5, 2004 1:27 pm | |
| Roger Kitain | Aug 6, 2004 4:08 am |
| Subject: | Re: REVIEW - Issue 26 - Tightly Coupled Renderer Dependencies | |
|---|---|---|
| From: | Roger Kitain (Roge...@Sun.COM) | |
| Date: | Aug 2, 2004 11:42:56 am | |
| List: | net.java.dev.javaserverfaces.dev | |
Hi Craig -
I see your point. However, part of the requirement (for things to work properly) is that there can be only one hidden field rendered per form - pertaining to the command links on that form. So, or example:
<form id="_id0" method="post" action="/jsf-standard/faces/commandLink_multiform_test.jsp" enctype="application/x-www-form-urlencoded">
<a href="#" onclick="document.forms['_id0']['_id0:_idcl'].value='_id0:_id1'; document.forms['_id0'].submit(); return false;">Link1</a>
<a href="#" onclick="document.forms['_id0']['_id0:_idcl'].value='_id0:_id3'; document.forms['_id0'].submit(); return false;">Link2</a>
<input type="hidden" name="com.sun.faces.VIEW" value="H4..."/> <input type="hidden" name="_id0" value="_id0" /><input type="hidden" name="_id0:_idcl" /></form>
<form id="_id5" method="post" action="/jsf-standard/faces/commandLink_multiform_test.jsp" enctype="application/x-www-form-urlencoded">
<a href="#" onclick="document.forms['_id5']['_id5:_idcl'].value='_id5:_id6'; document.forms['_id5'].submit(); return false;">Link3</a>
<a href="#" onclick="document.forms['_id5']['_id5:_idcl'].value='_id5:_id8';document.forms['_id5']['foo'].value='foo';document.forms['_id5']['foo'].value='foo'; document.forms['_id5'].submit(); return false;">Link4</a
<a href="#" onclick="document.forms['_id5']['_id5:_idcl'].value='_id5:_id12';document.forms['_id5']['bar'].value='bar';document.forms['_id5']['bar'].value='bar'; document.forms['_id5'].submit(); return false;">Link5</a>
<input type="hidden" name="com.sun.faces.VIEW" value="H4..."/>
<input type="hidden" name="_id5" value="_id5" /><input type="hidden" name="foo"/>
<input type="hidden" name="_id5:_idcl" /><input type="hidden" name="bar" /></form>
</body> </html>
So, somewhere we need to keep track of what's been rendered.. This isn't in the spec, but, can the FormRenderer take on that responsiblity? The FormRenderer would need to only render a *one* hidden field pertaining to its commandLink children... So, commandLinkRenderer would just simply do it's rendering.... You would need to implement *some* logic in your struts-faces form renderer to meet this requirement...
Comments?
Craig R. McClanahan wrote:
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@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>