ListView and GridView with Grouping crash with AccessViolation on Drag

[lukasf]

Describe the bug

When a ListView or GridView has Grouping enabled and AllowDrop=true, then when drag+drop something into the view area where scrolling should start, it crashes with AccessViolation.

The crash occurs exactly at the point where the list/grid would start scrolling up or down when dragging near the upper or lower area of the view. StackTrace shows it is probably trying to set the scroll velocity on something that is NULL. Dragging in the center works without crash, since no scrolling is required.

Steps to reproduce the bug

Please try attached sample. Drag e.g. some File from Windows Explorer into the middle of the view. No crash. Now move it near the bottom of the list. BANG. Watch it go down.

ListViewDragCrash.zip

Expected behavior

Drag+Drop is a super important scenario. Grouping is super important too. It should be possible to use both without issues.

AccesViolation is the worst type of error. It should never ever occur.

Screenshots

No response

NuGet package version

WinUI 3 - Windows App SDK 1.4.2: 1.4.231008000 (crash has occured since I started with WinUI on 1.2.x)

Windows version

Windows 11 (22H2): Build 22621

Additional context

StackTrace:

 	Microsoft.ui.xaml.dll!CInputServices::InitializeDirectManipulationViewportValues()	Unknown
 	Microsoft.ui.xaml.dll!CInputServices::SetConstantVelocities(class CUIElement *,class CUIElement *,float,float)	Unknown
 	Microsoft.ui.xaml.dll!CUIDMContainerHandler::SetConstantVelocities(class CUIElement *,float,float)	Unknown
 	Microsoft.ui.xaml.dll!CoreImports::ManipulationHandler_SetConstantVelocities(void *,class CUIElement *,float,float)	Unknown
 	Microsoft.ui.xaml.dll!DirectUI::ScrollViewer::SetConstantVelocities(float,float)	Unknown
 	Microsoft.ui.xaml.dll!DirectUI::ListViewBase::ScrollWithVelocity(struct DirectUI::ListViewBase::PanVelocity)	Unknown
 	Microsoft.ui.xaml.dll!DirectUI::ListViewBase::StartEdgeScrollTimerTick(struct IInspectable *,struct IInspectable *)	Unknown
 	Microsoft.ui.xaml.dll!DirectUI::ClassMemberEventHandler<class DirectUI::ListViewBase,struct ABI::Microsoft::UI::Xaml::Controls::IListViewBase,struct ABI::Windows::Foundation::IEventHandler<struct IInspectable *>,struct IInspectable,struct IInspectable>::Invoke(struct IInspectable *,struct IInspectable *)	Unknown
    Microsoft.ui.xaml.dll!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,ABI::Windows::Foundation::IEventHandler<IInspectable *>,IInspectable,IInspectable>::Raise()	Unknown
    Microsoft.ui.xaml.dll!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,ABI::Windows::Foundation::IEventHandler<IInspectable *>,IInspectable,IInspectable>::UntypedRaise()	Unknown
 	Microsoft.ui.xaml.dll!CCoreServices::CLR_FireEvent()	Unknown
 	Microsoft.ui.xaml.dll!CommonBrowserHost::CLR_FireEvent()	Unknown
 	Microsoft.ui.xaml.dll!CControlBase::ScriptCallback()	Unknown
 	Microsoft.ui.xaml.dll!CXcpDispatcher::OnScriptCallback()	Unknown
 	Microsoft.ui.xaml.dll!CXcpDispatcher::OnWindowMessage()	Unknown
 	Microsoft.ui.xaml.dll!CXcpDispatcher::WindowProc()	Unknown
 	Microsoft.ui.xaml.dll!CDeferredInvoke::DispatchQueuedMessage()	Unknown
 	Microsoft.ui.xaml.dll!Microsoft::WRL::Details::DelegateArgTraits<long (__cdecl ABI::Windows::Foundation::ITypedEventHandler_impl<ABI::Windows::Foundation::Internal::AggregateType<ABI::Microsoft::UI::Dispatching::DispatcherQueueTimer *,ABI::Microsoft::UI::Dispatching::IDispatcherQueueTimer *>,IInspectable *>::*)(ABI::Microsoft::UI::Dispatching::IDispatcherQueueTimer *,IInspectable *)>::DelegateInvokeHelper<Microsoft::WRL::Implements<Microsoft::WRL::RuntimeClassFlags<2>,ABI::Windows::Foundation::ITypedEventHandler<ABI::Microsoft::UI::Dispatching::DispatcherQueueTimer *,IInspectable *>,Microsoft::WRL::FtmBase>,`CXcpDispatcher::Init'::`75'::<lambda_1> &,1,ABI::Microsoft::UI::Dispatching::IDispatcherQueueTimer *,IInspectable *>::Invoke()	Unknown

[ranjeshj]

@lukasf drag and drop/reorder in grouped listview/gridview is not supported currently. I've marked this as a feature request.

[lukasf]

Come on, this is not a feature request. This is a bug. This is base functionality that has worked in every other XAML UI framework.

[lukasf]

I have looked into the implementation and I think I might have found the issue.

Please check this line:

https://github.com/microsoft/microsoft-ui-xaml/blob/685d2bfa86d6169aa1998a7eaa2c38bfcf9f74bc/dxaml/xcp/core/input/InputServices.cpp#L6524C17-L6524C43

            IFC(pDirectManipulationService->GetSecondaryClipContentTransform(
                pViewport,
                pContent,
                pContent->GetContentType(),
                translationX,
                translationY,
                uncompressedZoomFactor,
                zoomFactorX,
                zoomFactorY));

In all other places in this method, pDirectManipulationService is checked against null, before it is accessed. So obviously it can be null when the method is called. But in this line, the interface is used directly, without any check. And I get a AccessViolation from this exact method. I do not see any other unchecked calls in this method, so I am pretty confident that this is the exact place where the crash occurs.

I guess, instead of calling the method directly, it should be checked, and if null, the translation values must be calculated manually, like in the loop above:

            if (pDirectManipulationService)
            {
                IFC(pDirectManipulationService->GetSecondaryContentTransform(
                    pViewport,
                    pContent,
                    pContent->GetContentType(),
                    translationX,
                    translationY,
                    uncompressedZoomFactor,
                    zoomFactorX,
                    zoomFactorY));
            }
            else
            {
                // Use provided values for instance for edge scrolling scenarios.
                switch (pContent->GetContentType())
                {
                case XcpDMContentTypeTopLeftHeader:
                    translationX = 0.0f;
                    translationY = 0.0f;
                    break;
                case XcpDMContentTypeTopHeader:
                    translationX = initialTranslationX;
                    translationY = 0.0f;
                    break;
                case XcpDMContentTypeLeftHeader:
                    translationX = 0.0f;
                    translationY = initialTranslationY;
                    break;
                case XcpDMContentTypeCustom:
                case XcpDMContentTypeDescendant:
                    translationX = 0.0f;
                    translationY = 0.0f;
                    break;
                }
            }

Please look into this, it is super easy to reproduce and it could be easy to solve. This is a really important scenario.

Unfortunately, it is not possible for me to compile WinUI 3, there are no build instructions whatsoever, which is a shame. A lot of the WinUI bugs could probably be solved by the community, if only we had access to complete sources and build system.

We see that you are pretty much understaffed to do all this work. You should really work towards open sourcing WinUI3, even if not all of it is translated to C++/winrt yet. A lot of devs know plain C++ just as well.

[RBrid]

Thanks much for your input @lukasf. You were right about the needed changes, and they will be part of the next WinUI3 release.
Edge-scrolling in a grouped ListViewBase will no longer hit this null-ref exception.