-
Notifications
You must be signed in to change notification settings - Fork 697
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
VisualStateManager throws generic unhelpful exception when a Setter contains an Invalid Element Name #5575
Comments
Tried moving the VSM to the very top before the Grid.RowDefinitions as well just in case, but same issue. |
I even simplified the VSM to this and still received the crash: <VisualStateManager.VisualStateGroups>
<VisualStateGroup x:Name="AutoSuggestGroup">
<VisualState x:Name="AutoSuggestBoxVisible" />
<VisualState x:Name="AutoSuggestBoxCollapsed">
<VisualState.Setters>
<Setter Target="AutoSuggestArea.Visibility" Value="Collapsed" />
<Setter Target="TopPaneAutoSuggestArea.Visibility" Value="Collapsed" />
</VisualState.Setters>
</VisualState>
</VisualStateGroup>
<VisualStateGroup x:Name="BackButtonGroup">
<VisualState x:Name="BackButtonVisible">
<VisualState.Setters>
<Setter Target="BackButtonPlaceholderOnTopNav.Width" Value="{ThemeResource NavigationBackButtonWidth}" />
</VisualState.Setters>
</VisualState>
<VisualState x:Name="BackButtonCollapsed" />
</VisualStateGroup>
</VisualStateManager.VisualStateGroups> |
Been fiddling and got a slightly different stack, loaded the Microsoft.UI.Xaml symbols and saw this:
At least this stack is getting me farther as it's pointing to this function:
Though I don't see that function calling a visual state: microsoft-ui-xaml/dev/NavigationView/NavigationView.cpp Lines 4518 to 4534 in 5e815a2
The other line called out is here:
Which at least seems more useful. I tried removing that Visual State and then it was able to load. I can still add an AutoSuggestBox, so I'm not sure what this Visual State actually does... 🤔 Still no idea why this particular VisualState was causing a crash. Maybe because we didn't have a Yup removing that line of the VisualState caused it not to crash. So something about having a pointer in the Setter of a Visual State to an undefined element is causing this to crash internally with a super unhelpful error message compared to saying something like:
|
Yeah, Setter's will compile even if their targets are not defined, but when they are executed they will have a runtime crash. It would be great if the error message that was raised was more reasonable though. @RealTommyKlein could this error message be improved from the xaml compiler side? @michael-hawker sounds like you found the cause of your issue, maybe we can rename this issue to be about making the error message for this scenario better? |
@StephenLPeters done! This seems like something the Compiler or an Analyzer could be able to catch before runtime. But at least at runtime the error message should be more than 'Exception'! 🙂 |
We raise a stowed exception that contains a helpful error message, but breaking on those may not be turned on by default in VS. There's also slightly different behavior in this (and similar) error case depending on whether or not a debugger attached for historical reasons that we really should get rid of. I agree that overall it's a pretty meh experience. |
Not sure if this was addressed any with the error handling improvements, would have to check latest. Bumping based on #8638 |
Describe the bug
Trying to override the
NavigationView
style is causing a crash in OnApplyTemplate when we move the VisualStateManager to its proper location.Steps to reproduce the bug
Expected behavior
Not crash or coherent error message.
Version Info
NuGet package version: WinUI 2.6.1
Windows app type:
Additional context
Our template is about 80% the same as the inbox NavigationView, we have another Grid around the whole template, which I realized we hadn't moved our VSM up into (as the VSM wasn't firing). When moving the VSM up into the top-level Grid, it crashes. If you remove the VSM, it doesn't crash (but also doesn't behave as expected).
Stack Trace:
The text was updated successfully, but these errors were encountered: