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

Update TeachingTip to use XamlRootChanged_revoker #6121

Merged
merged 4 commits into from
Oct 20, 2021

Conversation

kmahone
Copy link
Member

@kmahone kmahone commented Oct 19, 2021

There is an issue with the XamlRoot Changed event handler that results in the C++/WinRT auto_revoker not working as expected. This had the effect that the event handler was not being revoked in TeachingTip and when the XamlRoot.Changed event fired, we would raise the event on a potentially deleted TeachingTip. This is causing crashes in apps.

The solution is to use a custom implementation of XamlRootChanged_revoker.

The underlying issue with the XamlRoot.Changed event is tracked by OS Bug 21302432.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Oct 19, 2021
@kmahone
Copy link
Member Author

kmahone commented Oct 19, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@RBrid
Copy link
Contributor

RBrid commented Oct 19, 2021

I think SwipeControl is using the old code too:
m_xamlRootChangedRevoker = xamlRoot.Changed(winrt::auto_revoke, { this, &SwipeControl::CurrentXamlRootChanged });

Maybe it's worth changing it too?

@kmahone
Copy link
Member Author

kmahone commented Oct 19, 2021

I think SwipeControl is using the old code too: m_xamlRootChangedRevoker = xamlRoot.Changed(winrt::auto_revoke, { this, &SwipeControl::CurrentXamlRootChanged });

Maybe it's worth changing it too?

Let me fix that too.

@kmahone
Copy link
Member Author

kmahone commented Oct 19, 2021

I think SwipeControl is using the old code too: m_xamlRootChangedRevoker = xamlRoot.Changed(winrt::auto_revoke, { this, &SwipeControl::CurrentXamlRootChanged });
Maybe it's worth changing it too?

Let me fix that too.

Fixed SwipeControl also.

@kmahone
Copy link
Member Author

kmahone commented Oct 19, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kmahone kmahone merged commit c5b776d into main Oct 20, 2021
@kmahone kmahone deleted the user/kmahone/teachingtipxamlroot branch October 20, 2021 01:15
@StephenLPeters StephenLPeters added area-SwipeControl area-TeachingTip team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Oct 21, 2021
StephenLPeters pushed a commit that referenced this pull request Oct 21, 2021
* Publish symbols to symweb (#6014)

* Fix CBS Vpack to handle WebView2 (#6033)

* Update TeachingTip and SwipeControl to use XamlRootChanged_revoker (#6121)

* Create and push Microsoft.UI.Xaml vpack as part of Release Build (#6131)
@ghost
Copy link

ghost commented Apr 14, 2022

🎉Microsoft.UI.Xaml v2.8.0-prerelease.220413001 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 13, 2022

🎉Microsoft.UI.Xaml v2.8.0-prerelease.220712001 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 18, 2022

🎉Microsoft.UI.Xaml v2.8.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jan 3, 2023

🎉Microsoft.UI.Xaml v2.8.2 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants