You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flex.apache.org by Erik de Bruin <er...@ixsoftware.nl> on 2014/06/05 10:01:40 UTC
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
-1 (binding)
I'm vetoing this commit because it is causing the following (sets of)
Mustella tests to fail:
- components/MenuBar/Halo/Styles/MenuBar_MenuStyles
- components/Menu/Halo/Properties/Menu_Properties
- components/Menu/Halo/Styles/Menu_Styles
- components/Menu/Spark/Properties/Menu_Properties_spark
- components/Tree/Properties/Tree_Properties
- components/Tree/Properties/Tree_Properties_spark
- components/Tree/Properties/Tree_PropertiesDragDrop
- components/Tree/Properties/Tree_PropertiesDragDrop_spark
- gumbo/components/DataGrid/Properties/DataGrid_dataProvider_test001
- gumbo/components/MXItemRenderer/integration/MXTIR_Integration_main
(aharui: "I looked at MXItemRenderer and it appears that the test is
expecting an updateComplete when an XML node is added to the collection.
Does this no longer happen because the parent is not notified?")
It has been three weeks without a working solution (or 'fix' of the tests)
and we need the successful Mustella runs because we're looking to get out a
new release, so: please revert - or fix - this commit and all related ones.
Thanks,
EdB
On Tue, May 20, 2014 at 1:19 AM, <qu...@apache.org> wrote:
> Repository: flex-sdk
> Updated Branches:
> refs/heads/FLEX-34283 [created] e780eaa81
>
>
> Now XMLListCollection behaves closer to be expected -- does not update
> parent from updated XMLList. I guess it should be debated wether it
> should...
>
>
> Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
> Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/e780eaa8
> Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/e780eaa8
> Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/e780eaa8
>
> Branch: refs/heads/FLEX-34283
> Commit: e780eaa814b5ac76e9dbfc88208f88022b57c5f4
> Parents: 79073bf
> Author: Nick Kwiatkowski <nk...@msu.edu>
> Authored: Mon May 19 19:18:15 2014 -0400
> Committer: Nick Kwiatkowski <nk...@msu.edu>
> Committed: Mon May 19 19:18:15 2014 -0400
>
> ----------------------------------------------------------------------
> .../src/mx/collections/XMLListAdapter.as | 32 ++++++++++++++------
> 1 file changed, 23 insertions(+), 9 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/e780eaa8/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> index 17d2173..7c29798 100644
> --- a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> +++ b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> @@ -248,16 +248,30 @@ public class XMLListAdapter extends EventDispatcher
> implements IList, IXMLNotifi
>
> 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];
> - }
> + var newSource:XMLList = new XMLList();
> +
> + for (var i:uint = 0; i <= (length); i++)
> + {
> + if (i < index)
> + {
> + newSource[i] = source[i];
> + }
> + else if (i == index)
> + {
> + newSource[i] = item;
> + }
> + else if (i > index)
> + {
> + newSource[i] = source[i-1];
> + }
> + }
> +
> + source = newSource;
> }
> -
> - source[index] = item;
> + else
> + {
> + source[index] = item;
> + }
>
> startTrackUpdates(item, seedUID + uidCounter.toString());
> uidCounter++;
>
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now XMLListCollection
behaves closer to be expected -- does not update parent from updated XMLList.
I guess it should be debated wether it should...
Posted by Stephan Plath <fl...@gmx.net>.
I don't have some insight into the issues of XMLListCollection, but
after reading this discussion I think it might be reasonable to create a
new class like "XMLListCollection2" with the improved or mutated behaviour.
Stephan
Am 09.06.2014 13:32, schrieb Nicholas Kwiatkowski:
> Have we been getting cleans passes of Mustella yet? I've got a few changes
> I have been working on in another branch that I want to merge in if we are..
>
> -Nick
>
>
> On Fri, Jun 6, 2014 at 8:01 PM, Alex Harui <ah...@adobe.com> wrote:
>
>> I think I understand. IMO, it still feels worth it to me to maintain the
>> old behavior, even if it had issues, and find a way to switch to new
>> behavior and make the new behavior the default.
>>
>> -Alex
>>
>> On 6/6/14 4:55 PM, "Michael A. Labriola" <la...@digitalprimates.net>
>> wrote:
>>
>>>> I haven't looked at the failing tests, but could it be true that those
>>>> tests are not using XMLListCollection directly? They may wrap the XML
>>>> in an XMLListCollection or just pass XML directly into the control where
>>>> it gets wrapped, and then they manipulate the XML? I think there are
>>>> lots of people doing that sort of thing so we should not break them.
>>>
>>>> You could be right that nobody really uses XMLListCollection today to
>>>> add/remove items. If you want to gamble that that is the case, I'm
>>>> willing to go along with that, but we should at minimum find a way that
>>>> folks doing the XML manipulation directly don't get broken.
>>>
>>> Alex,
>>>
>>> What I was saying is that anyone who is using the XML directly would have
>>> trouble also using the XMLListCollection with it in more than a basic
>>> way. XMLListCollection actually changes the XML source in unpredictable
>>> ways (re-parenting nodes, etc.)
>>>
>>> So, it's not that I don't think people are using it. I was just saying
>>> that anyone who uses it in a more than a casual manner is either working
>>> around those issues or (as we used to) is making a copy of the XML before
>>> XMLListCollection is allowed to touch it so it doesn't screw things up
>>> too badly.
>>>
>>> I don't know the right answer here. Honestly, I think the issue is that
>>> the ListCollection views are trying to overlay a structure that doesn't
>>> actually make sense onto XML. So, I would wager, people are mostly using
>>> it to wrap XML so that it can be viewed in things like Lists/DataGrids.
>>> They _may_ be adding and removing some nodes in a limited capacity, but
>>> most likely they are playing around with the actual XML still since that
>>> is the only way they could do e4x expressions etc.
>>>
>>> Mike
>>>
>>
>>
>
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Erik de Bruin <er...@ixsoftware.nl>.
Nope, the unstable runs cause the entire Jenkins/Java/Apache stack to
crumble every few days. I was away from computer for a few days, but there
were no runs successful and I just logged in to the VM only to see a huge
Java RTE dialog up. Rebooting as we speak, let's see what happens then.
EdB
On Mon, Jun 9, 2014 at 1:32 PM, Nicholas Kwiatkowski <ni...@spoon.as>
wrote:
> Have we been getting cleans passes of Mustella yet? I've got a few changes
> I have been working on in another branch that I want to merge in if we
> are..
>
> -Nick
>
>
> On Fri, Jun 6, 2014 at 8:01 PM, Alex Harui <ah...@adobe.com> wrote:
>
> > I think I understand. IMO, it still feels worth it to me to maintain the
> > old behavior, even if it had issues, and find a way to switch to new
> > behavior and make the new behavior the default.
> >
> > -Alex
> >
> > On 6/6/14 4:55 PM, "Michael A. Labriola" <la...@digitalprimates.net>
> > wrote:
> >
> > >>I haven't looked at the failing tests, but could it be true that those
> > >>tests are not using XMLListCollection directly? They may wrap the XML
> > >>in an XMLListCollection or just pass XML directly into the control
> where
> > >>it gets wrapped, and then they manipulate the XML? I think there are
> > >>lots of people doing that sort of thing so we should not break them.
> > >
> > >>You could be right that nobody really uses XMLListCollection today to
> > >>add/remove items. If you want to gamble that that is the case, I'm
> > >>willing to go along with that, but we should at minimum find a way that
> > >>folks doing the XML manipulation directly don't get broken.
> > >
> > >Alex,
> > >
> > >What I was saying is that anyone who is using the XML directly would
> have
> > >trouble also using the XMLListCollection with it in more than a basic
> > >way. XMLListCollection actually changes the XML source in unpredictable
> > >ways (re-parenting nodes, etc.)
> > >
> > >So, it's not that I don't think people are using it. I was just saying
> > >that anyone who uses it in a more than a casual manner is either working
> > >around those issues or (as we used to) is making a copy of the XML
> before
> > >XMLListCollection is allowed to touch it so it doesn't screw things up
> > >too badly.
> > >
> > >I don't know the right answer here. Honestly, I think the issue is that
> > >the ListCollection views are trying to overlay a structure that doesn't
> > >actually make sense onto XML. So, I would wager, people are mostly using
> > >it to wrap XML so that it can be viewed in things like Lists/DataGrids.
> > >They _may_ be adding and removing some nodes in a limited capacity, but
> > >most likely they are playing around with the actual XML still since that
> > >is the only way they could do e4x expressions etc.
> > >
> > >Mike
> > >
> >
> >
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Nicholas Kwiatkowski <ni...@spoon.as>.
Have we been getting cleans passes of Mustella yet? I've got a few changes
I have been working on in another branch that I want to merge in if we are..
-Nick
On Fri, Jun 6, 2014 at 8:01 PM, Alex Harui <ah...@adobe.com> wrote:
> I think I understand. IMO, it still feels worth it to me to maintain the
> old behavior, even if it had issues, and find a way to switch to new
> behavior and make the new behavior the default.
>
> -Alex
>
> On 6/6/14 4:55 PM, "Michael A. Labriola" <la...@digitalprimates.net>
> wrote:
>
> >>I haven't looked at the failing tests, but could it be true that those
> >>tests are not using XMLListCollection directly? They may wrap the XML
> >>in an XMLListCollection or just pass XML directly into the control where
> >>it gets wrapped, and then they manipulate the XML? I think there are
> >>lots of people doing that sort of thing so we should not break them.
> >
> >>You could be right that nobody really uses XMLListCollection today to
> >>add/remove items. If you want to gamble that that is the case, I'm
> >>willing to go along with that, but we should at minimum find a way that
> >>folks doing the XML manipulation directly don't get broken.
> >
> >Alex,
> >
> >What I was saying is that anyone who is using the XML directly would have
> >trouble also using the XMLListCollection with it in more than a basic
> >way. XMLListCollection actually changes the XML source in unpredictable
> >ways (re-parenting nodes, etc.)
> >
> >So, it's not that I don't think people are using it. I was just saying
> >that anyone who uses it in a more than a casual manner is either working
> >around those issues or (as we used to) is making a copy of the XML before
> >XMLListCollection is allowed to touch it so it doesn't screw things up
> >too badly.
> >
> >I don't know the right answer here. Honestly, I think the issue is that
> >the ListCollection views are trying to overlay a structure that doesn't
> >actually make sense onto XML. So, I would wager, people are mostly using
> >it to wrap XML so that it can be viewed in things like Lists/DataGrids.
> >They _may_ be adding and removing some nodes in a limited capacity, but
> >most likely they are playing around with the actual XML still since that
> >is the only way they could do e4x expressions etc.
> >
> >Mike
> >
>
>
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Alex Harui <ah...@adobe.com>.
I think I understand. IMO, it still feels worth it to me to maintain the
old behavior, even if it had issues, and find a way to switch to new
behavior and make the new behavior the default.
-Alex
On 6/6/14 4:55 PM, "Michael A. Labriola" <la...@digitalprimates.net>
wrote:
>>I haven't looked at the failing tests, but could it be true that those
>>tests are not using XMLListCollection directly? They may wrap the XML
>>in an XMLListCollection or just pass XML directly into the control where
>>it gets wrapped, and then they manipulate the XML? I think there are
>>lots of people doing that sort of thing so we should not break them.
>
>>You could be right that nobody really uses XMLListCollection today to
>>add/remove items. If you want to gamble that that is the case, I'm
>>willing to go along with that, but we should at minimum find a way that
>>folks doing the XML manipulation directly don't get broken.
>
>Alex,
>
>What I was saying is that anyone who is using the XML directly would have
>trouble also using the XMLListCollection with it in more than a basic
>way. XMLListCollection actually changes the XML source in unpredictable
>ways (re-parenting nodes, etc.)
>
>So, it's not that I don't think people are using it. I was just saying
>that anyone who uses it in a more than a casual manner is either working
>around those issues or (as we used to) is making a copy of the XML before
>XMLListCollection is allowed to touch it so it doesn't screw things up
>too badly.
>
>I don't know the right answer here. Honestly, I think the issue is that
>the ListCollection views are trying to overlay a structure that doesn't
>actually make sense onto XML. So, I would wager, people are mostly using
>it to wrap XML so that it can be viewed in things like Lists/DataGrids.
>They _may_ be adding and removing some nodes in a limited capacity, but
>most likely they are playing around with the actual XML still since that
>is the only way they could do e4x expressions etc.
>
>Mike
>
RE: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by "Michael A. Labriola" <la...@digitalprimates.net>.
>I haven't looked at the failing tests, but could it be true that those tests are not using XMLListCollection directly? They may wrap the XML in an XMLListCollection or just pass XML directly into the control where it gets wrapped, and then they manipulate the XML? I think there are lots of people doing that sort of thing so we should not break them.
>You could be right that nobody really uses XMLListCollection today to add/remove items. If you want to gamble that that is the case, I'm willing to go along with that, but we should at minimum find a way that folks doing the XML manipulation directly don't get broken.
Alex,
What I was saying is that anyone who is using the XML directly would have trouble also using the XMLListCollection with it in more than a basic way. XMLListCollection actually changes the XML source in unpredictable ways (re-parenting nodes, etc.)
So, it's not that I don't think people are using it. I was just saying that anyone who uses it in a more than a casual manner is either working around those issues or (as we used to) is making a copy of the XML before XMLListCollection is allowed to touch it so it doesn't screw things up too badly.
I don't know the right answer here. Honestly, I think the issue is that the ListCollection views are trying to overlay a structure that doesn't actually make sense onto XML. So, I would wager, people are mostly using it to wrap XML so that it can be viewed in things like Lists/DataGrids. They _may_ be adding and removing some nodes in a limited capacity, but most likely they are playing around with the actual XML still since that is the only way they could do e4x expressions etc.
Mike
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Alex Harui <ah...@adobe.com>.
I haven't looked at the failing tests, but could it be true that those
tests are not using XMLListCollection directly? They may wrap the XML in
an XMLListCollection or just pass XML directly into the control where it
gets wrapped, and then they manipulate the XML? I think there are lots of
people doing that sort of thing so we should not break them.
You could be right that nobody really uses XMLListCollection today to
add/remove items. If you want to gamble that that is the case, I'm
willing to go along with that, but we should at minimum find a way that
folks doing the XML manipulation directly don't get broken.
-Alex
On 6/6/14 3:19 PM, "Michael A. Labriola" <la...@digitalprimates.net>
wrote:
>>Are you saying that using XMLListCollection APIs to modify the source
>>data has always been broken, or that you didn't like the behavior in how
>>it >set up the parent? If there is some debate on what the behavior
>>should be then there probably needs to be a flag to control that
>>behavior. If it >was just plain broken, is there no way to make it work
>>in both cases, like set a flag on the XMLListAdapter to tell it that the
>>collection is about to >do mutation and therefore not react to some of
>>the notifications?
>
>
>XMLListCollection mangled, destroyed and in other ways violated the XML
>structures the nodes originated from. It worked in the simplest test
>cases only and, in practice, caused more problems than it solved. The
>test cases present for the framework never noticed the side effects that
>this code was having outside of the XMLListCollection itself, so, while
>the tests made things appear to be working well, the rest of the
>application was being trashed in many cases.
>
>Nick's fixes are trying to walk a line to fix XMLListCollection in a way
>that it is still a viable collection but at the same time not cause
>massive damage to other data through its use. It might not even be
>possible but that's what he was doing. The problem with all of this is
>that it's not an array, its XML and the rules around how XML nodes work
>are completely different. This entire class and implementation is now and
>always was broken. We were never able to use it in production apps
>because of the side effects and had our own collection implementations
>when we needed XML to work in controls.
>
>I do understand wanting to keep things working for existing users,
>however, the problem is that the way it was working was actually causing
>problems that they just haven't yet discovered.
>
>Mike
>
RE: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by "Michael A. Labriola" <la...@digitalprimates.net>.
>Are you saying that using XMLListCollection APIs to modify the source data has always been broken, or that you didn't like the behavior in how it >set up the parent? If there is some debate on what the behavior should be then there probably needs to be a flag to control that behavior. If it >was just plain broken, is there no way to make it work in both cases, like set a flag on the XMLListAdapter to tell it that the collection is about to >do mutation and therefore not react to some of the notifications?
XMLListCollection mangled, destroyed and in other ways violated the XML structures the nodes originated from. It worked in the simplest test cases only and, in practice, caused more problems than it solved. The test cases present for the framework never noticed the side effects that this code was having outside of the XMLListCollection itself, so, while the tests made things appear to be working well, the rest of the application was being trashed in many cases.
Nick's fixes are trying to walk a line to fix XMLListCollection in a way that it is still a viable collection but at the same time not cause massive damage to other data through its use. It might not even be possible but that's what he was doing. The problem with all of this is that it's not an array, its XML and the rules around how XML nodes work are completely different. This entire class and implementation is now and always was broken. We were never able to use it in production apps because of the side effects and had our own collection implementations when we needed XML to work in controls.
I do understand wanting to keep things working for existing users, however, the problem is that the way it was working was actually causing problems that they just haven't yet discovered.
Mike
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Alex Harui <ah...@adobe.com>.
On 6/6/14 1:36 PM, "Nicholas Kwiatkowski" <ni...@spoon.as> wrote:
>Well, the tests that are failing are because those items are directly
>manipulating the data underneath the the collection (this would be akin to
>manipulating the source array in an array collection, by putting watches
>on
>them, and changing the order without using the overlying collection.
>While
>that is technically legal, it's very bad practice.
I suppose it is bad practice, but IIRC, lots of people took advantage of
the fact that it could work since XML modifications were watchable, so we
tried to make it work. If Array was similarly capable of such
notifications, it isn't clear we would have created ArrayCollection in the
first place.
>Putting a flag for the
>old method is fine, but if people modify the source data using the
>collection instead of the source directly then it is broken.
Are you saying that using XMLListCollection APIs to modify the source data
has always been broken, or that you didn't like the behavior in how it set
up the parent? If there is some debate on what the behavior should be
then there probably needs to be a flag to control that behavior. If it
was just plain broken, is there no way to make it work in both cases, like
set a flag on the XMLListAdapter to tell it that the collection is about
to do mutation and therefore not react to some of the notifications?
-Alex
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Nicholas Kwiatkowski <ni...@spoon.as>.
Well, the tests that are failing are because those items are directly
manipulating the data underneath the the collection (this would be akin to
manipulating the source array in an array collection, by putting watches on
them, and changing the order without using the overlying collection. While
that is technically legal, it's very bad practice. Putting a flag for the
old method is fine, but if people modify the source data using the
collection instead of the source directly then it is broken. The reason
why we have failed tests is because even the SDK writers were working
around this bug (in mx:Menu and mx:Tree). It looks like it's been working
this way since 3.x at the least.
I'm still trying to get my own local copy of Mustella compiling right.
Sometime last month it stopped running fully on git bash (it's not
compiling tests). No idea on that one, but I'm trying to install it on my
Mac right now since I know that is what most of you use (so it should
work).
-Nick
On Fri, Jun 6, 2014 at 4:11 PM, Alex Harui <ah...@adobe.com> wrote:
> I still think you should add a flag that preserves old behavior. Mustella
> is telling you that there is a potential that folks are relying on old
> behavior. It is fine to default to new behavior and have the mustella
> tests set the flag for old behavior (and make copies of those tests with
> results expecting new behavior).
>
> Or provide a clear argument that nobody in their right mind could ever
> want the old behavior.
>
> -Alex
>
> On 6/6/14 11:55 AM, "Nicholas Kwiatkowski" <ni...@spoon.as> wrote:
>
> >I've reverted this back to the original version. I'll continue work in a
> >separate branch, but I need some help in testing that branch before I
> >merge
> >again. This fixes a pretty nasty bug that we should really take care of.
> >
> >-Nick
> >
> >
> >On Thu, Jun 5, 2014 at 4:01 AM, Erik de Bruin <er...@ixsoftware.nl> wrote:
> >
> >> -1 (binding)
> >>
> >> I'm vetoing this commit because it is causing the following (sets of)
> >> Mustella tests to fail:
> >>
> >> - components/MenuBar/Halo/Styles/MenuBar_MenuStyles
> >> - components/Menu/Halo/Properties/Menu_Properties
> >> - components/Menu/Halo/Styles/Menu_Styles
> >> - components/Menu/Spark/Properties/Menu_Properties_spark
> >> - components/Tree/Properties/Tree_Properties
> >> - components/Tree/Properties/Tree_Properties_spark
> >> - components/Tree/Properties/Tree_PropertiesDragDrop
> >> - components/Tree/Properties/Tree_PropertiesDragDrop_spark
> >> - gumbo/components/DataGrid/Properties/DataGrid_dataProvider_test001
> >> - gumbo/components/MXItemRenderer/integration/MXTIR_Integration_main
> >>
> >> (aharui: "I looked at MXItemRenderer and it appears that the test is
> >> expecting an updateComplete when an XML node is added to the collection.
> >> Does this no longer happen because the parent is not notified?")
> >>
> >> It has been three weeks without a working solution (or 'fix' of the
> >>tests)
> >> and we need the successful Mustella runs because we're looking to get
> >>out a
> >> new release, so: please revert - or fix - this commit and all related
> >>ones.
> >>
> >> Thanks,
> >>
> >> EdB
> >>
> >>
> >>
> >> On Tue, May 20, 2014 at 1:19 AM, <qu...@apache.org> wrote:
> >>
> >> > Repository: flex-sdk
> >> > Updated Branches:
> >> > refs/heads/FLEX-34283 [created] e780eaa81
> >> >
> >> >
> >> > Now XMLListCollection behaves closer to be expected -- does not update
> >> > parent from updated XMLList. I guess it should be debated wether it
> >> > should...
> >> >
> >> >
> >> > Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
> >> > Commit:
> >>http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/e780eaa8
> >> > Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/e780eaa8
> >> > Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/e780eaa8
> >> >
> >> > Branch: refs/heads/FLEX-34283
> >> > Commit: e780eaa814b5ac76e9dbfc88208f88022b57c5f4
> >> > Parents: 79073bf
> >> > Author: Nick Kwiatkowski <nk...@msu.edu>
> >> > Authored: Mon May 19 19:18:15 2014 -0400
> >> > Committer: Nick Kwiatkowski <nk...@msu.edu>
> >> > Committed: Mon May 19 19:18:15 2014 -0400
> >> >
> >> > ----------------------------------------------------------------------
> >> > .../src/mx/collections/XMLListAdapter.as | 32
> >> ++++++++++++++------
> >> > 1 file changed, 23 insertions(+), 9 deletions(-)
> >> > ----------------------------------------------------------------------
> >> >
> >> >
> >> >
> >> >
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/e780eaa8/frameworks/
> >>projects/framework/src/mx/collections/XMLListAdapter.as
> >> > ----------------------------------------------------------------------
> >> > diff --git
> >> > a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> >> > b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> >> > index 17d2173..7c29798 100644
> >> > ---
> >>a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> >> > +++
> >>b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> >> > @@ -248,16 +248,30 @@ public class XMLListAdapter extends
> >>EventDispatcher
> >> > implements IList, IXMLNotifi
> >> >
> >> > 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];
> >> > - }
> >> > + var newSource:XMLList = new XMLList();
> >> > +
> >> > + for (var i:uint = 0; i <= (length); i++)
> >> > + {
> >> > + if (i < index)
> >> > + {
> >> > + newSource[i] = source[i];
> >> > + }
> >> > + else if (i == index)
> >> > + {
> >> > + newSource[i] = item;
> >> > + }
> >> > + else if (i > index)
> >> > + {
> >> > + newSource[i] = source[i-1];
> >> > + }
> >> > + }
> >> > +
> >> > + source = newSource;
> >> > }
> >> > -
> >> > - source[index] = item;
> >> > + else
> >> > + {
> >> > + source[index] = item;
> >> > + }
> >> >
> >> > startTrackUpdates(item, seedUID + uidCounter.toString());
> >> > uidCounter++;
> >> >
> >> >
> >>
> >>
> >> --
> >> Ix Multimedia Software
> >>
> >> Jan Luykenstraat 27
> >> 3521 VB Utrecht
> >>
> >> T. 06-51952295
> >> I. www.ixsoftware.nl
> >>
>
>
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Alex Harui <ah...@adobe.com>.
I still think you should add a flag that preserves old behavior. Mustella
is telling you that there is a potential that folks are relying on old
behavior. It is fine to default to new behavior and have the mustella
tests set the flag for old behavior (and make copies of those tests with
results expecting new behavior).
Or provide a clear argument that nobody in their right mind could ever
want the old behavior.
-Alex
On 6/6/14 11:55 AM, "Nicholas Kwiatkowski" <ni...@spoon.as> wrote:
>I've reverted this back to the original version. I'll continue work in a
>separate branch, but I need some help in testing that branch before I
>merge
>again. This fixes a pretty nasty bug that we should really take care of.
>
>-Nick
>
>
>On Thu, Jun 5, 2014 at 4:01 AM, Erik de Bruin <er...@ixsoftware.nl> wrote:
>
>> -1 (binding)
>>
>> I'm vetoing this commit because it is causing the following (sets of)
>> Mustella tests to fail:
>>
>> - components/MenuBar/Halo/Styles/MenuBar_MenuStyles
>> - components/Menu/Halo/Properties/Menu_Properties
>> - components/Menu/Halo/Styles/Menu_Styles
>> - components/Menu/Spark/Properties/Menu_Properties_spark
>> - components/Tree/Properties/Tree_Properties
>> - components/Tree/Properties/Tree_Properties_spark
>> - components/Tree/Properties/Tree_PropertiesDragDrop
>> - components/Tree/Properties/Tree_PropertiesDragDrop_spark
>> - gumbo/components/DataGrid/Properties/DataGrid_dataProvider_test001
>> - gumbo/components/MXItemRenderer/integration/MXTIR_Integration_main
>>
>> (aharui: "I looked at MXItemRenderer and it appears that the test is
>> expecting an updateComplete when an XML node is added to the collection.
>> Does this no longer happen because the parent is not notified?")
>>
>> It has been three weeks without a working solution (or 'fix' of the
>>tests)
>> and we need the successful Mustella runs because we're looking to get
>>out a
>> new release, so: please revert - or fix - this commit and all related
>>ones.
>>
>> Thanks,
>>
>> EdB
>>
>>
>>
>> On Tue, May 20, 2014 at 1:19 AM, <qu...@apache.org> wrote:
>>
>> > Repository: flex-sdk
>> > Updated Branches:
>> > refs/heads/FLEX-34283 [created] e780eaa81
>> >
>> >
>> > Now XMLListCollection behaves closer to be expected -- does not update
>> > parent from updated XMLList. I guess it should be debated wether it
>> > should...
>> >
>> >
>> > Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
>> > Commit:
>>http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/e780eaa8
>> > Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/e780eaa8
>> > Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/e780eaa8
>> >
>> > Branch: refs/heads/FLEX-34283
>> > Commit: e780eaa814b5ac76e9dbfc88208f88022b57c5f4
>> > Parents: 79073bf
>> > Author: Nick Kwiatkowski <nk...@msu.edu>
>> > Authored: Mon May 19 19:18:15 2014 -0400
>> > Committer: Nick Kwiatkowski <nk...@msu.edu>
>> > Committed: Mon May 19 19:18:15 2014 -0400
>> >
>> > ----------------------------------------------------------------------
>> > .../src/mx/collections/XMLListAdapter.as | 32
>> ++++++++++++++------
>> > 1 file changed, 23 insertions(+), 9 deletions(-)
>> > ----------------------------------------------------------------------
>> >
>> >
>> >
>> >
>>
>>http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/e780eaa8/frameworks/
>>projects/framework/src/mx/collections/XMLListAdapter.as
>> > ----------------------------------------------------------------------
>> > diff --git
>> > a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
>> > b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
>> > index 17d2173..7c29798 100644
>> > ---
>>a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
>> > +++
>>b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
>> > @@ -248,16 +248,30 @@ public class XMLListAdapter extends
>>EventDispatcher
>> > implements IList, IXMLNotifi
>> >
>> > 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];
>> > - }
>> > + var newSource:XMLList = new XMLList();
>> > +
>> > + for (var i:uint = 0; i <= (length); i++)
>> > + {
>> > + if (i < index)
>> > + {
>> > + newSource[i] = source[i];
>> > + }
>> > + else if (i == index)
>> > + {
>> > + newSource[i] = item;
>> > + }
>> > + else if (i > index)
>> > + {
>> > + newSource[i] = source[i-1];
>> > + }
>> > + }
>> > +
>> > + source = newSource;
>> > }
>> > -
>> > - source[index] = item;
>> > + else
>> > + {
>> > + source[index] = item;
>> > + }
>> >
>> > startTrackUpdates(item, seedUID + uidCounter.toString());
>> > uidCounter++;
>> >
>> >
>>
>>
>> --
>> Ix Multimedia Software
>>
>> Jan Luykenstraat 27
>> 3521 VB Utrecht
>>
>> T. 06-51952295
>> I. www.ixsoftware.nl
>>
Re: git commit: [flex-sdk] [refs/heads/FLEX-34283] - Now
XMLListCollection behaves closer to be expected -- does not update parent
from updated XMLList. I guess it should be debated wether it should...
Posted by Nicholas Kwiatkowski <ni...@spoon.as>.
I've reverted this back to the original version. I'll continue work in a
separate branch, but I need some help in testing that branch before I merge
again. This fixes a pretty nasty bug that we should really take care of.
-Nick
On Thu, Jun 5, 2014 at 4:01 AM, Erik de Bruin <er...@ixsoftware.nl> wrote:
> -1 (binding)
>
> I'm vetoing this commit because it is causing the following (sets of)
> Mustella tests to fail:
>
> - components/MenuBar/Halo/Styles/MenuBar_MenuStyles
> - components/Menu/Halo/Properties/Menu_Properties
> - components/Menu/Halo/Styles/Menu_Styles
> - components/Menu/Spark/Properties/Menu_Properties_spark
> - components/Tree/Properties/Tree_Properties
> - components/Tree/Properties/Tree_Properties_spark
> - components/Tree/Properties/Tree_PropertiesDragDrop
> - components/Tree/Properties/Tree_PropertiesDragDrop_spark
> - gumbo/components/DataGrid/Properties/DataGrid_dataProvider_test001
> - gumbo/components/MXItemRenderer/integration/MXTIR_Integration_main
>
> (aharui: "I looked at MXItemRenderer and it appears that the test is
> expecting an updateComplete when an XML node is added to the collection.
> Does this no longer happen because the parent is not notified?")
>
> It has been three weeks without a working solution (or 'fix' of the tests)
> and we need the successful Mustella runs because we're looking to get out a
> new release, so: please revert - or fix - this commit and all related ones.
>
> Thanks,
>
> EdB
>
>
>
> On Tue, May 20, 2014 at 1:19 AM, <qu...@apache.org> wrote:
>
> > Repository: flex-sdk
> > Updated Branches:
> > refs/heads/FLEX-34283 [created] e780eaa81
> >
> >
> > Now XMLListCollection behaves closer to be expected -- does not update
> > parent from updated XMLList. I guess it should be debated wether it
> > should...
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/e780eaa8
> > Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/e780eaa8
> > Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/e780eaa8
> >
> > Branch: refs/heads/FLEX-34283
> > Commit: e780eaa814b5ac76e9dbfc88208f88022b57c5f4
> > Parents: 79073bf
> > Author: Nick Kwiatkowski <nk...@msu.edu>
> > Authored: Mon May 19 19:18:15 2014 -0400
> > Committer: Nick Kwiatkowski <nk...@msu.edu>
> > Committed: Mon May 19 19:18:15 2014 -0400
> >
> > ----------------------------------------------------------------------
> > .../src/mx/collections/XMLListAdapter.as | 32
> ++++++++++++++------
> > 1 file changed, 23 insertions(+), 9 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/e780eaa8/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> > ----------------------------------------------------------------------
> > diff --git
> > a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> > b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> > index 17d2173..7c29798 100644
> > --- a/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> > +++ b/frameworks/projects/framework/src/mx/collections/XMLListAdapter.as
> > @@ -248,16 +248,30 @@ public class XMLListAdapter extends EventDispatcher
> > implements IList, IXMLNotifi
> >
> > 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];
> > - }
> > + var newSource:XMLList = new XMLList();
> > +
> > + for (var i:uint = 0; i <= (length); i++)
> > + {
> > + if (i < index)
> > + {
> > + newSource[i] = source[i];
> > + }
> > + else if (i == index)
> > + {
> > + newSource[i] = item;
> > + }
> > + else if (i > index)
> > + {
> > + newSource[i] = source[i-1];
> > + }
> > + }
> > +
> > + source = newSource;
> > }
> > -
> > - source[index] = item;
> > + else
> > + {
> > + source[index] = item;
> > + }
> >
> > startTrackUpdates(item, seedUID + uidCounter.toString());
> > uidCounter++;
> >
> >
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
>