I think 1) the behavior should be undefined. We have two ways to
determine a node has been deleted - override destroy() or listen to the
ondestroy() event. This allows 2) Destroyed objects try to ignore any
invalid calls - but it puts the burden on the developer. I don't know
what we can do other than minimize the impact on a case-by-case basis.
Most of this is fallout from the components having been developed for
AS2 where NPEs are much less serious. We'll make the new components
more robust!
On 4/21/10 4:18 PM, André Bargull wrote:
Well, this is the question we need to answer at first:
What is the expected behaviour if you call a method on a destroyed object?
There are two possibilities:
1) The behaviour is undefined, because you must not call a method on a
destroyed object, it's simply forbidden. That means the caller is
responsible for any effects.
2) Destroyed objects try to ignore any invalid calls, if possible. That
means the callee is responsible.
I've used "setStyle(..)" only for demonstration purposes, here are a few
other components and different methods. So it's not too difficult to
provoke null pointer exceptions in the components.
<canvas debug="true">
<dataset name="ds"><e/></dataset>
<grid id="mygrid"/>
<edittext id="myedit"/>
<datacombobox id="mydcmb"/>
<tree id="mytree"/>
<handler name="oninit">
var g = mygrid;
g.destroy();
g.clearSort();
var e = myedit;
e.destroy();
e.getText();
var d = mydcmb;
d.destroy();
d.getText();
var t = mytree;
t.destroy();
t.getChildIndex(null);
</handler>
</canvas>
On 4/22/2010 12:14 AM, Max Carlson wrote:
On 4/21/10 2:54 PM, André Bargull wrote:
Not approved.
1) __LZdeleted is LFC-internal, it must not be used in the components
Should we have a public flag, or make __LZdeleted public?
Otherwise, I can't see how to ignore the setStyle() call in your
testcase at LPP-8880:
c.destroy();
// setStyle() calls _applystyle() later
c.setStyle({textcolor: 0});
2) strict equality check to avoid string coercion, re-throw error if e
!=== '__LZdeleted'!
+ } catch(e) {
+ // Construct may, through many tangled webs of replication and
+ // placement, actually end up deleting us! Bail out completely.
+ if (e == '__LZdeleted') {
+ return;
+ }
+ }
Will do.
3) There's an important devnote right before the added
"throw('__LZdeleted');":
// @devnote only add to subnodes if this node is not deleted which
// may happen as a side-effect of calling determinePlacement().
// We still need to set 'immediateparent' because legacy constructors
// expect to see this property.
This shouldn't matter anymore, because the throw() should prevent any
super calls from continuing... I'll remove/update the devnote.
On 4/21/2010 11:21 PM, Max Carlson wrote:
Change 20100421-maxcarlson-v by maxcarl...@friendly on 2010-04-21
14:09:52 PDT
in /Users/maxcarlson/openlaszlo/trunk-clean
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: Update basecomponent to not set styles, construct() to
automatically halt for deleted nodes
Bugs Fixed: LPP-8880 - ERROR @lz/textlistitem.lzx?23: TypeError: Error
#1009 in swf10, LPP-8929 - Prevent construction of destroyed LzNode
subclasses
Technical Reviewer: [email protected]
QA Reviewer: ptw
Details: LzNode - Throw an exception when __LZdeleted is true in
LzNode.construct().
LzInputText, LzText, LaszloView - Remove unneeded __LZdeleted test in
construct().
textlistitem - Remove unneeded test.
basecomponent - Don't try to set styles for deleted components.
Tests: See LPP-8880,
examples/components/component_sampler.lzx?debug=true in all runtimes
Files:
M WEB-INF/lps/lfc/core/LzNode.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 lps/components/lz/textlistitem.lzx
M lps/components/base/basecomponent.lzx
Changeset:
http://svn.openlaszlo.org/openlaszlo/patches/20100421-maxcarlson-v.tar
