Slot parameter problem in UWP DataGrid

[federgraph]

Bug report and fix suggestion.

Current behavior

In DataGridRows.cs, in method GetExactSlotElementHeight, the slot parameter can somtimes be -1, then the assert will fail, and the application will crash.

private double GetExactSlotElementHeight(int slot)
{
  Debug.Assert(slot >= 0, "Expected positive slot.");
  //...
}

Expected behavior

Don't call the method when slot == -1. The fix that I am running with is inside of method internal bool ScrollSlotIntoView(int slot, bool scrolledHorizontally):

firstFullSlot = this.DisplayData.LastScrollingSlot; //line 691

//---begin of new code
if (firstFullSlot == -1)
{
    // Assertion(slot >= 0) would otherwise crash the app
    // when calling GetExactSlotElementHeight
    return true;
}
//---end of new code

double rowHeight = GetExactSlotElementHeight(this.DisplayData.LastScrollingSlot);

Minimal reproduction of the problem with instructions

The situation when the problem shows:

• vertical scroll bar is NOT shown (few items in the list)
• DataGrid is sorted by a column
• a cell edit has just been committed
• and a new ItemsSource instance (containing Items in new order) is assigned in code.

Environment

Nuget Package(s): 
  Microsoft.Toolkit.Uwp.UI.Controls.DataGrid
Package Version(s): 
   v5.0.0

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [x ] October 2018 Update (17763)
- [ ] Insider Build (build number: )

App min and target version:
- [ ] Fall Creators Update (16299)
- [x ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] Insider Build (xxxxx)

Device form factor:
- [x ] Desktop (only tested on desktop)
- [ ] Mobile
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [x ] 2017 (version: Community)
- [ ] 2017 Preview (version: )

[federgraph]

I am attaching a MainPage.xaml.cs test file (zipped) for a minimal test project.

MainPage.xaml.cs.zip

Steps

1) New empty uwp project 2) reference Microsoft.Toolkit.Uwp.UI.Controls.DataGrid 3) In Mainpage.xaml add x:Name="hg" to Grid 4) update MainPage.xaml.cs from attached file 5) run app 6) Edit cell in R1 column, last row: ok 7) Edit cell R1 column, 1st or 2nd row: crash

Notes:

1) xaml looks like this

<Grid x:Name="hg"></Grid>

2) Edit any editable cell, does not matter. 2) Problem shows if edited row is NOT last row.

[RBrid]

@federgraph, thanks for the detailed bug report.

[windowstoolkitbot]

This issue seems inactive. It will automatically be closed in 14 days if there is no activity.

[ghost]

Thanks federgraph for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

[ghost]

Thanks federgraph for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

[ghost]

Thanks federgraph for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

[federgraph]

I retested today with the latest code from main, and I think the issue is still valid.

I can reproduce the problem when I take the naive approach and reassign to ItemsSource. In my real code I found a much better way - which avoids assigning a new ItemSource, but the test case project should not crash when someone does something similar.

The comment above links to the exact location of my "quick fix":

                if (firstFullSlot == -1)
                {
                    // Assertion(slot >= 0) would otherwise crash the app when calling GetExactSlotElementHeight below
                    Debug.WriteLine("bingo");
                    return true;
                }

It will probably be difficult to find the root source of the problem, so why not take the quick fix if there is no risk?

[ghost]

Thanks federgraph for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks