[Mono-winforms-list] New to the list

[Jonathan Pobst]

Hey!

I think the simpler way to fix that is to check the Value in the Maximum 
setter, and if the Value is greater than the Maximum, set the Value to 
the Maximum.  This appears to be what MS does as well.

Change made and test added in r85647.

This fixes your provided test case, if there is something else it 
doesn't fix, please let me know.  :)

Jon


Ernesto wrote:
> Jonathan Pobst wrote:
>> Hey Ernesto!
>>
>> The changes look harmless, but I have a feeling they are simply hiding 
>> a bigger problem, ie: what is trying to set a scroll position bigger 
>> than the maximum and why.  The ListView maintainer (calberto) thinks 
>> the same about the ListView one.  Do you have a test case or code 
>> snippet for each of them that reproduces the crash so we can dig a bit 
>> a deeper?
>>
> Heh... yes, I have that feeling too. I just didn't want to made 
> design-level changes (and even if I did want, I didn't dare since this 
> is my first day with the winforms source code).
> However, I can explain how I see the problem so you guys decide if 
> deeper changes are necesary.
> 
> The ListView problem:
> 
> 1) When a new SubItem is added, the ListViewSubItem constructor sets the 
> SubItem text.
> 2) Text.set in the SubItem Text property calls Invalidate(Bounds).
> 3) Bounds.get calls GetBounds() and this calls GetItemLocation().
> 4) GetItemLocation() uses the items_location[] array.
> 
> Now, items_location[] is set to an initial lenght of [16] by the 
> ListView constructor, and extended by AdjustItemsPositionArray if needed.
> AdjustItemsPositionArray is only called by CalculateListView, which is 
> called only before a redraw.
> 
> So, if you add a 17th item before a redraw, you get an index out of 
> bounds at ListView.GetItemLocation().
> 
> I think the problem here is that Invalidate() ends up usign 
> items_location[], which is only calculated when redrawing. That doesn't 
> seem to make sense sometimes, since Invalidate() happens _before_ the 
> redraw.
> 
> Anyway, my patch only ensures the items_location[] array grows when the 
> collection grows. It doesn't seem to care if items_location[] has valid 
> information because we are not redrawing, just invalidating.
> 
> 
> The ScrollBar problem is much simpler.
> 
>            vscrollbar.LargeChange = 0;
>            vscrollbar.Maximum = 100;
>            vscrollbar.Value = 20;
>            vscrollbar.Maximum = 0;   // BONGGGGG!!!
> 
> It only happens if LargeChange == 0, because of this code in UpdatePos() 
> in ScrollBar.cs:
> 
>    if (newPos > maximum + 1 - large_change)
>        pos = maximum + 1 - large_change;
>    else
>        pos = newPos;
> 
> For a large_change value of 5
>    if (20 > 0 + 1 - 5)
>        pos = -4;
>    else
>        pos = newPos;
> 
> and then
> 
>    if (pos < minimum)
>        pos = minimum;
> 
> will reset pos to 0 (or wharever minimum is).
> 
> But for large_change value of 0,
> 
>    if (20 > 0 + 1 - 0)
>        pos = maximum + 1 - large_change;
>    else
>        pos = newPos;
> 
> pos == 1, which is more than Maximum, so a SetValue (pos) a few lines 
> below will fail.
> 
> LargeChange values of 0 should be accepted. On the other hand "if 
> (newPos > maximum + 1 - large_change) pos = maximum + 1 - large_change;" 
> sounds correct to me, but may result in negative values that must be 
> corrected.
> 
> In short, I tought it was a good idea to give pos a last check before 
> calling SetValue(pos).
> However, if you think that's not correct, and that I should find another 
> solution to fix UpdatePos(), I will. I just want the problem solved, you 
> can tell me how.
> 
> Regards,
> Ernesto
> 
> _______________________________________________
> Mono-winforms-list maillist  -  Mono-winforms-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-winforms-list
> 
>