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

Port "remove ContextMenu owner" fix from 4.8 #5843

Merged
merged 1 commit into from
Jan 19, 2022

Conversation

SamBent
Copy link
Contributor

@SamBent SamBent commented Dec 16, 2021

Addresses #5835
This is a port of a servicing fix in .NET 4.7-4.8.

Description

There are two reasons the context menu does not display. The first reason is that its owner gets removed from the tree after the right-button-up gesture but before the PopupControlService initiates the display logic. The logic notices that the owner is not being displayed, and therefore doesn't display the context menu. This is by design - you shouldn't display a context menu for an element that isn't itself visible.

The second reason is that the (shared) context menu is in a bad state, where it's marked IsOpen=true, but isn't actually being displayed. The display logic doesn't do anything, since it thinks the menu is already open. This is a bug, left over from the first case, which doesn't reset IsOpen to false when it aborts the display logic. The net effect is that once a context menu doesn't open because of reason 1, it will never open again, even in cases where it should.

The fix is to reset the context menu's state when aborting the display logic.

Customer Impact

ContextMenus stop working

Regression

No

Testing

Ad-hoc around customer scenario.
Standard regression testing.

Risk

Low. Port of a .NETFx servicing fix released earlier this year.

@SamBent SamBent requested a review from a team as a code owner December 16, 2021 02:40
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Dec 16, 2021
@ghost ghost requested review from fabiant3 and ryalanms December 16, 2021 02:40
@vishalmsft vishalmsft self-assigned this Dec 27, 2021
Copy link
Contributor

@vishalmsft vishalmsft left a comment

Choose a reason for hiding this comment

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

Reviewed with the test project attached in the issue.

@singhashish-wpf singhashish-wpf merged commit a539c26 into dotnet:main Jan 19, 2022
dipeshmsft added a commit that referenced this pull request Feb 7, 2022
Co-authored-by: Sam Bent <sambent@microsoft.com>
dipeshmsft added a commit that referenced this pull request Feb 7, 2022
Co-authored-by: Sam Bent <sambent@microsoft.com>
dipeshmsft pushed a commit that referenced this pull request Feb 7, 2022
Co-authored-by: Sam Bent <sambent@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Apr 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants