Fix message constants in Interop.ComCtl32.TB.

[madewokherd]

Toolbar message constants are based on WM_USER, not LVM_FIRST.

Fixes this test case from Mono: https://github.com/mono/mono/blob/8683fa6db798298d576c120dd9e1443cd3c5b6de/mcs/class/System.Windows.Forms/Test/System.Windows.Forms/ToolBarTest.cs#L111

Proposed changes

• Changes the constants in Interop.ComCtl32.TB to be based on 0x400 (WM_USER) instead of 0x1000 (LVM_FIRST), as the win32 api expects.

Customer Impact

• I only know for sure that the test case is broken, but I expect that any modifications to a ToolBarButton after it's been displayed will not work without this change.

Regression?

• Yes

Risk

• I consider it low risk since because it's a clear regression from baa5058 and we're going back to the old values for these constants.

Test methodology

• I ran only this one test on Wine in Wine Mono, using Wine Mono's fork of winforms.

Microsoft Reviewers: Open in CodeFlow

[weltkante]

/cc @RussKie @hughbe looks like a regression from porting, does this need to be fixed in 3.1 as well?

Also probably should have an issue and not just a PR so it can be tracked properly?

[RussKie]

Thinking about it, I do not believe we should have introduced TB.FIRST in the first place.

Win32 have the following definitions:

Unlike, say LVM_* messages, which have LVM_FIRST message, and all messages are offset from that point, e.g.:

By adding a new member to the enum, we are creating a possibility (as unlikely as it may be) of calling interops with an incorrect value.
E.g. the following isn't jumping out and screaming "I'm incorrect":

User32.SendMessageW(this, (User32.WindowMessage)ComCtl32.TB.FIRST, (IntPtr)index, ref tbbi);

So I think we should have defined the enum as follows:

        public enum TB : int
        {
            GETBUTTONINFOW = User32.WM_USER + 63,
            SETBUTTONINFOW = User32.WM_USER + 64,
            INSERTBUTTONW = User32.WM_USER + 67,
        }

[RussKie]

/cc: @hughbe @JeremyKuhne

[RussKie]

looks like a regression from porting, does this need to be fixed in 3.1 as well?

No, this is regression introduced on the master branch in #2075. I'm guessing @hughbe accidentally written the wrong number.

[RussKie]

Thank you @madewokherd for spotting it! 👍

[madewokherd]

I see this was assigned to me but I'm confused as to what I should do to push this forward.

Using the WM_USER constant as you describe makes sense to me.

[RussKie]

I see this was assigned to me

I assigned to you since it is your PR :)

but I'm confused as to what I should do to push this forward.

To push it forward please make the changes as discussed above

[codecov]

Codecov Report

Merging #2372 into master will increase coverage by 0.00808%.
The diff coverage is n/a.

@@                 Coverage Diff                 @@
##              master       #2372         +/-   ##
===================================================
+ Coverage   28.47446%   28.48255%   +0.00809%     
===================================================
  Files            999         999                 
  Lines         259573      259573                 
  Branches       36836       36836                 
===================================================
+ Hits           73912       73933         +21     
+ Misses        180838      180822         -16     
+ Partials        4823        4818          -5

Flag	Coverage Δ
#Debug	28.48255% <ø> (+0.00808%)	⬆️
#production	28.48255% <ø> (+0.00808%)	⬆️
#test	100% <ø> (ø)	⬆️

[RussKie]

I just realised that this enum is used for ToolBar type that has been deprecated and removed in #2157.
So essentially this fix is not required.

I wasn't aware Mono still builds WinForms (which is exciting) and depends on our master. If it is indeed so, Mono must also remove deprecated types.

[madewokherd]

Mono has its own implementation of winforms and will not be affected. Wine Mono, a package used by Wine to provide an open-source replacement for .NET, uses winforms. Removing these controls is not an option for us as we must maintain compatibility with existing .NET applications.