-
Notifications
You must be signed in to change notification settings - Fork 706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
App crashes when rearranging tabs in TabView where TabItemsSource is used #2457
Comments
This is also reproducible on build 18363 (v1909). |
Thank you for reporting this issue! This looks to be a regression in the TabView control, so I'm going to move this over to the WinUI repo. |
I was able to get this under the debugger finally, and it seems that we a TabItemTemplate which binds to the content is an issues. If we remove the binding of content, the app does not crash. The error message:
|
Thanks @chingucoding. You can go ahead with investigating a fix for this. |
Can you track back and see when it stopped working by trying each of the pre-releases builds ? I assume this is not an issue in the platform's listview since we have not made any recent changes there. |
I've tested the XAML Controls Gallery sample with the commit it was added with, and it still crashed. The sample was added 7 months ago and used WinUI 2.2, I am trying to find out which commit exactly could have caused this (the first versions of drag and drop did in fact not crash), but building the solution in those old states is difficult at times. |
Might be easier to do in a blank app than controls gallery. Thanks! |
The earliest version where the crash occurs is 2.2.190822001-prerelease. I will look into what changed between the release before that and that prerelease. |
So I've finally found out what causes the crash. Turns out using ItemsStackPanel as the ListView's panel, is causing the crash. Unfortunately removing the ItemsStackPanel (e.g. switching to StackPanel or removing the setter for the What would be the best direction moving forward with this? @ranjeshj @teaP Edit: #1158 introduced the use of ItemsStackPanel. |
@RBrid can you please help investigate this crash ? |
@dpaulino, @chingucoding, do you know how easily I should be running into this crash? I have been trying ~100 times with the latest master bits and have not hit the problem. Thanks! |
@RBrid it crashes instantly for me in the xaml controls gallery app. Makes sure you scroll down to the demo for TabViewItemsSource. I just tried it now and still crashes instantly when I attempt a tab rearrange |
The repro steps I noted at the top of the issue reliably reproduces the crash. Note that I'm using the app that is deployed to the store. And I'm not sure if master branch is ahead of the store build. |
@RBrid - I am able to repo 100% as well using the debug version of the Gallery from GitHub. It sounds like this also crashes on the store version. Perhaps it's OS related? I'm on 19635.mn_release.200522. |
In case it helps, I'm on Windows 10 2004 Build 19041.264 |
I could not repro whether I used the GitHub master version or the Microsoft Store version, on my machine which runs Windows 10.0.18362.836 (files from 05/12/2020). I then installed the Store version on another machine that runs Windows 10.0.18362.1 (files from 03/18/2019). Bam! Instant and consistent crash. So, it looks like, somehow we fixed it in the OS between the two versions above. Although 19635.mn_release.200522 came after 10.0.18362.836, right? This is puzzling. |
@RBrid can you record a video of your repro attempt and share it here? I'm curious if there's anything different about what you do and what I have done |
FYI I can also repro on a third machine with the Store version. It runs OS 10.0.18362.329 (from 01/10/2020). What is special about the machine which runs Windows 10.0.18362.836 on which I cannot repro? Maybe it is that I use a 'Remote Desktop Connection' to try to repro on that machine since it is far far away from me... |
I think my assumption above was right. If you turn off system animations on your machine, the bug will no longer repro. I think Remote Desktop Connection turns off animations by default. I can now repro on my Windows 10.0.18362.836 machine after turning on system animations on it. |
That's fascinating. I'll try to test that as well when I get back to my desktop. |
I created PR microsoft/WinUI-Gallery#457 for the Xaml Controls Gallery repo. A few details: Some of the stack trace:
Without animations, the old listview item container gets put into the recycling pool early enough that it is picked up to contain the dropped content.
When animations run, the old container has not been put into the recycling pool and a brand new container is being instantiated and populated through the DataContext. The UIElement content (that is still hosted in the old container) is being parented into the newly created container – this causes the “element already parented” error. The fix is to either not use UIElement in the DataContext - here it's a Frame, or remove animations from the inner ListView's ItemContainerTransitions. See the PR sent out for the Xaml Controls Gallery app. |
Thanks @RBrid! A few questions:
Honestly, none of these options seem great. Using TabItemsSource seems like a common scenario. @michael-hawker, how does the WCT support this case? |
I tested this on my system as well running the Store Version of XAML and it crashes. |
This does not appear to be a regression. The TabView started to use an ItemsStackPanel in its inner ListView thus the ModernCollectionBasePanel involvement in the stack above. We did not run into this old bug before the TabView used a ModernCollectionBasePanel virtualizing panel. |
Hmm, so it sounds like the change to use ModernCollectionBasePanel introduced the issue? What was it using before? Should we revert? |
The problem is that, if we don't use the ItemsStackPanel, the items don't render properly (at least it did when I tried). |
Before the TabView used a regular StackPanel. I have been experimenting with a ListView alone and as I said it repros there too. With a StackPanel, I am seeing this report in VS when re-ordering items data-bound to a UIElement: "Exception thrown: 'System.Runtime.InteropServices.COMException' in MUXControlsTestApp.exe But the app does not crash and the dropped item loses its content. With an ItemsStackPanel, the same error causes a crash. So none of the two panels work properly. |
FYI, the animations
were added to the TabView's inner ListView by Tammy on 07/17/2019, in TabView.xaml. |
@stmoy the Toolkit version uses StackPanel and not ItemsStackPanel, so that's probably how we've avoided this issue. |
Gotcha, thanks all. @RBrid, it sounds like this scenario is broken with either a Since neither panel works, it seems like we'll need to do two things:
I'm still not sure how to do Step 2. What will we recommend people do? I see three options to update the guidance:
Are there other options? Thoughts on the best recommendation? Disabling the animations is probably the best approach that maintains functionality, but I'm not sure if the implementation in the PR is the most straightforward/discoverable way to do it. |
@stmoy @RBrid, that's an interesting note about the item losing content. I seem to remember someone mentioning an issue with TabView in the Toolkit and datacontext disappearing. I don't seem to be able to find an issue for it though, and I don't remember if it as related to drag and drop, but if so that could have been the underlying issue there as well. |
I am currently looking into Issue #2455 - it may be related. The scenario is a bit different because there the TabItemsSource is an observable collection of TabViewItem, as opposed to an observable collection of data objects. |
Can you see if removing ReorderThemeTransition from the transition collection avoids the crash ? If that works, perhaps that is a more palatable workaround since the crash is in the OS XAML code and we cannot fix that until winui3 at the earliest. |
As you can see in my fix microsoft/WinUI-Gallery#457, I am removing the AddDeleteThemeTransition/ContentThemeTransition animations. Removing ReorderThemeTransition was the first thing I had tried and it did not help unfortunately. |
Describe the bug
If you try to rearrange tabs in the TabView which uses describes how to use
TabItemsSource
, the app crashes.Steps to reproduce the bug
Steps to reproduce the behavior:
A TabView bound to a collection of MyData objects
Expected behavior
app does not crash
Actual behavior
app crashes after dropping the tab
Screenshots
Version Info
Gallery version (found on Settings page):
1.2.15.0
Windows 10 version:
Device form factor:
Additional context
The text was updated successfully, but these errors were encountered: