[Laszlo-dev] For Review: Change 20090423-maxcarlson-5 Summary: Add warnings for non-string LzDataElement attributes

[André Bargull]

I think the warnings produce more noise than actually necessary. And 
it's not really forbidden to use non-string values, the values will just 
be coerced to strings, so a warning is also a bit misleading because 
people may think they must not use non-string values (which isn't true 
as explained).
If it's considered to risky to change (which means to fix) the behaviour 
in one release without further notification, I like to propose to back 
out the warnings and the coercion, and instead make a release note for 
the next release (4.4?) explaining the upcoming change for the release 
after that one (4.5?).
Thoughts?


On 4/25/2009 9:08 PM, Max Carlson wrote:
> I agree completely - I'm just worried about users who will get bit by 
> this, since we didn't enforce the restriction before.  One of our 
> biggest customers (webtop) is seeing a lot of warnings related to this 
> change...
> 
> André Bargull wrote:
>> But string coercion is already enforced in 
>> "lz.DataElement#setAttr(..)", it's actually required by the DOM-spec 
>> [1] and without coercion you can't work reliably with xpaths.
>> For example this testcase only works correctly in trunk, if you try it 
>> in OL4.3, you'll only see the text-element displaying "first".
>>
>>> <canvas debug="true">   <dataset name="ds" />
>>>   <vbox>
>>>     <text datapath="ds:/item[@shown='true']/text()" />
>>>   </vbox>
>>>   <handler name="oninit">
>>>     canvas.ds.appendChild(new lz.DataElement('item', {shown: 'true'}, 
>>> [new lz.DataText("first")]));
>>>     canvas.ds.appendChild(new lz.DataElement('item', {shown: true}, 
>>> [new lz.DataText("second")]));
>>>   </handler>
>>> </canvas>
>>
>>
>> [1] 
>> http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-F68F082 
>>
>>
>>
>>
>> On 4/25/2009 7:17 PM, Max Carlson wrote:
>>> I think we should back out the string coercion for at least one 
>>> release, and leave in these warnings.  This is a pretty major change 
>>> for folks that use LzDataElements to store non-string attribute 
>>> values.  Thoughts?
>>>
>>> André Bargull wrote:
>>>> Two nits:
>>>> - please add `*`-type to the "val"-variable
>>>> - maybe it should say "lz.DataElement" instead of "LzDataElement"
>>>>
>>>> Otherwise approved.
>>>>
>>>>
>>>> On 4/24/2009 12:50 AM, Max Carlson wrote:
>>>>> Change 20090423-maxcarlson-5 by maxcarlson at Bank.local on 2009-04-23 
>>>>> 15:45:43 PDT
>>>>>     in /Users/maxcarlson/openlaszlo/trunk-clean
>>>>>     for http://svn.openlaszlo.org/openlaszlo/trunk
>>>>>
>>>>> Summary: Add warnings for non-string LzDataElement attributes
>>>>>
>>>>> Bugs Fixed: LPP-8083 - LzDataElement.setAttribute('attributes', 
>>>>> {...}) forces attribute values to strings
>>>>>
>>>>> Technical Reviewer: andre.bargull at udo.edu
>>>>> QA Reviewer: ptw at laszlosystems.com
>>>>>
>>>>> Details: Add warnings when an attribute is assigned a non-string 
>>>>> value.
>>>>>
>>>>> Tests: See LPP-8083
>>>>>
>>>>> Files:
>>>>> M      WEB-INF/lps/lfc/data/LzDataElement.lzs
>>>>>
>>>>> Changeset: 
>>>>> http://svn.openlaszlo.org/openlaszlo/patches/20090423-maxcarlson-5.tar
>>>>>
>>>
>