Skip to content
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

MenuFlyout crashes in Xaml Islands #3953

Closed
sylveon opened this issue Jan 15, 2021 · 14 comments
Closed

MenuFlyout crashes in Xaml Islands #3953

sylveon opened this issue Jan 15, 2021 · 14 comments
Labels
area-Islands Xaml Islands feature area-Menus duplicate This issue or pull request already exists team-Controls Issue for the Controls team

Comments

@sylveon
Copy link
Contributor

sylveon commented Jan 15, 2021

Describe the bug
When opening submenus in quick succession in MenuFlyout on Xaml Islands, there is a chance it can crash the whole program.

Steps to reproduce the bug

  1. Create a new blank WPF .NET Framework app.
  2. Add the Microsoft.Toolkit.Wpf.UI.XamlHost NuGet package.
  3. Add a project reference to the UniversalApiContract winmd from the Windows SDK.
  4. Add an app manifest to the app, and within <compatibility> add <maxversiontested Id="10.0.19041.0" />
  5. In MainWindow.xaml, write the following
    <Window x:Class="WpfApp2.MainWindow"
            xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
            xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
            xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
            xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
            xmlns:local="clr-namespace:WpfApp2" xmlns:xamlhost="clr-namespace:Microsoft.Toolkit.Wpf.UI.XamlHost;assembly=Microsoft.Toolkit.Wpf.UI.XamlHost"
            mc:Ignorable="d"
            Title="MainWindow" Height="450" Width="800">
        <Grid>
            <xamlhost:WindowsXamlHost x:Name="host" />
        </Grid>
    </Window>
  6. In MainWindow.xaml.cs, write this code
    public partial class MainWindow : Window
    {
        private Windows.UI.Xaml.Controls.MenuFlyoutSubItem MakeSubMenu(int i)
        {
            var sub = new Windows.UI.Xaml.Controls.MenuFlyoutSubItem
            {
                Text = $"Submenu {i}"
            };
    
            foreach (var j in Enumerable.Range(0, 10))
            {
                sub.Items.Add(new Windows.UI.Xaml.Controls.MenuFlyoutItem
                {
                    Text = $"Item {j}"
                });
            }
    
            return sub;
        }
    
        public MainWindow()
        {
            InitializeComponent();
    
            var rect = new Windows.UI.Xaml.Shapes.Rectangle
            {
                Fill = new Windows.UI.Xaml.Media.SolidColorBrush(Windows.UI.Colors.Red),
                Width = 50,
                Height = 50
            };
    
            var flyout = new Windows.UI.Xaml.Controls.MenuFlyout();
            foreach (var i in Enumerable.Range(0, 10))
            {
                flyout.Items.Add(MakeSubMenu(i));
            }
    
            rect.ContextFlyout = flyout;
            host.Child = rect;
        }
    }
  7. Launch the app.
  8. Right-click on the red square
  9. Open different sub-menus in quick succession.

Expected behavior
The app does not crash

Screenshots

Version Info

NuGet package version:
[Microsoft.Toolkit.Wpf.UI.XamlHost 6.1.2]

Windows app type:

UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT

Additional context
This also happens in my C++-based XAML islands app, so this is not an issue specific to the WCT WPF host, it just makes reproduction easier.

Here's a video recording of the issue: https://imgur.com/pHTPAB2

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jan 15, 2021
@ranjeshj ranjeshj added area-Islands Xaml Islands feature area-Menus needs-author-feedback Asked author to supply more information. needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jan 16, 2021
@ranjeshj
Copy link
Contributor

Can you please attach a repro app ?

@sylveon
Copy link
Contributor Author

sylveon commented Jan 16, 2021

Sure, here you go: WpfApp2.zip

@ghost ghost added needs-triage Issue needs to be triaged by the area owners and removed needs-author-feedback Asked author to supply more information. labels Jan 16, 2021
@sylveon
Copy link
Contributor Author

sylveon commented Jan 17, 2021

After playing a bit with it, it seems this is triggered when a sub-menu is opened while the animation for opening the previous sub-menu hasn't finished playing. I've had a few crashes not triggered like that, but since the crash is the exact same the fix would probably resolve them too.

@zadjii-msft
Copy link
Member

FYI this looks related to #3529 and #3530, and is probably the root cause for microsoft/terminal#8238

@StephenLPeters
Copy link
Contributor

@Austin-Lamb and @JesseCol for the XamlIslands perspective

@StephenLPeters StephenLPeters removed the needs-triage Issue needs to be triaged by the area owners label Jan 20, 2021
@lyahdav
Copy link

lyahdav commented Apr 26, 2021

I found a workaround: #3529 (comment)

@sylveon
Copy link
Contributor Author

sylveon commented Jun 29, 2021

Seems to be fixed in Windows 11, would appreciate the fix to be backported to Windows 10.

@zadjii-msft
Copy link
Member

@Austin-Lamb Any way you could confirm that this is fixed in Win11? the Terminal team would greatly appreciate a backport for that fix to Win10 (if that's even possible). Feel free to ping me on a mail thread if needed.

@sylveon
Copy link
Contributor Author

sylveon commented Jul 6, 2021

At least from my testing, I could reliably get it to crash in 10 and can't no matter how hard it try on 11. I wonder if this fix was prompted by the taskbar dogfooding Xaml islands.

@zadjii-msft
Copy link
Member

(fyi Austin is out for the week, maybe @JesseCol can answer in his place)

@JesseCol
Copy link

Hi -- sorry, this bug doesn't ring a bell to me. If it's related to the animation, @codendone might recognize it.

@lyahdav
Copy link

lyahdav commented Jul 13, 2021

I believe @gegao18 was the one who fixed this.

@zadjii-msft
Copy link
Member

As an update - I think this was MSFT:31809589 internally, which is definitely fixed in Windows 11. I'd reckon that this thread, #3529, and #3530 can all get combined into a single tracking issue, though I'll leave it to the WinUI folks to decide how they want to run their own repo 😛

@krschau
Copy link
Contributor

krschau commented Jul 6, 2022

Duplicate of #3529

@krschau krschau marked this as a duplicate of #3529 Jul 6, 2022
@krschau krschau closed this as completed Jul 6, 2022
@krschau krschau removed the needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) label Jul 6, 2022
@krschau krschau added the duplicate This issue or pull request already exists label Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Islands Xaml Islands feature area-Menus duplicate This issue or pull request already exists team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

7 participants