Bug 8366 - BindingList<T> is not raising ItemChanged event if Item implements INotifyPropertyChanged
Summary: BindingList<T> is not raising ItemChanged event if Item implements INotifyPro...
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll (show other bugs)
Version: 6.0.x
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Sebastien Pouliot
URL:
Depends on:
Blocks:
 
Reported: 2012-11-13 07:33 UTC by Michal
Modified: 2012-11-14 11:33 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Michal 2012-11-13 07:33:56 UTC
Current implementation of BindingList<T> is not complete. It is not raising ListChangedType.ItemChanged event for items implementing INotifyPropertyChanged interface when some property of such item changes.
Comment 1 Sebastien Pouliot 2012-11-13 16:08:17 UTC
Mono's unit tests for BindingList<T> works correctly. Could you provide us a test case to duplicate the issue you're seeing ? Thanks!
Comment 2 Michal 2012-11-14 03:57:27 UTC
And are Mono's unit tests complete? They are working correctly does not mean they are covering everything :)

Try this:
        private class Item : INotifyPropertyChanged
        {
            public event PropertyChangedEventHandler PropertyChanged;

            private string _name;
            public string Name
            {
                get { return _name; }
                set
                {
                    if (_name != value)
                    {
                        _name = value;
                        OnPropertyChanged("Name");
                    }
                }
            }

            private void OnPropertyChanged(string name)
            {
                var fn = this.PropertyChanged;
                if (fn != null)
                {
                    var args = new PropertyChangedEventArgs(name);
                    fn(this, args);
                }
            }
        }

        private void TestBindingList()
        {
            var list = new BindingListEx<Item>();
            list.ListChanged += (object sender, ListChangedEventArgs e) => 
            {
                Console.WriteLine(e.ListChangedType);
            };

            var item = new Item() { Name = "1" };
            list.Add(item);

            item.Name = "2";
        }

Call TestBindingList method. Current output is ItemAdded, but it should be two lines: ItemAdded and then also ItemChanged.

MonoTouch 6.0.6 which is the latest stable version MonoDevelop is offerring us.
Comment 3 Sebastien Pouliot 2012-11-14 08:11:37 UTC
> And are Mono's unit tests complete?

Unlikely. Still even if it was 100% covered by unit tests it would not mean it's bug free either.

OTOH providing a test case to us makes sure you'll get a fix for *your* bug, not a similar one we could find while hunting for it (which would require you to re-open the bug on the next MT release). Also we can turn provided test cases into new unit tests to ensure the feature does not regress in the future.
Comment 4 Michal 2012-11-14 08:19:52 UTC
I understand and thank you
Comment 5 Sebastien Pouliot 2012-11-14 09:07:39 UTC
It looks like mono/master already has the fix for this. I'll backport (to 2-10/monotouch) and test this.
Comment 6 Sebastien Pouliot 2012-11-14 11:33:16 UTC
Fixed in:
ios6: d5e0fa42b97388903d7ac99437aa5d509b9e1444
master: 3c402b9f9d4fb99cd920190f1992bb45083cd8a6

Test added:
mono/master: 0dd29ed7219f6c943cc9c979b83eb5c0b874d6e0
mono-2-10: a214478534f8d261cdcbec33f0d9107b39038945
mobile-master: f41893591b3dda508d52b1831ac6f5eae4556499