Hi Hans,That all well and good but what I have a problem with is short term solutions to long term problems, especially in the framework where things can be difficult to change further down the track. The widget code especially can be very difficult to navigate if things are allowed to get messy.
My personal preference would be that things remain broken until a solid solution is put into place but I haven't got the time to propose or create one so I guess we just have to put up with these hackish solutions until someone has the time to do it properly.
Even if you chose not to create a jira issue and patch for every single contribution I would beg with you to consider doing it for framework contributions and allow the community to have a say about changes to such an important piece of OFBiz.
Regards Scott On 1/10/2009, at 3:24 PM, Hans Bakker wrote:
Hi Scott, The tip for the loop counter of freemarker is taken and we will change it. If you have a proposal to fix this error a better way sure we have no problem accepting it. As it is now, it fixes a real problem because it happens very frequently that portlets are called pretty often in the same screen. By the way, that is good so we reuse code. We are thinking about making text portlets for blocks of text where wecan build normal websites from. This error however always prevented thisdevelopment. Regards, Hans On Wed, 2009-09-30 at 23:47 +1300, Scott Gray wrote:1. You don't need to use a counter in FreeMarker loops, there is a built-in variable provided2. Having to know in advance that you are going to render a form more than once is far from an ideal solution, what is two different screensuse the same form on the same page? Quick fixes do not belong in the framework. 3. What about screens that use the screen cache? 4. The ScreenRenderer class now has a field calledrenderFormSeqNumber even though the ScreenRenderer knows nothing aboutthe form widget.In general I think this solution is messy and not well thought out, itconcerns me because we already have the iterateId solution you provided the other day and I'm worried that we are going to end up with a whole stack of strange variables that make no sense to look at in the code. I would prefer to see a solution where the widgets or the screens or the context keeps track of what widgets have previously been renderedand have then automatically take care of uniquely identifying the formelements themselves. Regards Scott On 30/09/2009, at 10:51 PM, [email protected] wrote:Author: hansbak Date: Wed Sep 30 09:51:01 2009 New Revision: 820232 URL: http://svn.apache.org/viewvc?rev=820232&view=rev Log: fix for OFBIZ-2414: if a portlet is included more than once the links do not work, this fix will make these links unique. Sponsored By Antwebsystems Co.Ltd: employee Berm Modified: ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftlofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.javaofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java Modified: ofbiz/trunk/framework/common/webcommon/portal/ showPortalPage.ftl URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl?rev=820232&r1=820231&r2=820232&view=diff = = = = = = = == = ====================================================================--- ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl (original) +++ ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl Wed Sep 30 09:51:01 2009 @@ -20,6 +20,7 @@ <#if portalPage?has_content> <table width="100%"> <tr> + <#assign renderSeq = 0/> <#list portalPageColumns?if_exists as portalPageColumn> <td style="vertical-align: top; <#if portalPageColumn.columnWidthPercentage?has_content> width:$ {portalPageColumn.columnWidthPercentage}%;</#if>"> <#assign firstInColumn = true/> @@ -31,7 +32,9 @@ ${setRequestAttribute("portalPortletId", portlet.portalPortletId)} ${setRequestAttribute("portletSeqId", portlet.portletSeqId)} ${screens.render(portlet.screenLocation, portlet.screenName)} + ${screens.setRenderFormUniqueSeq(renderSeq)} </div> + <#assign renderSeq = renderSeq+1/> </#if> <#assign firstInColumn = false/> </#if> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=820232&r1=820231&r2=820232&view=diff = = = = = = = == = ====================================================================--- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java Wed Sep 30 09:51:01 2009 @@ -268,15 +268,19 @@ ModelForm modelForm = modelFormField.getModelForm(); Integer itemIndex = (Integer) context.get("itemIndex"); String iterateId = ""; + String formUniqueId = ""; String formName = (String) context.get("formName"); if (UtilValidate.isEmpty(formName)) { formName = modelForm.getName(); } if (UtilValidate.isNotEmpty(context.get("iterateId"))) { - iterateId = (String) context.get("iterateId"); + iterateId = (String) context.get("iterateId"); + } + if (UtilValidate.isNotEmpty(context.get("formUniqueId"))) { + formUniqueId = (String) context.get("formUniqueId"); } if (itemIndex != null) { - return formName + modelForm.getItemIndexSeparator() + itemIndex.intValue() + iterateId + modelForm.getItemIndexSeparator() + modelFormField.getName(); + return formName + modelForm.getItemIndexSeparator() + itemIndex.intValue() + iterateId + formUniqueId + modelForm.getItemIndexSeparator() + modelFormField.getName(); } else { return formName + modelForm.getItemIndexSeparator() + modelFormField.getName(); } Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelForm.java?rev=820232&r1=820231&r2=820232&view=diff = = = = = = = == = ====================================================================--- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java Wed Sep 30 09:51:01 2009 @@ -1423,6 +1423,10 @@ ModelFormAction.runSubActions(this.rowActions, localContext); localContext.put("itemIndex", Integer.valueOf(itemIndex - lowIndex)); + if (UtilValidate.isNotEmpty(context.get("renderFormSeqNumber"))) { + localContext.put("formUniqueId", "_"+context.get("renderFormSeqNumber")); + } + this.resetBshInterpreter(localContext); if (Debug.verboseOn()) Debug.logVerbose("In form got another row, context is: " + localContext, module); Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ScreenRenderer.java?rev=820232&r1=820231&r2=820232&view=diff = = = = = = = == = ====================================================================--- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java Wed Sep 30 09:51:01 2009 @@ -73,6 +73,7 @@ protected Appendable writer; protected MapStack<String> context; protected ScreenStringRenderer screenStringRenderer; + protected int renderFormSeqNumber = 0; public ScreenRenderer(Appendable writer, MapStack<String> context, ScreenStringRenderer screenStringRenderer) { this.writer = writer; @@ -129,10 +130,15 @@ writer.append(gwo.toString()); } } else { + context.put("renderFormSeqNumber", String.valueOf(renderFormSeqNumber)); modelScreen.renderScreenString(writer, context, screenStringRenderer); } return ""; } + + public void setRenderFormUniqueSeq (int renderFormSeqNumber) { + this.renderFormSeqNumber = renderFormSeqNumber; + } public ScreenStringRenderer getScreenStringRenderer() { return this.screenStringRenderer;-- Antwebsystems.com: Quality OFBiz services for competitive rates
smime.p7s
Description: S/MIME cryptographic signature
