[Mono-winforms-list] New to the list

[Ernesto]

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