XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

Was looking at this bug:
https://issues.apache.org/jira/browse/FLEX-33913

And it looks like it's a Flash Player issue. Does anyone know if there a know Flash Player issue with for each loops and XMLList?

Thanks,
Justin

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

Actually it's even more broken that i thought.

trace("Length before adding: " + mainXML.item.length());
var some:XMLListCollection = new XMLListCollection();
var length:int = mainXML.item.length();
for  (var i:int = 0; i <  length; i++)	{ 
	trace("adding " + mainXML.item[i].@id);
	some.addItem(mainXML.item[i]); 
} 				
trace("Length after adding: " + mainXML.item.length());
var some:XMLListCollection = new XMLListCollection();
for each (var o:Object in mainXML.item)	{ 
	trace("adding " + o.@id);
	some.addItem(o); 
} 
trace("Length after adding: " + mainXML.item.length());

Produces:

Length before adding: 3
adding 1
adding 2
adding 2
Length after adding: 5
adding 1
adding 2
adding 2
adding 2
adding 3
Length after adding: 9

So I guess the question is it a Flex SDK issue or a FP issue? Can we work around it?

Justin

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

>Thanks for the contribution - perhaps we should make you a committer? :-)

Let's not rush into anything :)

Mike

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

Perfect that fixes the issue and all the XMLListCollection tests pass.

Thanks for the contribution - perhaps we should make you a committer? :-)

Justin

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

Thanks for that (slightly) slower and working is preferable to not working :-)

I'll apply the patch and run the tests and see if any other changes need to be made.

Thanks,
Justin

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

Here is my proposed fix in XMLListAdapter:: addItemAt() :

...

if (length > 0) {
  var localLength:uint = source.length();

  //Adjust all indexes by 1
  for ( var i:uint = localLength; i>index; i-- ) {
    source[i] = source[ i - 1 ];
  }
}
 
source[ index ] = item;

...

We may be able to optimize this too. For example, we could check if the incoming node already has a parent. If it does, then we need to do this work. If it does not, we could use the older method which is faster.

Mike

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

>mainXML.item.length() which shows the same problem.

Actually, It shows a worse problem... and infinite loop, cause length() keeps growing so the loop will never terminate. If you save the invariant to a local variable first, obviously it works better... but the XMLList still grows.

Mike

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

>I will write it all up tonight if possible. I can provide example of each step and what's going on.

In XMLListAdapter, addItemAt() we see this after the first addItem()

source[0] = length > 0 ? item + source[0] : item;

this effectively sets source[0] to the unchanged item... and the parent doesn't change. so, remember, item still has the original XMLList as a parent

So, at this point, source[0].parent() is the original XML list... not source. After this first item is added, the XMLListCollection has 1 item in it and the mainXML still has 3 items. All is well.

... the loop continues

After the second addItemAt() we get into XMLListAdapter again, however, since there is already an item in the list we are funneled into a different condition:

source[index - 1] += item;

So, we take source of index-1, which is the first node... and we add item to it... but the first node's parent is the _original_ XMLList (mainXML). 

After this line executes, the original XMLlist now has 4 items, including two copies of item #2.

We, of course, do this all again, and now the mainXML has 5 items including two copies of item #2 and two copies of item #3.

...
Now, it works in the index cases.... well cause it doesn't and it's a false reading:

for (var i:int=0; i < mainXML.item.length; i++)
{ some.addItem(mainXML.item[i]); }

So, the mainXML.item.length evaluates to 0. It never runs the loop and hence, the original XMLList is unchanged. That's because length is a function on an XMLList, you meant: mainXML.item.length() which shows the same problem.

Mike

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

>Thanks for all that. The code isn't adding anything to the original list , it's looping though the nodes of a list then and adding them to a new collection.
>So why does using a loop index work, but using in doesn't?

I will write it all up tonight if possible. I can provide example of each step and what's going on.

As far as your question, I think we can work around it with a potential performance hit. When I posed these questions before the Flash Player team said it was intended behavior (the parenting) so my personal opinion is it's a FP issue we can work around...but that's me

Mike

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

> Let me take some of that back, I was describing a different issue. I think this might be related to parenting issues within XMLList. It can produce very unexpected behavior. In this case, its adding a node to the original parent. 

Thanks for all that. The code isn't adding anything to the original list , it's looping though the nodes of a list then and adding them to a new collection.

var some:XMLListCollection = new XMLListCollection();
for each (var o:Object in  mainXML.item) { 
	some.addItem(o); 
} 

In this case mainXML is modified when it shouldn't be and ends up with 5 elements instead of it's original 3. It certainly possible that when adding "o" it's parent is changed but not sure why that would result in duplicate items in mainXML.

And this code works correctly even though it is calling addItem with the same objects.

var some:XMLListCollection = new XMLListCollection();
for  (var i:int =0; i <  mainXML.item.length; i++) { 
	some.addItem(mainXML.items[i]); 
}

So why does using a loop index work, but using in doesn't?

Thanks,
Justin

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

>And the same code is called when using an index loop (which works as expected) so far as I can tell it's a "side effect" of using "in".

Let me take some of that back, I was describing a different issue. I think this might be related to parenting issues within XMLList. It can produce very unexpected behavior. In this case, its adding a node to the original parent. 

Since items added to an XMLList are passed "by reference," its state, including its parent node/place in an existing XML structure, are persisted. Hence, for example, when you create an XMLList out of the elements() of an existing XML object, they will still have a parent value pointing to that original XML object.

However, XMLLists also automatically parent new nodes that aren't parented. If you are *adding* a new node to an XMLList, here is the behavior:
*	If you add an XML node to an empty XMLList, it will not be (re)parented.
*	If you add an XML node that already has a parent to an XMLList, its parent property will be unaffected.
*	If you add an unparented XML node to an XMLList...
o	If added at index 0, it will take the parent of the subsequent XML node (ie: current index 0, which will become index 1).
o	If added at index > 0, it will take the parent of the previous XML node (ie: index - 1).

This entails a few things. Most noticeably, if your first node in an XMLList is unparented and you add an unparented node at index 0, it will remain unparented (because it takes the parent of the subsequent node, which is null).

The automatic parenting also occurs when *replacing* an existing node in an XMLList with a different one. Here is the behavior:
*	If you replace a node (original node) in an XMLList with another node (replacement node), the replacement node will also replace the original node in its parent XML structure as well.
*	If you replace a node (original node) in an XMLList with another node (replacement node), the replacement node's parent() function will return the value that was returned from the original node.
*	If you replace a node (original node) in an XMLList with another node (replacement node), the replacement node will still be a child of its previous parent.

The significant part of this is that, although an XML node can only have one parent() reference, it may exist in n children sets. Ie: The exact same XML node might be in numerous XML hierarchies, but will only reference the last parent assigned to it.

Now take all of that into account. When you add the new node to the XMLListCollection, it looks up the node's parent to determine where it should be added.... and hence we get here.

Mike

RE: XMLListCollection issue

["Michael A. Labriola" <la...@digitalprimates.net>]

>And the same code is called when using an index loop (which works as expected) so far as I can tell it's a "side effect" of using "in".

It is. The issue exists with XMLList as well and also Dictionary (although less reproducible with the latter) 

In all cases using the in while modifying properties of the core object have known issues. I can try to find the original bugs I filed on these in the Flash Player many years ago. The Dictionary one is particularly rough as indexed approaches aren't really viable.

Mike

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

>> Did it go into a whole bunch of XMLListAdapter code?
> Only addItemAt. and that as far as I can see seem to be doing the right thing.

And the same code is called when using an index loop (which works as expected) so far as I can tell it's a "side effect" of using "in".

Thanks,
Justin

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi

> Did it go into a whole bunch of XMLListAdapter code?
Only addItemAt. and that as far as I can see seem to be doing the right thing.

Justin

Re: XMLListCollection issue

[Alex Harui <ah...@adobe.com>]

Did it go into a whole bunch of XMLListAdapter code?  That's where I
suspect the problem is.  Or did you do a simple test of adding somehow to
an XMLList without XMLListCollection being involved?

On 1/2/14 11:07 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> Did you step into addItem?
>Yep. The first item is added correctly as is the second but after the
>second is added the original list changes.
>
>>  Honestly, I'm not sure addItem of parented items is supposed to work.
>Could see why that might be an issue, but I would expect the first list
>to have less items not more in that case.
>
>Changing to use a for index loop rather than a for each loop fixes the
>issue which to me indicates it a Flash Player issue related.
>
>The items are in the same order for a for each and the index loop and in
>both cases it loops 3 times.
>
>Thanks,
>Justin

Re: XMLListCollection issue

[Justin Mclean <ju...@classsoftware.com>]

Hi,

> Did you step into addItem?  
Yep. The first item is added correctly as is the second but after the second is added the original list changes.

>  Honestly, I'm not sure addItem of parented items is supposed to work.
Could see why that might be an issue, but I would expect the first list to have less items not more in that case.

Changing to use a for index loop rather than a for each loop fixes the issue which to me indicates it a Flash Player issue related.

The items are in the same order for a for each and the index loop and in both cases it loops 3 times.

Thanks,
Justin

Re: XMLListCollection issue

[Alex Harui <ah...@adobe.com>]

Did you step into addItem?  IIRC, there is a crapload of Flex code that
runs that could be at fault.  Honestly, I'm not sure addItem of parented
items is supposed to work.

-Alex

On 1/2/14 10:35 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>Was looking at this bug:
>https://issues.apache.org/jira/browse/FLEX-33913
>
>And it looks like it's a Flash Player issue. Does anyone know if there a
>know Flash Player issue with for each loops and XMLList?
>
>Thanks,
>Justin