You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2010/09/25 04:23:34 UTC

Re: svn commit: r1001097 - /ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl

jleroux@apache.org wrote:
> Author: jleroux
> Date: Fri Sep 24 22:18:34 2010
> New Revision: 1001097
> 
> URL: http://svn.apache.org/viewvc?rev=1001097&view=rev
> Log:
> Fixes typos
> 
> Modified:
>     ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
> 
> Modified: ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
> URL: http://svn.apache.org/viewvc/ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl?rev=1001097&r1=1001096&r2=1001097&view=diff
> ==============================================================================
> --- ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl (original)
> +++ ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl Fri Sep 24 22:18:34 2010
> @@ -33,16 +33,16 @@ jQuery(document).ready(function() {
>    if (isWidget) {                     
>        widget.asmSelect({
>          addItemTarget: 'top',
> -        sortable: ${asm_sortable}!'false'},
> -        removeLabel: '${uiLabelMap.CommonRemove}!'Remove'}'
> +        sortable: ${asm_sortable!'false'},
> +        removeLabel: '${uiLabelMap.CommonRemove!'Remove'}'
>      });
>    }
>    // use asmSelect in Freemarker Templates
>    else if (isFtl) {    
>        ftl.asmSelect({
>          addItemTarget: 'top',
> -        sortable: ${asm_sortable}!'false'},
> -        removeLabel: '${uiLabelMap.CommonRemove}!'Remove'}'
> +        sortable: ${asm_sortable!'false'},
> +        removeLabel: '${uiLabelMap.CommonRemove!'Remove'}'
>          //,debugMode: true

This will break if the CommonRemove label has quotes, or other special
chars.  The example here, 'remove', may not have special chars, but in
general, you need to be careful when stuffing things into javascript.


Re: svn commit: r1001097 - /ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl

Posted by Adam Heath <do...@brainfood.com>.
Jacques Le Roux wrote:
> From: "Adam Heath" <do...@brainfood.com>
>> jleroux@apache.org wrote:
>>> Author: jleroux
>>> Date: Fri Sep 24 22:18:34 2010
>>> New Revision: 1001097
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1001097&view=rev
>>> Log:
>>> Fixes typos
>>>
>>> Modified:
>>>    
>>> ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
>>>
>>>
>>> Modified:
>>> ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl?rev=1001097&r1=1001096&r2=1001097&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
>>> (original)
>>> +++
>>> ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
>>> Fri Sep 24 22:18:34 2010
>>> @@ -33,16 +33,16 @@ jQuery(document).ready(function() {
>>>    if (isWidget) {
>>>        widget.asmSelect({
>>>          addItemTarget: 'top',
>>> -        sortable: ${asm_sortable}!'false'},
>>> -        removeLabel: '${uiLabelMap.CommonRemove}!'Remove'}'
>>> +        sortable: ${asm_sortable!'false'},
>>> +        removeLabel: '${uiLabelMap.CommonRemove!'Remove'}'
>>>      });
>>>    }
>>>    // use asmSelect in Freemarker Templates
>>>    else if (isFtl) {
>>>        ftl.asmSelect({
>>>          addItemTarget: 'top',
>>> -        sortable: ${asm_sortable}!'false'},
>>> -        removeLabel: '${uiLabelMap.CommonRemove}!'Remove'}'
>>> +        sortable: ${asm_sortable!'false'},
>>> +        removeLabel: '${uiLabelMap.CommonRemove!'Remove'}'
>>>          //,debugMode: true
>>
>> This will break if the CommonRemove label has quotes, or other special
>> chars.  The example here, 'remove', may not have special chars, but in
>> general, you need to be careful when stuffing things into javascript.
> 
> 
> Thanks for the advice, could be a nightmare to track indeed.

The correct solution, is to build a collection(list, map, whatever),
storing simple values in it, then convert it to json.  Trying to do
individual quoting on singleton vars won't scale.

There is some code in UtilIO, that makes use of JSONWriter(written by
me), that could help with this.  Maybe copy *just* the json parts, and
add a new toJSON() method.


Re: svn commit: r1001097 - /ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl

Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adam Heath" <do...@brainfood.com>
> jleroux@apache.org wrote:
>> Author: jleroux
>> Date: Fri Sep 24 22:18:34 2010
>> New Revision: 1001097
>>
>> URL: http://svn.apache.org/viewvc?rev=1001097&view=rev
>> Log:
>> Fixes typos
>>
>> Modified:
>>     ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
>>
>> Modified: ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl
>> URL: 
>> http://svn.apache.org/viewvc/ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl?rev=1001097&r1=1001096&r2=1001097&view=diff
>> ==============================================================================
>> --- ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl (original)
>> +++ ofbiz/branches/jquery/framework/common/webcommon/includes/setMultipleSelectJs.ftl Fri Sep 24 22:18:34 2010
>> @@ -33,16 +33,16 @@ jQuery(document).ready(function() {
>>    if (isWidget) {
>>        widget.asmSelect({
>>          addItemTarget: 'top',
>> -        sortable: ${asm_sortable}!'false'},
>> -        removeLabel: '${uiLabelMap.CommonRemove}!'Remove'}'
>> +        sortable: ${asm_sortable!'false'},
>> +        removeLabel: '${uiLabelMap.CommonRemove!'Remove'}'
>>      });
>>    }
>>    // use asmSelect in Freemarker Templates
>>    else if (isFtl) {
>>        ftl.asmSelect({
>>          addItemTarget: 'top',
>> -        sortable: ${asm_sortable}!'false'},
>> -        removeLabel: '${uiLabelMap.CommonRemove}!'Remove'}'
>> +        sortable: ${asm_sortable!'false'},
>> +        removeLabel: '${uiLabelMap.CommonRemove!'Remove'}'
>>          //,debugMode: true
>
> This will break if the CommonRemove label has quotes, or other special
> chars.  The example here, 'remove', may not have special chars, but in
> general, you need to be careful when stuffing things into javascript.


Thanks for the advice, could be a nightmare to track indeed.

As soon as possible I will rewrite this stuff to be handled directly from widgets and templates (at r1001161 I have abandoned the 
idea to adapt to templates where an id is not used, it will be easier to put ids there).

It was easier to begin like that and fortunately there were not much cases to handle. The parts which are more thoughtful is when 
you have to handle a relation between 2 widgets (dependent dropdowns, here is another way to see it, look into Webtools/Geo 
Management/Link Zones for an example)

Jacques