fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) #13295

Epica3055 · 2025-04-11T09:22:15Z

Proposed changes

revert PR#12431 , use another approach to fix Issue#12440
Introduce EditMouseDown flag in PropertyGridView.flag. This flag is on when the TextBox in the PropertyGrid.GridEntry is focused and is off when editing is done.
The cause of Issue#12440 is that after editing is done, GridEntryAccessibleObject will raise AutomationFocusChangedEvent

GridEntry.GridEntryAccessibleObject.cs

        ...
        internal override void SetFocus()
        {
            if (PropertyGridView is null || !PropertyGridView.IsHandleCreated)
            {
                return;
            }

            base.SetFocus();

            RaiseAutomationEvent(UIA_EVENT_ID.UIA_AutomationFocusChangedEventId);
        }

So in order to not let TextBox loose rectangle, add conditions to that event to exclude two cases:

when InPropertySet is on
when EditMouseDown is on

Regression?

Yes

Screenshots

Before

GetProperties method didn't get invoked described in #13071

After

Issues13071.01.mp4

Issues13071.02.mp4

Test methodology

manually

Test environment(s)

10.0.0-preview.3.25165.3

…not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) : revert PR#12431 , use another approach.

codecov · 2025-04-11T10:30:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.56834%. Comparing base (88c3a11) to head (ccd78fc).
Report is 150 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #13295         +/-   ##
===================================================
+ Coverage   61.34314%   62.56834%   +1.22520%     
===================================================
  Files           1547        1560         +13     
  Lines         158479      159675       +1196     
  Branches       14751       14904        +153     
===================================================
+ Hits           97216       99906       +2690     
+ Misses         60561       59000       -1561     
- Partials         702         769         +67

Flag	Coverage Δ
Debug	`62.56834% <ø> (-12.60374%)`	⬇️
integration	`11.31915% <ø> (-7.55283%)`	⬇️
production	`40.74551% <ø> (-34.42657%)`	⬇️
test	`95.69057% <ø> (∅)`
unit	`38.14619% <ø> (-36.11083%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

LeafShi1

LGTM！Adding a new Flag is indeed a good idea.

Tanya-Solyanik · 2025-04-16T00:01:58Z

@Epica3055 - is it possible to add unit test? Please send to @Olina-Zhang's team for testing

Olina-Zhang · 2025-04-16T09:30:03Z

Tested this PR change, GH issues: 13071, 12440, 12421 were fixed. Also tested some properties values list to choose in expand/collapse state using Accessibility Tools: Inspect/AccessibilityInsight/Narrator/NVDA, no new issue found.

src/test/unit/System.Windows.Forms/System/Windows/Forms/PropertyGridTests.cs

Tanya-Solyanik

Thank you, nice fix! Please backport it to 9!

Epica3055 · 2025-04-23T07:55:03Z

/backport to release/9.0

github-actions · 2025-04-23T07:55:13Z

Started backporting to release/9.0: https://github.com/dotnet/winforms/actions/runs/14612889585

github-actions · 2025-04-23T07:55:36Z

@Epica3055 backporting to release/9.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) : revert PR#12431 , use another approach.
Using index info to reconstruct a base tree...
A	src/System.Windows.Forms/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/GridEntry.GridEntryAccessibleObject.cs
A	src/System.Windows.Forms/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/PropertyGridView.Flags.cs
A	src/System.Windows.Forms/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/PropertyGridView.cs
Falling back to patching base and 3-way merge...
Auto-merging src/System.Windows.Forms/src/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/PropertyGridView.cs
CONFLICT (content): Merge conflict in src/System.Windows.Forms/src/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/PropertyGridView.cs
Auto-merging src/System.Windows.Forms/src/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/GridEntry.GridEntryAccessibleObject.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) : revert PR#12431 , use another approach.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

…ll does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) (#13356) Backport of [#13295](#13295) to release/9.0 Fixes #13071 ## Proposed changes - - revert [PR#12431](#12431) , use another approach to fix [Issue#12440](#12440) - Introduce `EditMouseDown` flag in PropertyGridView.flag. This flag is on when the TextBox in the PropertyGrid.GridEntry is focused and is off when editing is done. - The cause of [Issue#12440](#12440) is that after editing is done, GridEntryAccessibleObject will raise AutomationFocusChangedEvent GridEntry.GridEntryAccessibleObject.cs ``` c# ... internal override void SetFocus() { if (PropertyGridView is null || !PropertyGridView.IsHandleCreated) { return; } base.SetFocus(); RaiseAutomationEvent(UIA_EVENT_ID.UIA_AutomationFocusChangedEventId); } ``` So in order to not let TextBox loose rectangle, add conditions to that event to exclude two cases: 1. when InPropertySet is on 2. when EditMouseDown is on ## Regression? - Yes  ## Screenshots  ### Before GetProperties method didn't get invoked described in [#13071](#13071) ### After https://github.com/user-attachments/assets/4d013d27-8ebc-4020-a977-d2f825280bfc https://github.com/user-attachments/assets/7885c561-f37f-45fd-971c-6af9686db030 ## Test methodology  - - manually -

github-actions bot added the needs-area-label label Apr 11, 2025

dotnet-policy-service bot assigned Epica3055 Apr 11, 2025

fix dotnet#13071 Changing properties with RefreshProperties.All does …

c590898

…not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) : revert PR#12431 , use another approach.

Epica3055 force-pushed the Fix_issues_13071 branch from 50fac01 to c590898 Compare April 11, 2025 09:57

dotnet-policy-service bot added the draft draft PR label Apr 11, 2025

Epica3055 marked this pull request as ready for review April 14, 2025 02:21

Epica3055 requested a review from a team as a code owner April 14, 2025 02:21

dotnet-policy-service bot removed the draft draft PR label Apr 14, 2025

LeafShi1 previously approved these changes Apr 15, 2025

View reviewed changes

LeafShi1 requested a review from Tanya-Solyanik April 15, 2025 08:47

Tanya-Solyanik added waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) area-controls-PropertyGrid PropertyGrid and editor related issues and removed needs-area-label labels Apr 16, 2025

Olina-Zhang removed the waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Apr 16, 2025

Epica3055 dismissed LeafShi1’s stale review via 0d58217 April 17, 2025 02:30

Epica3055 commented Apr 17, 2025

View reviewed changes

src/test/unit/System.Windows.Forms/System/Windows/Forms/PropertyGridTests.cs Show resolved Hide resolved

add unit test

ccd78fc

Epica3055 force-pushed the Fix_issues_13071 branch from 0d58217 to ccd78fc Compare April 17, 2025 03:01

Tanya-Solyanik approved these changes Apr 23, 2025

View reviewed changes

Epica3055 mentioned this pull request Apr 23, 2025

[release/9.0] fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) #13356

Merged

Epica3055 merged commit 330a7c6 into dotnet:main Apr 23, 2025
8 checks passed

github-actions bot locked and limited conversation to collaborators May 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) #13295

fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) #13295

Uh oh!

Epica3055 commented Apr 11, 2025 •

edited

Loading

Uh oh!

codecov bot commented Apr 11, 2025 •

edited

Loading

Uh oh!

LeafShi1 left a comment

Uh oh!

Tanya-Solyanik commented Apr 16, 2025

Uh oh!

Olina-Zhang commented Apr 16, 2025

Uh oh!

Uh oh!

Tanya-Solyanik left a comment

Uh oh!

Epica3055 commented Apr 23, 2025

Uh oh!

github-actions bot commented Apr 23, 2025

Uh oh!

github-actions bot commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) #13295

fix #13071 Changing properties with RefreshProperties.All does not requery the property list in the PropertyGrid in .net 9 (worked in .net 8) #13295

Uh oh!

Conversation

Epica3055 commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Regression?

Screenshots

Before

After

Test methodology

Test environment(s)

Uh oh!

codecov bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LeafShi1 left a comment

Choose a reason for hiding this comment

Uh oh!

Tanya-Solyanik commented Apr 16, 2025

Uh oh!

Olina-Zhang commented Apr 16, 2025

Uh oh!

Uh oh!

Tanya-Solyanik left a comment

Choose a reason for hiding this comment

Uh oh!

Epica3055 commented Apr 23, 2025

Uh oh!

github-actions bot commented Apr 23, 2025

Uh oh!

github-actions bot commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

Epica3055 commented Apr 11, 2025 •

edited

Loading

codecov bot commented Apr 11, 2025 •

edited

Loading