Krypton-Suite / Standard-Toolkit

An update to Component factory's krypton toolkit to support .NET Framework 4.6.2 - 4.8.1 to .NET 8 - 9
BSD 3-Clause "New" or "Revised" License
408 stars 62 forks source link

[Bug]: CheckedListBox CheckedIndices NullRef #1314

Closed iwarp closed 8 months ago

iwarp commented 9 months ago

Describe the bug Just converting an existing app using Krypton to the standard-toolkit, when looping thru the CheckedIndices of a CheckedListBox I get a null ref issue in multiple places across my code base.

StackTrace: at Krypton.Toolkit.KryptonCheckedListBox.InternalCheckedListBox.InnerArrayIndexOfIdentifier(Object identifier, Int32 stateMask) in C:\git\Krypton-Standard-Toolkit\Source\Krypton Components\Krypton.Toolkit\Controls Toolkit\KryptonCheckedListBox.cs:line 759 at Krypton.Toolkit.KryptonCheckedListBox.CheckedIndexCollection.InnerArrayIndexOfIdentifier(Object identifier, Int32 stateMask) in C:\git\Krypton-Standard-Toolkit\Source\Krypton Components\Krypton.Toolkit\Controls Toolkit\KryptonCheckedListBox.cs:line 153 at Krypton.Toolkit.KryptonCheckedListBox.CheckedIndexCollection.get_Item(Int32 index) in C:\git\Krypton-Standard-Toolkit\Source\Krypton Components\Krypton.Toolkit\Controls Toolkit\KryptonCheckedListBox.cs:line 127 at Krypton.Toolkit.KryptonCheckedListBox.CheckedIndexCollection.CopyTo(Array array, Int32 index) in C:\git\Krypton-Standard-Toolkit\Source\Krypton Components\Krypton.Toolkit\Controls Toolkit\KryptonCheckedListBox.cs:line 70 at Krypton.Toolkit.KryptonCheckedListBox.CheckedIndexCollection.GetEnumerator() in C:\git\Krypton-Standard-Toolkit\Source\Krypton Components\Krypton.Toolkit\Controls Toolkit\KryptonCheckedListBox.cs:line 81 at KryptonTest.Form1.kryptonButton1_Click(Object sender, EventArgs e) in

To Reproduce Simple form with CheckedListBox with Designer added items.

foreach(var item in this.kryptonCheckedListBox1.CheckedIndices) { MessageBox.Show(item.ToString()); }

Expected behavior Not to crash and return the selected indexes back to my foreach

Desktop (please complete the following information):

Possible Resolution image When this method is executed its dynamically loading the method for "IndexOfIdentifier" however there is no method named that on a ItemArray.

List of Method on the ItemArray InnerArray.GetType().GetMethods() {System.Reflection.MethodInfo[25]} [0]: {Boolean GetState(Int32, Int32)} [1]: {Void SetState(Int32, Int32, Boolean)} [2]: {Int32 GetCount(Int32)} [3]: {Int32 get_Version()} [4]: {System.Object Add(System.Object)} [5]: {Void Clear()} [6]: {Int32 CreateMask()} [7]: {Int32 GetActualIndex(Int32, Int32)} [8]: {Int32 get_Count()} [9]: {System.Collections.IEnumerator GetEnumerator(Int32)} [10]: {System.Collections.IEnumerator GetEnumerator(Int32, Boolean)} [11]: {System.Object GetItem(Int32, Int32)} [12]: {Int32 IndexOf(System.Object, Int32)} [13]: {Void Insert(Int32, System.Object)} [14]: {Void InsertEntry(Int32, Entry)} [15]: {Void Remove(System.Object)} [16]: {Void RemoveAt(Int32)} [17]: {Void SetItem(Int32, System.Object)} [18]: {Entry GetEntry(System.Object)} [19]: {Int32 BinarySearch(Entry)} [20]: {Void Sort()} [21]: {System.Type GetType()} [22]: {System.String ToString()} [23]: {Boolean Equals(System.Object)} [24]: {Int32 GetHashCode()}

This problem can be fixed if the Method name is switched to "IndexOf" instead. https://github.com/Krypton-Suite/Standard-Toolkit/blob/4c38c8492a2290291047e2f668fa17efe8417931/Source/Krypton%20Components/Krypton.Toolkit/Controls%20Toolkit/KryptonCheckedListBox.cs#L756

Smurf-IV commented 8 months ago

I'm confused... There is already a redirect for the IndexOf in the API above that...

image

Why is that not being used ?

Notes: IndexOfIdentifier does exist for .net image

You used the API list from List of Method on the ItemArray which is not a ListBox.

@iwarp Can you add an example of your "Designer added items" please ?

iwarp commented 8 months ago

@Smurf-IV simple demo of the bug here https://github.com/iwarp/KryptonBugDemo check some items and hit the button.

Designer Added Items is super simple. kryptonCheckedListBox1.Items.AddRange(new object[] { "Test1", "Test2", "Test3", "Test4", "Test5" });

Smurf-IV commented 8 months ago

Argg.. It worked in <= .net48 image

So it must be a netcore core "Improvement" thing

Smurf-IV commented 8 months ago

Top is the type from net48 Bottom is the type in netCore image

Smurf-IV commented 8 months ago

It was changed during the preview of netCore 6 !! https://github.com/dotnet/winforms/commit/1f4a593a6de32e75ff0f5fa97d35191c1facbc93

Because the ItemArray was drastically chnaged: https://github.com/dotnet/winforms/commit/1f4a593a6de32e75ff0f5fa97d35191c1facbc93#diff-fc2945ee2669ddb08500f48c7ae59bf19b266a27ffba223da89958e97924695a

Smurf-IV commented 8 months ago

@Wagnerp Should this be retrofitted into V80 as it is a regression issue ?

PWagner1 commented 8 months ago

@Wagnerp Should this be retrofitted into V80 as it is a regression issue ?

@Smurf-IV How 'big' are the changes, as it may mess up the merging flow?

Smurf-IV commented 8 months ago

Should be the same as the ones in this PR.

PWagner1 commented 8 months ago

Should be the same as the ones in this PR.

@Smurf-IV Yes, but it should not be pushed to master, as it would mean that master would be 'ahead' of everything else.

Smurf-IV commented 8 months ago

Just theses changes, as a separate PR (Not a merge), will indicate in GitHub that it has a different code base (i.e. 1 change with how ever many files) Then when the V90 is merged later in the year, it will just replace over as usual.

iwarp commented 8 months ago

Thanks for getting to the bottom of this, is this now in nightly for me to test out? 90.24.3.64-alpha?

PWagner1 commented 8 months ago

Just theses changes, as a separate PR (Not a merge), will indicate in GitHub that it has a different code base (i.e. 1 change with how ever many files) Then when the V90 is merged later in the year, it will just replace over as usual.

Going to put it in a separate branch called master-patch-2

PWagner1 commented 8 months ago

Thanks for getting to the bottom of this, is this now in nightly for me to test out? 90.24.3.64-alpha?

@iwarp Yes, but fixes have been applied to 80.24.3.64, if you need a stable version.