[Laszlo-dev] For Review: Change 20071102-ptw-t Summary: Uses 'plists' rather than Object's to pass instance attributes

[P T Withington]

[cc-ing Don -- can you look at the CommonGenerator.java change as  
André mentions in comment 1?]

On 2007-11-05, at 18:33 EST, André Bargull wrote:

> Approved!
> comments:
> 1) you may want to assign someone else for reviewing the changes in  
> "CommonGenerator.java", because I haven't got any experience of the  
> OL-Server
> 2) you missed these files for updating DeclareEvent(..) to  
> LzDeclaredEvent.
>   trunk/lps/components/extensions/drawview.lzx
>   trunk/lps/components/utils/replicator/replicator.lzx
>   trunk/lps/components/utils/traits/cssable.lzx

Thanks.  Bad regexp in tags-query-replace.  I think I have them all now!
>
> 3) some notes on Class.lzs:
>   a) do you really want to pop all name-value pairs from the static-/ 
> instanceProperties arrays? What if someone needs to reuse these  
> plists?

You suggestion is that someone may pass these in as parameters and not  
expect them to be modified.  Unlikely but plausible.  I will change to  
iterating over the plists instead.

>   b) this.instanceProperties.push(name);
>       this.instanceProperties.push(value);
>       => just for brevity: this.instanceProperties.push(name, value);

Sure.  I always forget that push can take more than one argument.

> 4) toString for user-classes does not work, but this is not related  
> to this changeset.
>   (all toString-methods for user-classes get installed on  
> "_instanceAttrs", do we have got a JIRA for this?)

We don't, but that will be subsumed by the project to make the tag  
compiler output JS2 class declarations.  [Hm.  I can't seem to find a  
Jira task for that.  If I could, I would add your comment to it.]

> 5) I guess it's not that important if "toString" et. al. are skipped  
> in "Instance.validateClassStructure(..)" for IE, is it?

Not crucial, since that is supposed to go away eventually.  That is  
meant to catch people who add a method manually to a class prototype  
(rather than using the approved `addProperty` interface.  If such a  
manual add is not caught, `super` will not work in the manually added  
method...  If you think this is important enough to be addressed, file  
a bug.  It would mean that classes would have to keep a copy of the  
plist arguments to validate against.

I'm sending you a change that addresses comments 2 and 3.

> On 11/5/2007 12:05 AM, P T Withington wrote:
>> Change 20071102-ptw-t by ptw at dueling-banjos.local on 2007-11-02  
>> 22:24:07 EDT
>>    in /Users/ptw/OpenLaszlo/ringding-2
>>    for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: Uses 'plists' rather than Object's to pass instance  
>> attributes
>>    to Class.make
>>
>> Bugs Fixed:
>> LPP-4997 'DHTML: text.format() doesn't work unless debugging  
>> enabled.'
>>
>> Technical Reviewer: a.bargull at intensis.de (pending)
>> QA Reviewer: philip at pbrdev.com (pending)
>>
>> Details:
>>    Using Object's as hashes is fraught with danger because our
>>    favorite browser has quirks in how it enumerates the properties of
>>    an object; hence replace all uses of Object/hash in the Class
>>    framework with a 'plist' (i.e., a property list, i.e., an array of
>>    alternating key/value entries).
>>
>>    lztest-class-impl, UserClass: Use new protocol.
>>
>>    LzLoader, LzMediaLoader, LzContextMenuItem, LzLibrary,
>>    LzContextMenu, LzKeys, LzHistory, LzGlobalMouse, LzFocus, LzNode,
>>    LzInputText, LzText, LaszloView, LaszloCanvas, LzCommand, LzState,
>>    LzAnimatorGroup, LzReplicationManager, LzDatapointer, LzDataText,
>>    LzDataNode, LzDataRequest, LzDataElement, LzDataset, LzDatapath,
>>    Replace all calls to DeclareEvent with instance var declaration
>>    with LzDeclaredEvent initial value.  (We want this anyways so we
>>    don't have to munge class prototypes at runtime, but it is needed
>>    in this change because Mixin's no longer store their instance
>>    properties on a psuedo-prototype object.)
>>
>>    LzDefs: remove DeclareEvent(s).
>>
>>    LzFormatter: Make the bootstrap LzMessage class more like the real
>>    class.  Fix some type-ohs.
>>
>>    Class, CommonGenerator: replace hashes with plists.
>>
>>    Class: remove obsolete 'make' and passThrough's.
>>
>> Tests:
>>    Test case from bug passes (in IE6)
>>
>>    smokecheck, lztest
>>
>> Files:
>> M      test/lztest/lztest-class-impl.lzx
>> M      WEB-INF/lps/lfc/kernel/swf/LzLoader.lzs
>> M      WEB-INF/lps/lfc/kernel/swf/LzMediaLoader.lzs
>> M      WEB-INF/lps/lfc/kernel/swf/LzContextMenu.lzs
>> M      WEB-INF/lps/lfc/kernel/dhtml/LzLibrary.js
>> M      WEB-INF/lps/lfc/kernel/dhtml/LzContextMenu.js
>> M      WEB-INF/lps/lfc/services/platform/swf/LzKeys.as
>> M      WEB-INF/lps/lfc/services/platform/swf/LzHistory.as
>> M      WEB-INF/lps/lfc/services/platform/dhtml/LzHistory.js
>> M      WEB-INF/lps/lfc/services/platform/dhtml/LzKeys.js
>> M      WEB-INF/lps/lfc/services/LzGlobalMouse.lzs
>> M      WEB-INF/lps/lfc/services/LzIdle.lzs
>> M      WEB-INF/lps/lfc/services/LzFocus.lzs
>> M      WEB-INF/lps/lfc/services/LzModeManager.lzs
>> M      WEB-INF/lps/lfc/core/LzNode.lzs
>> M      WEB-INF/lps/lfc/core/UserClass.lzs
>> M      WEB-INF/lps/lfc/core/LzDefs.lzs
>> M      WEB-INF/lps/lfc/views/LzInputText.lzs
>> M      WEB-INF/lps/lfc/views/LzText.lzs
>> M      WEB-INF/lps/lfc/views/LaszloView.lzs
>> M      WEB-INF/lps/lfc/views/LaszloCanvas.lzs
>> M      WEB-INF/lps/lfc/helpers/LzCommand.lzs
>> M      WEB-INF/lps/lfc/helpers/LzState.lzs
>> M      WEB-INF/lps/lfc/controllers/LzAnimatorGroup.lzs
>> M      WEB-INF/lps/lfc/data/LzReplicationManager.lzs
>> M      WEB-INF/lps/lfc/data/LzDatapointer.lzs
>> M      WEB-INF/lps/lfc/data/LzDataText.lzs
>> M      WEB-INF/lps/lfc/data/LzDataNode.lzs
>> M      WEB-INF/lps/lfc/data/LzDataRequest.lzs
>> M      WEB-INF/lps/lfc/data/LzDataElement.lzs
>> M      WEB-INF/lps/lfc/data/LzDataset.lzs
>> M      WEB-INF/lps/lfc/data/LzDatapath.lzs
>> M      WEB-INF/lps/lfc/compiler/LzFormatter.lzs
>> M      WEB-INF/lps/lfc/compiler/Class.lzs
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java
>>
>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20071102-ptw-t.tar
>>