fix 2091 Remove deprecated controls #2157

RussKie · 2019-10-23T06:49:12Z

Fixes #2091

Proposed changes

Remove the following API because they have been obsolete since .NET 2.0:
- Menu
- MainMenu
- ContextMenu
- MenuItem
- IMenuEditorService
- ToolBar
- ToolBarAppearance
- ToolBarButton
- ToolBarButtonClickEventArgs
- ToolBarButtonClickEventHandler
- ToolBarButtonStyle
- ToolBarTextAlign
- DataGrid
- DataGridAddNewRow
- DataGridBoolColumn
- DataGridCaption
- DataGridCell
- DataGridColumnCollection
- DataGridColumnStyle
- DataGridDefaultColumnWidthTypeConverter
- DataGridLineStyle
- DataGridParentRows
- DataGridParentRowsLabel
- DataGridRelationshipRow
- DataGridRow
- DataGridState
- DataGridTableCollection
- DataGridTableStyle
- DataGridTextBox
- DataGridTextBoxColumn
- DataGridToolTip
- GridTablesFactory
- IDataGridEditingService
Remove the following members in AxHost type:
- ContextMenu ContextMenu { get; set; }
- event EventHandler ContextMenuChanged
Remove the following members in Control type:
- ContextMenu ContextMenu { get; set; }
- event EventHandler ContextMenuChanged
- void OnContextMenuChanged(EventArgs e)
Remove the following members in Form type:
- MainMenu Menu { get; set; }
- MainMenu MergedMenu { get; }
- void Control.OnContextMenuChanged(EventArgs e)
Remove the following members in NotifyIcon type:
- ContextMenu ContextMenu { get; set; }
Remove the following members in PrintPreviewDialog type:
- MainMenu Menu { get; set; }
- ContextMenu ContextMenu { get; set; }
- event EventHandler ContextMenuChanged
Remove the following members in ToolStripDropDown type:
- ContextMenu ContextMenu { get; set; }
- event EventHandler ContextMenuChanged
Remove the following members in TreeNode type:
- ContextMenu ContextMenu { get; set; }
Remove the following members in UpDownBase type:
- ContextMenu ContextMenu { get; set; }

Customer Impact

Removed controls no longer available to customers and will cause builds to break
Customers will need to replace the removed controls with available alternatives.

Regression?

No

Risk

Medium, the controls in questions have been deprecated in .NET 2.0 era, however there is a recorded use of those.

Testing

Tested by the CTI team

Microsoft Reviewers: Open in CodeFlow

mikedn · 2019-10-23T06:52:05Z

It seems strange that controls such as Menu and ContextMenu are marked as deprecated. These are just wrappers over OS provided functionality and I haven't heard of that being deprecated.

RussKie · 2019-10-23T06:55:12Z

codecov · 2019-10-23T06:57:45Z

Codecov Report

Merging #2157 into release/3.1 will decrease coverage by 0.84281%.
The diff coverage is 13.95349%.

@@                 Coverage Diff                 @@
##           release/3.1     #2157         +/-   ##
===================================================
- Coverage     26.41782%   25.575%   -0.84282%     
===================================================
  Files              818       793         -25     
  Lines           268811    252958      -15853     
  Branches         38145     35956       -2189     
===================================================
- Hits             71014     64694       -6320     
+ Misses          192719    183539       -9180     
+ Partials          5078      4725        -353

Flag	Coverage Δ
#Debug	`25.575% <13.95349%> (-0.84282%)`	⬇️
#production	`25.575% <13.95349%> (-0.84282%)`	⬇️
#test	`100% <ø> (ø)`	⬆️

mikedn · 2019-10-23T06:58:29Z

I don't see how that documentation piece implies deprecation.

weltkante · 2019-10-23T07:11:20Z

@mikedn the documentation says the classes/properties couldn't be removed for backwards compatibility and you should use the replacement instead for any new code, sounds pretty much like deprecation to me even if it isn't spelled out. Its not said the underlying win32 API is deprecated, but having the same concept mapped twice into WinForms is unnecessary to say the least. WinForms already started this deprecation in other PRs, this is just continuing it. If you have problems with it and reasons to use those classes in your application its probably a good idea to open a separate issue to discuss it, as some PRs have already passed I think.

For what it matters the corresponding properties to set e.g. ContextMenu have been removed from designers for a while, it is just consequential to also hide the classes

mikedn · 2019-10-23T07:29:25Z

the documentation says the classes/properties couldn't be removed for backwards compatibility and you should use the replacement instead for any new code, sounds pretty much like deprecation to me even if it isn't spelled out

The documentation does not say "you should use the replacement". The documentation says "retained for both backward compatibility and future use if you choose".

If you have problems with it and reasons to use those classes in your application its probably a good idea to open a separate issue to discuss it, as some PRs have already passed I think.

Thanks but considering RussKie's teleported documentation paragraph without any other comment and your claim that the documentation says something that it actually doesn't I'm probably wasting my time here.

weltkante · 2019-10-23T08:02:10Z

@mikedn language lawyering is probably wasting time, yes. If something has been "retained for backwards compatibility" it indicates it was considered to be removed/replaced. Deprecation is not a formal process (even if it sometimes would be nice to have one) and documentation isn't specification (so arguing as such is also wasting time). I'm just explaining, if you don't care or are aware thats ok, I won't continue the discussion.

"retained for [...] future use if you choose"

Well, that should probably be updated in the .NET Core documentation then, assuming they are planning on actually removing the API in later versions and not just keep hiding it from view.

merriemcgaw · 2019-10-23T23:53:48Z

In .NET Framework we were bound to never remove any types because of the backwards compatibility reasons. .NET Core, being a SxS framework allows us more flexibility in this area. We've created, and the bulk of our customers use, the replacements of MenuStrip and DataGridView classes. As we modernize WinForms on Core we may be removing classes for which there are better alternatives. If a customer really wants to use the Menu, DataGrid and ContextMenu they are able to keep their app on the current version .NET Core (3.0) and use the classes. If they want to use other improvements in the stack we encourage them to look at the replacement classes.

RussKie · 2019-10-30T05:02:19Z

src/System.Windows.Forms/src/System/Windows/Forms/ComboBox.cs

-                    if (ContextMenu != null)
-                    {
-                        CaptureInternal = false;
-                    }


This looks like a bug, originally CaptureInternal = true if ContextMenu != null || ContextMenuStrip != null, however it was reverted only in case of ContextMenu != null...

Could also have been a workaround for HMENU specific behavior, maybe HWND based menus didn't need to reset the capture, the events and call chains would have been entirely different. Without comments why it was done the way it is thats really hard to tell.

RussKie · 2019-10-30T05:07:14Z

src/System.Windows.Forms/src/System/Windows/Forms/RichTextBox.cs

@@ -4157,49 +4151,15 @@ public HRESULT GetDragDropEffect(BOOL fDrag, int grfKeyState, ref int pdwEffect)
            public HRESULT GetContextMenu(short seltype, IntPtr lpoleobj, ref Richedit.CHARRANGE lpchrg, out IntPtr hmenu)


❗️ I'm really unsure about this method 😕

I don't think ContextMenuStrip.Handle is a HMENU, is it? (Probably HWND) I don't know how this GetContextMenu method is used but if its for interop with win32 richedit you can't just rip out and replace their usage of HMENU. In fact, if you need HMENU for interop with a native controls menu you can't get rid of the old class at all (well you can replace it with something internal I guess)

Just to confirm, I've checked base classes (see my other comment) and my suspicion is correct and you introduced a bug here by using ContextMenuStrip.Handle (HWND) in place of ContextMenu.Handle (HMENU)

I see you did the same update here as with the other HMENU Handle access, I agree removing the code block for now until some regression can be identified (if there is any) is definitely better than leaving broken code in place.

For what its worth context menus appear to work without having this code block in place, so if it triggers at all it probably needs very special conditions and doesn't affect the general usecase.

src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/DocumentDesigner.cs

src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/IMenuEditorService.cs

weltkante · 2019-10-30T08:23:37Z

Marking things as [Obsolete(error: true)] means you can't use them anymore, right? Why retain implementation at all then? From what I can see in the PR you are ripping out random implementation details, that doesn't seem right to me. Either you retain implementation and the ability to use the classes, or you remove it cleanly everywhere.

IMHO the title of the PR is misleading, you are not just marking deprecated controls as obsolete, you are already removing the functionality.

This should definitely NOT go into 3.1, thats something for a major version change. ~~Sorry for not noticing this initially~~ [edit] right, I see the title was changed and the [Obsolete(error: true)] was added later, that explains my confusion. I think this PR is going too far now for a 3.1 PR.

RussKie · 2019-10-30T23:48:22Z

We have new directions every day 😁
We are planning to rip all these types out. So everything about this PR will change.

weltkante · 2019-10-31T08:18:05Z

Ok, just make sure you get the interop right, I'm not an expert on that part of the WinForms implementation but I think the replacement classes are a WinForms-only reimplementation (not a wrapper around win32 anymore) while the classes to be removed were interoperable with the win32 HMENU API. That means the replacement will be incompatible with any win32 interop (e.g. richedit came up above, ActiveX hosting and various OLE interfaces are other candidates which may interop over HMENU)

bigworld12 · 2019-11-08T03:50:32Z

i have a concern though, winforms (unlike wpf) is heavily reliant on the visual designer, i.e. any modifications to the design will have to go through the visual designer, how do you plan on simplifying that process to replace old deprecated controls with new ones?

RussKie · 2019-11-08T04:50:39Z

how do you plan on simplifying that process to replace old deprecated controls with new ones?

It is not possible to replace controls via the Designer.
If you're porting a project that contains any of the above controls to .NET Core 3.1 you will need to replace them manually. Everything else should just work.

RussKie · 2019-11-12T01:26:24Z

Docs: dotnet/docs#15813

`ToolBar` implementation was removed in dotnet#2157. Closes dotnet#2372

RussKie added 📫 waiting-approval Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Oct 23, 2019

RussKie requested a review from a team as a code owner October 23, 2019 06:49

RussKie self-assigned this Oct 23, 2019

merriemcgaw added Ask-Mode and removed 📫 waiting-approval Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Oct 23, 2019

leecow added Servicing-approved .NET Shiproom approved the PR for merge and removed Ask-Mode labels Oct 29, 2019

RussKie added the 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 30, 2019

RussKie changed the title ~~Fix 2091 Mark deprecated controls as non-browsable~~ WIP: fix 2091 Mark deprecated controls as non-browsable Oct 30, 2019

RussKie force-pushed the fix_2091_mark_deprecated_controls_as_nonbrowsable branch 2 times, most recently from 1291188 to 527587c Compare October 30, 2019 05:44

RussKie changed the title ~~WIP: fix 2091 Mark deprecated controls as non-browsable~~ WIP: fix 2091 Mark deprecated controls as obsolete Oct 30, 2019

RussKie commented Oct 30, 2019

View reviewed changes

RussKie removed the Servicing-approved .NET Shiproom approved the PR for merge label Oct 30, 2019

RussKie changed the title ~~WIP: fix 2091 Mark deprecated controls as obsolete~~ WIP: fix 2091 Remove deprecated controls Oct 30, 2019

RussKie force-pushed the fix_2091_mark_deprecated_controls_as_nonbrowsable branch 2 times, most recently from fe34344 to c4c39bf Compare October 31, 2019 05:52

RussKie force-pushed the fix_2091_mark_deprecated_controls_as_nonbrowsable branch from ef75d09 to c25937a Compare November 7, 2019 01:42

RussKie added 📖 documentation: breaking A change in behavior that could be breaking for applications. Needs to be documented. and removed 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Nov 7, 2019

RussKie added this to the 3.1 milestone Nov 7, 2019

RussKie merged commit 1317264 into dotnet:release/3.1 Nov 7, 2019

RussKie deleted the fix_2091_mark_deprecated_controls_as_nonbrowsable branch November 7, 2019 23:10

RussKie mentioned this pull request Nov 8, 2019

[release/3.1] Update dependencies from dotnet/winforms dotnet/wpf#2161

Merged

This was referenced Nov 8, 2019

Fixup to WPF to adapt to removal of legacy API's from WinForms dotnet/wpf#2168

Merged

Fixup WPF to adapt to WinForms api removal dotnet/wpf#2169

Closed

vatsan-madhavan mentioned this pull request Nov 14, 2019

Fixup WPF to adapt to WinForms API removal dotnet/wpf#2188

Closed

RussKie added a commit to RussKie/winforms that referenced this pull request Nov 27, 2019

Remove remnants of ToolBar class

4d7a6ae

`ToolBar` implementation was removed in dotnet#2157. Closes dotnet#2372

This was referenced Nov 27, 2019

Remove remnants of ToolBar class #2435

Merged

Fix message constants in Interop.ComCtl32.TB. #2372

Closed

[WinForms] Remove deprecated controls mono/mono#17929

Open

RussKie mentioned this pull request Jan 2, 2020

What happened to ContextMenu/MenuItem UI classes? #2614

Closed

RussKie mentioned this pull request Mar 2, 2020

fix: Fail roll forward .NET Core 3.x -> .NET 5.0 #2923

Merged

RussKie mentioned this pull request Apr 30, 2020

[WinForms] MenuMerge enum deprecated - mark as obsolete in 3.1? dotnet/dotnet-api-docs#4180

Open

jscholes mentioned this pull request May 23, 2020

Accessibility: problematic screen reader UX for ContextMenuStrip (NVDA) #3334

Closed

RussKie added a commit to RussKie/winforms that referenced this pull request May 14, 2021

Remove unused var, was used prior to dotnet#2157

a0e7d5e

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix 2091 Remove deprecated controls #2157

fix 2091 Remove deprecated controls #2157

RussKie commented Oct 23, 2019 •

edited

Loading

mikedn commented Oct 23, 2019

RussKie commented Oct 23, 2019

codecov bot commented Oct 23, 2019 •

edited

Loading

mikedn commented Oct 23, 2019

weltkante commented Oct 23, 2019 •

edited

Loading

mikedn commented Oct 23, 2019

weltkante commented Oct 23, 2019 •

edited

Loading

merriemcgaw commented Oct 23, 2019

RussKie Oct 30, 2019

weltkante Nov 5, 2019 •

edited

Loading

RussKie Oct 30, 2019

weltkante Oct 30, 2019 •

edited

Loading

weltkante Nov 5, 2019 •

edited

Loading

weltkante Nov 6, 2019 •

edited

Loading

weltkante commented Oct 30, 2019 •

edited

Loading

RussKie commented Oct 30, 2019

weltkante commented Oct 31, 2019

bigworld12 commented Nov 8, 2019

RussKie commented Nov 8, 2019

RussKie commented Nov 12, 2019

		@@ -4157,49 +4151,15 @@ public HRESULT GetDragDropEffect(BOOL fDrag, int grfKeyState, ref int pdwEffect)
		public HRESULT GetContextMenu(short seltype, IntPtr lpoleobj, ref Richedit.CHARRANGE lpchrg, out IntPtr hmenu)

fix 2091 Remove deprecated controls #2157

fix 2091 Remove deprecated controls #2157

Conversation

RussKie commented Oct 23, 2019 • edited Loading

Proposed changes

Customer Impact

Regression?

Risk

Testing

Microsoft Reviewers: Open in CodeFlow

mikedn commented Oct 23, 2019

RussKie commented Oct 23, 2019

codecov bot commented Oct 23, 2019 • edited Loading

Codecov Report

mikedn commented Oct 23, 2019

weltkante commented Oct 23, 2019 • edited Loading

mikedn commented Oct 23, 2019

weltkante commented Oct 23, 2019 • edited Loading

merriemcgaw commented Oct 23, 2019

RussKie Oct 30, 2019

Choose a reason for hiding this comment

weltkante Nov 5, 2019 • edited Loading

Choose a reason for hiding this comment

RussKie Oct 30, 2019

Choose a reason for hiding this comment

weltkante Oct 30, 2019 • edited Loading

Choose a reason for hiding this comment

weltkante Nov 5, 2019 • edited Loading

Choose a reason for hiding this comment

weltkante Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

weltkante commented Oct 30, 2019 • edited Loading

RussKie commented Oct 30, 2019

weltkante commented Oct 31, 2019

bigworld12 commented Nov 8, 2019

RussKie commented Nov 8, 2019

RussKie commented Nov 12, 2019

RussKie commented Oct 23, 2019 •

edited

Loading

codecov bot commented Oct 23, 2019 •

edited

Loading

weltkante commented Oct 23, 2019 •

edited

Loading

weltkante commented Oct 23, 2019 •

edited

Loading

weltkante Nov 5, 2019 •

edited

Loading

weltkante Oct 30, 2019 •

edited

Loading

weltkante Nov 5, 2019 •

edited

Loading

weltkante Nov 6, 2019 •

edited

Loading

weltkante commented Oct 30, 2019 •

edited

Loading