Fixing MCGRIDINFO definition #2911

vladimir-krestov · 2020-02-26T09:07:34Z

Fixes #2912
Related issue #2475

MonthCalendarAccessibleObject gets incorrect SYSTEMTIME info when sending Windows messages due to incorrect MCGRIDINFO definition.

Proposed changes

Edit MCGRIDINFO to avoid SYSTEMTIME value errors

Customer Impact

A user can use a correct MonthCalendar without exceptions

Regression?

Yes, from PR [Accessibility]: Adding MonthCalendar control UI Automation providers to improve calendar control accessibility #2090

Risk

Minimal

Screenshots

Before

Incorrect SYSTEMTIME value

After

Correct SYSTEMTIME value

Test methodology

CTI
Manual UI testing
Unit testing

Accessibility testing

Using Inspect and Narrator tools

Test environment(s)

.NET Core SDK: 5.0.0-alpha.1.20072.3
Microsoft Windows [Version 10.0.18363.592]

Microsoft Reviewers: Open in CodeFlow

src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.SendMessageW.cs

codecov · 2020-02-26T09:24:25Z

Codecov Report

Merging #2911 into master will decrease coverage by 0.00311%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                 @@
##              master       #2911         +/-   ##
===================================================
- Coverage   62.14766%   62.14455%   -0.00311%     
===================================================
  Files           1257        1257                 
  Lines         449428      449428                 
  Branches       39227       39227                 
===================================================
- Hits          279309      279295         -14     
- Misses        164638      164661         +23     
+ Partials        5481        5472          -9

Flag	Coverage Δ
#Debug	`62.14455% <100.00000%> (-0.00311%)`	⬇️
#production	`33.33637% <100.00000%> (-0.00556%)`	⬇️
#test	`98.97490% <ø> (ø)`

src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.SendMessageW.cs

vladimir-krestov · 2020-02-26T16:10:22Z

@RussKie, @weltkante, @hughbe, please confirm and I'll give these changes to testing.

weltkante

As far as I'm concerned the root cause for the interop bug has been identified and will be fixed, I have no further concerns regarding the PR.

src/System.Windows.Forms.Primitives/src/Interop/ComCtl32/Interop.MCGRIDINFO.cs

weltkante · 2020-03-05T13:38:55Z

The interop fix would probably also fix #2912

vladimir-krestov · 2020-03-10T15:34:13Z

Adding unit tests...
Changes for DateTimePicker.cs and MonthCalendar.MonthCalendarAccessibleObject.cs in first commit will be added in another PR.

RussKie

👍
Please add tests similar to https://github.com/dotnet/winforms/blob/master/src/System.Windows.Forms.Primitives/tests/Interop/Richedit/EDITSTREAMTests.cs, or better yet like this: https://github.com/dotnet/winforms/blob/master/src/System.Windows.Forms.Primitives/tests/Interop/Comdlg32/PRINTDLGWTests.cs

vladimir-krestov · 2020-03-13T11:41:06Z

Added sizeof unit tests like PRINTDLGWTests
Added Marshal unit tests as @weltkante said #2911 (comment)

/cc @RussKie

vladimir-krestov · 2020-03-14T08:04:55Z

The changes from the first commit have been added in PR #2975

vladimir-krestov · 2020-03-14T09:41:43Z

Testing the fix for #2912 and #2475 in progress...

RussKie · 2020-03-16T01:47:06Z

src/System.Windows.Forms.Primitives/tests/Interop/ComCtl32/MCGRIDINFOTests.cs

+        [ConditionalFact(nameof(Is32bit))]
+        public unsafe void MCGRIDINFO_x32_Marshal_Size()
+        {
+            Assert.Equal(84, Marshal.SizeOf<MCGRIDINFO>());
+        }


Just FYI: there is no pressing need to test the classic marshaller, since it shouldn't be used it for blittable structs.

Do you mean I need to remove this?
Can I keep it here?

#2911 (comment)

since this struct actually contains bools and this particular layout difference was cause of a regression I think there is some value to test both in order to prevent future regressions

RussKie · 2020-03-16T01:49:33Z

src/System.Windows.Forms.Primitives/tests/Interop/ComCtl32/MCGRIDINFOTests.cs

+        public static bool Is32bit => IntPtr.Size == 4;
+        public static bool Is64bit => IntPtr.Size == 8;


Please move these out to src/Common/tests/, and update https://github.com/dotnet/winforms/blob/master/src/System.Windows.Forms.Primitives/tests/Interop/Comdlg32/PRINTDLGWTests.cs as well.

@RussKie, I moved these to the new ArchitectureDetection class and updated MCGRIDINFOTests and PRINTDLGWTests tests. Please check if I made it correctly.

RussKie · 2020-03-16T09:09:41Z

👍

vladimir-krestov · 2020-03-16T10:51:20Z

Testers approved that Issue #2912 has been fixed. ✔️
Related Issue #2475 will be fixed by PR #2975.

to get correct data from WinAPI messages Fixes Issue dotnet#2912 Related Issue dotnet#2475 Related PR dotnet#2975

vladimir-krestov · 2020-03-23T20:16:37Z

PR is ready.
/cc @RussKie

against incorrect parameters of methods to avoid exception throwing Related Issues dotnet#2912 and dotnet#2475 Related PR dotnet#2911

Add protection for MonthCalendarAccessibleObject against incorrect parameters of methods to avoid exception throwing Related Issues #2912 and #2475 Related PR #2911

against incorrect parameters of methods to avoid exception throwing Related Issues dotnet#2912 and dotnet#2475 Related PR dotnet#2911

vladimir-krestov requested a review from a team as a code owner February 26, 2020 09:07

ghost assigned vladimir-krestov Feb 26, 2020

vladimir-krestov added the waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Feb 26, 2020

vladimir-krestov commented Feb 26, 2020

View reviewed changes

src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.SendMessageW.cs Outdated Show resolved Hide resolved

vladimir-krestov changed the title ~~[Accessibility] Fixing MonthCalendar AccessibleObject~~ WIP: [Accessibility] Fixing MonthCalendar AccessibleObject Feb 26, 2020

vladimir-krestov requested a review from RussKie February 26, 2020 09:18

hughbe reviewed Feb 26, 2020

View reviewed changes

src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.SendMessageW.cs Outdated Show resolved Hide resolved

vladimir-krestov force-pushed the dev/v-vlkres/MonthCalendarNullReferenceException branch from 2fb6c37 to ce30b1a Compare February 26, 2020 12:30

weltkante previously approved these changes Feb 26, 2020

View reviewed changes

JeremyKuhne reviewed Feb 26, 2020

View reviewed changes

src/System.Windows.Forms.Primitives/src/Interop/ComCtl32/Interop.MCGRIDINFO.cs Show resolved Hide resolved

weltkante mentioned this pull request Feb 27, 2020

fix: PrintDialog not shown #2851

Merged

weltkante mentioned this pull request Mar 5, 2020

An exception occurred when MonthCalendar switched dates #2912

Closed

merriemcgaw linked an issue Mar 6, 2020 that may be closed by this pull request

An exception occurred when MonthCalendar switched dates #2912

Closed

vladimir-krestov dismissed weltkante’s stale review via 6976b0e March 6, 2020 08:53

vladimir-krestov changed the title ~~WIP: [Accessibility] Fixing MonthCalendar AccessibleObject~~ WIP: Fixing MCGRIDINFO definition Mar 10, 2020

RussKie reviewed Mar 11, 2020

View reviewed changes

vladimir-krestov added the waiting-review This item is waiting on review by one or more members of team label Mar 13, 2020

vladimir-krestov mentioned this pull request Mar 14, 2020

[Refactoring] Added protection for MonthCalendarAccessibleObject #2975

Merged

vladimir-krestov force-pushed the dev/v-vlkres/MonthCalendarNullReferenceException branch from 5c1ba42 to 0b3d075 Compare March 14, 2020 08:25

RussKie removed the waiting-review This item is waiting on review by one or more members of team label Mar 16, 2020

RussKie reviewed Mar 16, 2020

View reviewed changes

vladimir-krestov added 📫 waiting-approval and removed waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) labels Mar 16, 2020

RussKie previously approved these changes Mar 19, 2020

View reviewed changes

Fixed MCGRIDINFO definition

8c3830f

to get correct data from WinAPI messages Fixes Issue dotnet#2912 Related Issue dotnet#2475 Related PR dotnet#2975

vladimir-krestov dismissed RussKie’s stale review via 8c3830f March 23, 2020 19:58

vladimir-krestov force-pushed the dev/v-vlkres/MonthCalendarNullReferenceException branch from 16ea82a to 8c3830f Compare March 23, 2020 19:58

vladimir-krestov changed the title ~~WIP: Fixing MCGRIDINFO definition~~ Fixing MCGRIDINFO definition Mar 23, 2020

RussKie approved these changes Mar 24, 2020

View reviewed changes

RussKie removed the 📫 waiting-approval label Mar 24, 2020

RussKie merged commit e75f8af into dotnet:master Mar 24, 2020

ghost added this to the 5.0 milestone Mar 24, 2020

weltkante mentioned this pull request Jul 13, 2020

Replace use of Marshal.SizeOf with sizeof for blittable structs dotnet/wpf#3218

Open

vladimir-krestov deleted the dev/v-vlkres/MonthCalendarNullReferenceException branch November 4, 2020 07:37

ghost locked as resolved and limited conversation to collaborators Feb 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing MCGRIDINFO definition #2911

Fixing MCGRIDINFO definition #2911

vladimir-krestov commented Feb 26, 2020 •

edited

Loading

codecov bot commented Feb 26, 2020 •

edited

Loading

vladimir-krestov commented Feb 26, 2020

weltkante left a comment

weltkante commented Mar 5, 2020

vladimir-krestov commented Mar 10, 2020 •

edited

Loading

RussKie left a comment

vladimir-krestov commented Mar 13, 2020

vladimir-krestov commented Mar 14, 2020

vladimir-krestov commented Mar 14, 2020

RussKie Mar 16, 2020

vladimir-krestov Mar 16, 2020

vladimir-krestov Mar 16, 2020

weltkante Mar 16, 2020

RussKie Mar 16, 2020

vladimir-krestov Mar 16, 2020

RussKie commented Mar 16, 2020 via email •

edited

Loading

vladimir-krestov commented Mar 16, 2020

vladimir-krestov commented Mar 23, 2020

		public static bool Is32bit => IntPtr.Size == 4;
		public static bool Is64bit => IntPtr.Size == 8;

Fixing MCGRIDINFO definition #2911

Fixing MCGRIDINFO definition #2911

Conversation

vladimir-krestov commented Feb 26, 2020 • edited Loading

Proposed changes

Customer Impact

Regression?

Risk

Screenshots

Before

After

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

codecov bot commented Feb 26, 2020 • edited Loading

Codecov Report

vladimir-krestov commented Feb 26, 2020

weltkante left a comment

Choose a reason for hiding this comment

weltkante commented Mar 5, 2020

vladimir-krestov commented Mar 10, 2020 • edited Loading

RussKie left a comment

Choose a reason for hiding this comment

vladimir-krestov commented Mar 13, 2020

vladimir-krestov commented Mar 14, 2020

vladimir-krestov commented Mar 14, 2020

RussKie Mar 16, 2020

Choose a reason for hiding this comment

vladimir-krestov Mar 16, 2020

Choose a reason for hiding this comment

vladimir-krestov Mar 16, 2020

Choose a reason for hiding this comment

weltkante Mar 16, 2020

Choose a reason for hiding this comment

RussKie Mar 16, 2020

Choose a reason for hiding this comment

vladimir-krestov Mar 16, 2020

Choose a reason for hiding this comment

RussKie commented Mar 16, 2020 via email • edited Loading

vladimir-krestov commented Mar 16, 2020

vladimir-krestov commented Mar 23, 2020

vladimir-krestov commented Feb 26, 2020 •

edited

Loading

codecov bot commented Feb 26, 2020 •

edited

Loading

vladimir-krestov commented Mar 10, 2020 •

edited

Loading

RussKie commented Mar 16, 2020 via email •

edited

Loading