Winui2 should have tests which ensure that code behind does not require template parts.

[MartinZikmund]

Was NavigationView crashes if PaneTitleOnTopPane is not in the control template

Describe the bug

If a custom template is used for NavigationView and the TextBlock with x:Name="PaneTitleOnTopPane" is not present, the control fails to load.

Steps to reproduce the bug

Create custom template and remove PaneTitleOnTopPane.

Expected behavior

Should not crash

Screenshots

Version Info

NuGet package version: 2.6.1

Windows app type:

UWP	Win32
Yes

Windows 10 version	Saw the problem?
Insider Build (22200)	Yes
October 2020 Update (19042)
May 2020 Update (19041)
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

[mrlacey]

I now have two bigger questions for the project off the back of this:

1. Is there a way to make all controls more resilient to possible issues when retemplating?
2. How can all existing controls be tested so that they can cope when any (all?) named part(s?) are excluded when retemplated?

[marcelwgn]

Is there a way to make all controls more resilient to possible issues when retemplating?

Yup, controls do that in a lot of areas, however it's something developers need to think about. That's also because something like this bug happen because we forgot a null check.

How can all existing controls be tested so that they can cope when any (all?) named part(s?) are excluded when retemplated?

I think unless we write a lot of unit tests for this by hand, it will stay hard. Ideally of course, we would test every control without different templates with varying degrees of parts existing.

[StephenLPeters]

We do our best to not be reliant on template parts being present, this particular example slipped through the cracks. I think adding a test that applies an empty template to all of our controls wouldn't be that hard and would likely catch a lot of these issues. @ranjeshj what do you think? @MartinZikmund or @chingucoding interested in looking into that?

[mrlacey]

Is there a way to make all controls more resilient to possible issues when retemplating?

Yup, controls do that in a lot of areas, however it's something developers need to think about. That's also because something like this bug happen because we forgot a null check.

It sounds like you're saying "This is hard, but developers need to do better."
That's not an encouraging approach for a platform to take.
When ANY bug is found it is always appropriate to ask if things can be made more resilient to help avoid issues like this in the future, and is an opportunity to make the development experience better for all developers and avoid other apps crashing in the future.
There are lots of potential ways things here could be improved both for the controls within this codebase and in other apps built on top of WinUI3:

• Compile-time checks for named elements?
• Compile-time checks that all named elements in the original templates are also included in the new ones.
• Static analysis of the code-base to identity possible missing null checks.
• Change the way the custom templates are applied (in the call to OnApplyTemplate?) so that it can catch (or wrap?) any failures here so developers have a better indication of the cause of a crash.
• more/other options...
  They may not all be possible, practical, or appropriate, but it's definitely worth exploring what can be done.

---

When a bug is found, it's also appropriate to ask if there are any other similar issues elsewhere that haven't been found yet...

How can all existing controls be tested so that they can cope when any (all?) named part(s?) are excluded when retemplated?

I think unless we write a lot of unit tests for this by hand, it will stay hard. Ideally of course, we would test every control without different templates with varying degrees of parts existing.

Such a check wouldn't need to be and entirely manual process. If looking to do this, I'd write a script to walk through all current controls with a template and then iterated through each template to find named items and then create versions of the template without each named element. It's a bit of work but not overly complicated.

[marcelwgn]

Is there a way to make all controls more resilient to possible issues when retemplating?

Yup, controls do that in a lot of areas, however it's something developers need to think about. That's also because something like this bug happen because we forgot a null check.

It sounds like you're saying "This is hard, but developers need to do better."
That's not an encouraging approach for a platform to take.
When ANY bug is found it is always appropriate to ask if things can be made more resilient to help avoid issues like this in the future, and is an opportunity to make the development experience better for all developers and avoid other apps crashing in the future.

Not sure how it suggested that it would be hard, I was just trying to say "Yes we can do this, we just need to think about it and not forget to add a null check". And yes it is valid to ask how things can be made more resilient, and I also proposed some way we could possibly catch this more easily.

There are lots of potential ways things here could be improved both for the controls within this codebase and in other apps built on top of WinUI3:

• Compile-time checks for named elements?
• Compile-time checks that all named elements in the original templates are also included in the new ones.
• Static analysis of the code-base to identity possible missing null checks.
• Change the way the custom templates are applied (in the call to OnApplyTemplate?) so that it can catch (or wrap?) any failures here so developers have a better indication of the cause of a crash.
• more/other options...
  They may not all be possible, practical, or appropriate, but it's definitely worth exploring what can be done.

Yes those are all valid options, the compile time checks and static analysis will probably be difficult to implement. Also, the issue is that we cannot really catch this error in one single point since we can't just stop a control from calling functions that need some control parts. Everywhere where we need template parts we need a way to check if we actually have that template.

When a bug is found, it's also appropriate to ask if there are any other similar issues elsewhere that haven't been found yet...

How can all existing controls be tested so that they can cope when any (all?) named part(s?) are excluded when retemplated?

I think unless we write a lot of unit tests for this by hand, it will stay hard. Ideally of course, we would test every control without different templates with varying degrees of parts existing.

Such a check wouldn't need to be and entirely manual process. If looking to do this, I'd write a script to walk through all current controls with a template and then iterated through each template to find named items and then create versions of the template without each named element. It's a bit of work but not overly complicated.

Yes we could do this automated, however the problem is that you need to test all kind of scenarios a control can get into to check whether every state of the control respects template parts being missing.

We do our best to not be reliant on template parts being present, this particular example slipped through the cracks. I think adding a test that applies an empty template to all of our controls wouldn't be that hard and would likely catch a lot of these issues. @ranjeshj what do you think? @MartinZikmund or @chingucoding interested in looking into that?

If it is not time critical, I can take a look. Should I create an issue for tracking @StephenLPeters ?

[michael-hawker]

The best way to test would to just be blank out all the styles from all controls and then try and load them, right? Though I suppose you could have cascades, where a component is only looked at if another exists...

It should catch at least some though...

[StephenLPeters]

The best way to test would to just be blank out all the styles from all controls and then try and load them, right? Though I suppose you could have cascades, where a component is only looked at if another exists...

It should catch at least some though...

Yes, I think this would be pretty easy and catch the majority of cases.

@mrlacey its not as simple as named element means the code behind requires the element. Sometimes we name an element simply to reference it other places in the template with Bindings or with Setters. This means that some of the suggestions you had would be too restrictive as there are valid scenarios for deleting named template parts.

Even if a template part is used by code behind to wire something up, it can still be valid to delete that template part, and the code should be resilient to that.

I definitely agree this is a place we could really improve the framework.

[mrlacey]

I'm fine with this not being a simple thing (or at least as simple as my suggestions may have suggested)
What I'm interested in is seeing this as an opportunity for improvement.

I definitely agree this is a place we could really improve the framework.

Yay!

[michael-hawker]

I know this specific issue has been resolved, but I don't think there's a general tracking issue for improving this across the repo, eh?

[StephenLPeters]

We can repurpose this one.

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.