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.

Sign up for GitHub

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

Jump to bottom

Fix bug with calling SwipeItems Clear function would crash #1383

Merged
merged 5 commits into from
Oct 2, 2019
Merged

Fix bug with calling SwipeItems Clear function would crash #1383

merged 5 commits into from
Oct 2, 2019

Conversation

marcelwgn
Copy link
Collaborator

Description

Added checks for the code in question where m_interactionTracker would be null when calling clear.

When no new SwipeItem has been added to the SwipeItems object that Clear was invoked on, the SwipeControls that uses this SwipeItems object will not perform swipe operations in that direction.

Motivation and Context

Fixes #891

How Has This Been Tested?

Tested manually and by adding a new unit test (CanClearItemsWithoutCrashing) and a new test page (SwipeControlClearPage)

Screenshots (if appropriate):

@marcelwgn marcelwgn requested a review from a team as a code owner September 26, 2019 15:09
@jevansaks
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

I ran the test you authored without the product change and it is passing. Are you sure that the test exercises the issue?

@marcelwgn
Copy link
Collaborator Author

I ran the test you authored without the product change and it is passing. Are you sure that the test exercises the issue?

Hmm, yes I see. I will fix the TestApp to correctly represent the situation described in #891.

@marcelwgn
Copy link
Collaborator Author

I fixed the test page. Now without the fix, the test should crash.

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

looks good!

@StephenLPeters StephenLPeters self-requested a review October 1, 2019 22:15
StephenLPeters
StephenLPeters approved these changes Oct 1, 2019
View reviewed changes
Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@StephenLPeters StephenLPeters merged commit 18ef85a into microsoft:master Oct 2, 2019
@jevansaks jevansaks added the release note PR that we want to call out in the next release summary label Oct 2, 2019
@Sergio0694
Copy link
Member

Thanks for the fix @chingucoding! 🐱‍🏍

@marcelwgn
Copy link
Collaborator Author

Thanks for the fix @chingucoding! 🐱‍🏍

Always happy to help 😄

@msft-github-bot
Copy link
Collaborator

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

Handy links:

@msft-github-bot msft-github-bot mentioned this pull request Oct 7, 2019
Closed
@msft-github-bot
Copy link
Collaborator

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

Handy links:

@marcelwgn marcelwgn deleted the swipeitem-clear-crash branch April 16, 2021 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephenLPeters StephenLPeters StephenLPeters approved these changes

Assignees

@StephenLPeters StephenLPeters

Labels
release note PR that we want to call out in the next release summary
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SwipeItems.Clear() throws an AccessViolationException if the items are already in use
5 participants
@marcelwgn @jevansaks @StephenLPeters @Sergio0694 @msft-github-bot