Re: svn commit: r1435528 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java

[Adrian Crum-3]

If there are duplicate screenlet IDs, then that is a flaw in the screen
widget XML file, not in the renderer. In other words, it is caused by
user error.

This change is not necessary.

-Adrian

On 1/19/2013 12:27 PM, [hidden email] wrote:

> Author: jleroux
> Date: Sat Jan 19 12:27:25 2013
> New Revision: 1435528
>
> URL: http://svn.apache.org/viewvc?rev=1435528&view=rev
> Log:
> A patch from Sumit Pandit for "HTML Validation error - Duplicate DIV id is created in <screenlet title=...></screenlet> tag." https://issues.apache.org/jira/browse/OFBIZ-5121
>
> When ID has not been provided in <screenlet title=.../> tag, The generated DIV has an ID="_col". Including multiple screenlet in a page get result in multiple DIVs with duplicate ID ("_col").
>
> For reference find duplicate div id("_col") in following page -
> https://demo-trunk.ofbiz.apache.org/catalog/control/EditProdCatalogCategories?prodCatalogId=DemoCatalog
>
>
> Modified:
>      ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
>
> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java?rev=1435528&r1=1435527&r2=1435528&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java (original)
> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java Sat Jan 19 12:27:25 2013
> @@ -74,6 +74,7 @@ public class MacroScreenRenderer impleme
>       private int elementId = 999;
>       protected boolean widgetCommentsEnabled = false;
>       private static final String formrenderer = UtilProperties.getPropertyValue("widget", "screen.formrenderer");
> +    private int screenLetsIdCounter = 1;
>  
>       public MacroScreenRenderer(String name, String macroLibraryPath) throws TemplateException, IOException {
>           macroLibrary = FreeMarkerWorker.getTemplate(macroLibraryPath);
> @@ -627,11 +628,17 @@ public class MacroScreenRenderer impleme
>           }
>  
>           Map<String, Object> parameters = FastMap.newInstance();
> -        parameters.put("id", screenlet.getId(context));
>           parameters.put("title", title);
>           parameters.put("collapsible", collapsible);
>           parameters.put("saveCollapsed", screenlet.saveCollapsed());
> -        parameters.put("collapsibleAreaId", screenlet.getId(context) + "_col");
> +        if (UtilValidate.isNotEmpty (screenlet.getId(context))) {
> +            parameters.put("id", screenlet.getId(context));
> +            parameters.put("collapsibleAreaId", screenlet.getId(context) + "_col");
> +        } else {
> +            parameters.put("id", "screenlet_" + screenLetsIdCounter);
> +            parameters.put("collapsibleAreaId","screenlet_" + screenLetsIdCounter + "_col");
> +            screenLetsIdCounter++;
> +        }
>           parameters.put("expandToolTip", expandToolTip);
>           parameters.put("collapseToolTip", collapseToolTip);
>           parameters.put("fullUrlString", fullUrlString);
>
>

[Jacques Le Roux]

Theoritically, since I found this in many places and it can't hurt, at least in the meantime if ever someone want to tackle all cases, I prefer to keep it.

Another one https://demo-trunk.ofbiz.apache.org/webtools/control/FindGeneric?entityName=OrderHeader&find=true&VIEW_SIZE=50&VIEW_INDEX=0
etc.

Jacques

From: "Adrian Crum" <[hidden email]>

> If there are duplicate screenlet IDs, then that is a flaw in the screen
> widget XML file, not in the renderer. In other words, it is caused by
> user error.
>
> This change is not necessary.
>
> -Adrian
>
> On 1/19/2013 12:27 PM, [hidden email] wrote:
>> Author: jleroux
>> Date: Sat Jan 19 12:27:25 2013
>> New Revision: 1435528
>>
>> URL: http://svn.apache.org/viewvc?rev=1435528&view=rev
>> Log:
>> A patch from Sumit Pandit for "HTML Validation error - Duplicate DIV id is created in <screenlet title=...></screenlet> tag." https://issues.apache.org/jira/browse/OFBIZ-5121
>>
>> When ID has not been provided in <screenlet title=.../> tag, The generated DIV has an ID="_col". Including multiple screenlet in a page get result in multiple DIVs with duplicate ID ("_col").
>>
>> For reference find duplicate div id("_col") in following page -
>> https://demo-trunk.ofbiz.apache.org/catalog/control/EditProdCatalogCategories?prodCatalogId=DemoCatalog
>>
>>
>> Modified:
>>      ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
>>
>> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java?rev=1435528&r1=1435527&r2=1435528&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java (original)
>> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java Sat Jan 19 12:27:25 2013
>> @@ -74,6 +74,7 @@ public class MacroScreenRenderer impleme
>>       private int elementId = 999;
>>       protected boolean widgetCommentsEnabled = false;
>>       private static final String formrenderer = UtilProperties.getPropertyValue("widget", "screen.formrenderer");
>> +    private int screenLetsIdCounter = 1;
>>  
>>       public MacroScreenRenderer(String name, String macroLibraryPath) throws TemplateException, IOException {
>>           macroLibrary = FreeMarkerWorker.getTemplate(macroLibraryPath);
>> @@ -627,11 +628,17 @@ public class MacroScreenRenderer impleme
>>           }
>>  
>>           Map<String, Object> parameters = FastMap.newInstance();
>> -        parameters.put("id", screenlet.getId(context));
>>           parameters.put("title", title);
>>           parameters.put("collapsible", collapsible);
>>           parameters.put("saveCollapsed", screenlet.saveCollapsed());
>> -        parameters.put("collapsibleAreaId", screenlet.getId(context) + "_col");
>> +        if (UtilValidate.isNotEmpty (screenlet.getId(context))) {
>> +            parameters.put("id", screenlet.getId(context));
>> +            parameters.put("collapsibleAreaId", screenlet.getId(context) + "_col");
>> +        } else {
>> +            parameters.put("id", "screenlet_" + screenLetsIdCounter);
>> +            parameters.put("collapsibleAreaId","screenlet_" + screenLetsIdCounter + "_col");
>> +            screenLetsIdCounter++;
>> +        }
>>           parameters.put("expandToolTip", expandToolTip);
>>           parameters.put("collapseToolTip", collapseToolTip);
>>           parameters.put("fullUrlString", fullUrlString);
>>
>>
>

[Sumit Pandit]

Hello Adrian,
The issue come when developer does not provide id for screenlet. For reference please have a look at product/widget/catalog/CatalogScreens.xml#EditProdCatalogCategories; Screenlets ar line #112 and #115 ids are missing.

In above case a default id="_col" is created. The fix is generate and assigned a unique id for such screenlets.

--
Thanks And Regards
Sumit Pandit
Tata Consultancy Services
Cell:- 917503046188
____________________________________________

----- Original Message -----

| From: "Adrian Crum" <[hidden email]>
| To: [hidden email]
| Sent: Saturday, January 19, 2013 6:38:26 PM
| Subject: Re: svn commit: r1435528 -
| /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java

| If there are duplicate screenlet IDs, then that is a flaw in the
| screen
| widget XML file, not in the renderer. In other words, it is caused by
| user error.

| This change is not necessary.

| -Adrian

| On 1/19/2013 12:27 PM, [hidden email] wrote:
| > Author: jleroux
| > Date: Sat Jan 19 12:27:25 2013
| > New Revision: 1435528
| >
| > URL: http://svn.apache.org/viewvc?rev=1435528&view=rev
| > Log:
| > A patch from Sumit Pandit for "HTML Validation error - Duplicate
| > DIV id is created in <screenlet title=...></screenlet> tag."
| > https://issues.apache.org/jira/browse/OFBIZ-5121
| >
| > When ID has not been provided in <screenlet title=.../> tag, The
| > generated DIV has an ID="_col". Including multiple screenlet in a
| > page get result in multiple DIVs with duplicate ID ("_col").
| >
| > For reference find duplicate div id("_col") in following page -
| > https://demo-trunk.ofbiz.apache.org/catalog/control/EditProdCatalogCategories?prodCatalogId=DemoCatalog
| >
| >
| > Modified:
| > ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
| >
| > Modified:
| > ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
| > URL:
| > http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java?rev=1435528&r1=1435527&r2=1435528&view=diff
| > ==============================================================================
| > ---
| > ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
| > (original)
| > +++
| > ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/MacroScreenRenderer.java
| > Sat Jan 19 12:27:25 2013
| > @@ -74,6 +74,7 @@ public class MacroScreenRenderer impleme
| > private int elementId = 999;
| > protected boolean widgetCommentsEnabled = false;
| > private static final String formrenderer =
| > UtilProperties.getPropertyValue("widget", "screen.formrenderer");
| > + private int screenLetsIdCounter = 1;
| >
| > public MacroScreenRenderer(String name, String macroLibraryPath)
| > throws TemplateException, IOException {
| > macroLibrary = FreeMarkerWorker.getTemplate(macroLibraryPath);
| > @@ -627,11 +628,17 @@ public class MacroScreenRenderer impleme
| > }
| >
| > Map<String, Object> parameters = FastMap.newInstance();
| > - parameters.put("id", screenlet.getId(context));
| > parameters.put("title", title);
| > parameters.put("collapsible", collapsible);
| > parameters.put("saveCollapsed", screenlet.saveCollapsed());
| > - parameters.put("collapsibleAreaId", screenlet.getId(context) +
| > "_col");
| > + if (UtilValidate.isNotEmpty (screenlet.getId(context))) {
| > + parameters.put("id", screenlet.getId(context));
| > + parameters.put("collapsibleAreaId", screenlet.getId(context) +
| > "_col");
| > + } else {
| > + parameters.put("id", "screenlet_" + screenLetsIdCounter);
| > + parameters.put("collapsibleAreaId","screenlet_" +
| > screenLetsIdCounter + "_col");
| > + screenLetsIdCounter++;
| > + }
| > parameters.put("expandToolTip", expandToolTip);
| > parameters.put("collapseToolTip", collapseToolTip);
| > parameters.put("fullUrlString", fullUrlString);
| >
| >
=====-----=====-----=====
Notice: The information contained in this e-mail
message and/or attachments to it may contain
confidential or privileged information. If you are
not the intended recipient, any dissemination, use,
review, distribution, printing or copying of the
information contained in this e-mail message
and/or attachments to it are strictly prohibited. If
you have received this communication in error,
please notify us by reply e-mail or telephone and
immediately and permanently delete the message
and any attachments. Thank you

[Adrian Crum-3]

I understand that. The proper fix would be to check if the model
contains an ID, and only create the HTML id attribute if one exists. All
of the extra code to maintain a counter is not necessary.

-Adrian

On 1/21/2013 5:40 AM, Sumit Pandit wrote:
> Hello Adrian,
> The issue come when developer does not provide id for screenlet. For reference please have a look at product/widget/catalog/CatalogScreens.xml#EditProdCatalogCategories; Screenlets ar line #112 and #115 ids are missing.
>
> In above case a default id="_col" is created. The fix is generate and assigned a unique id for such screenlets.
>

[Jacques Le Roux]

This makes sense indeed Adrian

Jacques

From: "Adrian Crum" <[hidden email]>

>I understand that. The proper fix would be to check if the model
> contains an ID, and only create the HTML id attribute if one exists. All
> of the extra code to maintain a counter is not necessary.
>
> -Adrian
>
> On 1/21/2013 5:40 AM, Sumit Pandit wrote:
>> Hello Adrian,
>> The issue come when developer does not provide id for screenlet. For reference please have a look at product/widget/catalog/CatalogScreens.xml#EditProdCatalogCategories; Screenlets ar line #112 and #115 ids are missing.
>>
>> In above case a default id="_col" is created. The fix is generate and assigned a unique id for such screenlets.
>>
>