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

Hosting UserControl via ActiveX does not work correctly #4370

Closed
arknu opened this issue Dec 15, 2020 · 45 comments · Fixed by #4626
Closed

Hosting UserControl via ActiveX does not work correctly #4370

arknu opened this issue Dec 15, 2020 · 45 comments · Fixed by #4626
Assignees
Labels
area-COM area-Interop 💥 regression-preview Regression from a preview release 📭 waiting-author-feedback The team requires more information from the author 💤 no-recent-activity

Comments

@arknu
Copy link

arknu commented Dec 15, 2020

  • .NET Core Version: 5.0

  • Have you experienced this same bug with .NET Framework?: No

Problem description:
I am experimenting with doing Office Addins without VSTO and using .NET 5. So far, I have succesfully manged to add ribbon buttons and I'm now working on a custom TaskPane, hosting a Winforms UserControl.

So far, so good. However, when instantiating the UserControl, an exception is thrown here:

HRESULT hr = _inPlaceUiWindow.GetWindow(&hwndParent);

_inPlaceUiWindow is null here and there is no check for that case.

Comparing to .NET Framework 4.8 reference source (https://referencesource.microsoft.com/#System.Windows.Forms/winforms/Managed/System/WinForms/Control.cs,17065), the implementation is different and I suspect that a regression has snuck in.

Expected behavior:
No NullReferenceException should be thrown.

Minimal repro:
You can download the project here: https://1drv.ms/u/s!AkwgkpetCvCpr4JeJeZfCGlKlHs4qA?e=PeB0So
You need to manually run regsvr32 OutlookAddinNet5.comhost.dll in the bin folder and register the addin in outlook:

Windows Registry Editor Version 5.00

[HKEY_CURRENT_USER\SOFTWARE\Microsoft\Office\Outlook\Addins\OutlookAddinNet5]
"CommandLineSafe"=dword:00000001
"Description"="OutlookAddinNet5 description"
"FirendlyName"="OutlookAddinNet5 name"
"LoadBehavior"=dword:00000003

Then Open Outlook and click the [2] button added to the ribbon.

@weltkante
Copy link
Contributor

weltkante commented Dec 15, 2020

I am experimenting with doing Office Addins without VSTO and using .NET 5.

Unfortunately there is no interest to support that scenario from the Office side (see e.g. #1763) so while the last statement from the WinForms team is to be willing to look into and fix bugs, you'll probably have a tough road ahead of you to get it working.

As far as Office integration support is concerned you cannot rely on compatibility of WinForms in .NET Core with .NET Framework since the Desktop Framework was using undocumented "Component Manager" integration with Office, and the Office team is not providing support to WinForms in .NET Core to support it properly (#247).

This is just FYI and no argument against fixing the problem you found.

Also, aside from above, TLB support has been dropped from COM interop in .NET Core so you'll either have to reinvent that or use Desktop Framework to generate the TLB for scenarios that need it. The new COM interop layer currently being built will most likely not support TLB out of the box either.

@arknu
Copy link
Author

arknu commented Dec 15, 2020

@weltkante Thank you for the context, wasn't having much luck searching. The Office team really needs to wake up and realize that HTML/JS is fine for simple addins, but complex addins with actual functionality need a proper language and runtime with full system access, i.e. .NET and C#. VSTO needs a path forward, even if that is just doing raw COM addins with C#.

My impression was always that since Office addins under the covers are just COM and WinForms is just a Win32 wrapper, it should "just work", once you implement the correct interfaces and get Office to load you addin. If you can do a native addin in C++ and COM, surely WinForms should work without extra support from the Office team. As long as they support COM addins...

However, I am not sure that this bug is anything Office-specific. This appears to be a more general regression in the COM/ActiveX interop that I just happened to hit doing my Office addin experiments.

@weltkante
Copy link
Contributor

weltkante commented Dec 15, 2020

However, I am not sure that this bug is anything Office-sepcific.

No, definitely not, ActiveX is separate and can be hosted outside Office as well, just wanted to inform you that you are likely to run into more (possibly subtile) problems.

surely WinForms should work without extra support from the Office team

Unfortunately WinForms in Desktop Framework integrated with Office to communicate form modality (WPF does not). The WinForms team didn't agree to remove that code (see discussions in the linked issues for their reasons), so since it isn't supported or tested in any way regressions are possible (and potentially subtile since they relate to how Office behaves when you show a modal WinForms form).

If you have the resources to see your project through that surely would help improve quality, we have this kind of Office Addins as well but currently all our .NET Core efforts are halted until custom control support is finished, so it may be a while before I get around to come back to rewrite and test our own addins.

@RussKie RussKie added area-COM area-Interop area-VSTO Visual Studio Tools for Office and general Office interop labels Dec 15, 2020
@RussKie
Copy link
Member

RussKie commented Dec 15, 2020

Thank you @weltkante.
@arknu VSTO interop story is currently a grey area, even for us. That said, if you stumble across any bugs in our codebase - feel free to send fixes in :)

@arknu
Copy link
Author

arknu commented Dec 15, 2020

@RussKie I don't believe this has anything to do with VSTO, so that label should be removed. In fact the original post explicitly states that I am not using VSTO at all. This is an ActiveX/COM interop issue that just happens to surface in Office, because that was where I was using it.

The code in question is missing null checks. Every other use of _inPlaceUiWindow in the code mentioned above has null checks, except this one. That seems like a mistake.

@RussKie RussKie removed the area-VSTO Visual Studio Tools for Office and general Office interop label Dec 16, 2020
@1R053
Copy link
Contributor

1R053 commented Mar 1, 2021

I just had a look at the source for this in InPlaceActivate(OLEIVERB verb). There is a call to

HRESULT hr = _inPlaceUiWindow.GetWindow(&hwndParent);

at a location in the code where _inPlaceUiWindow has never been instantiated. This piece of code cannot work anywhere.

@1R053
Copy link
Contributor

1R053 commented Mar 1, 2021

created a PR #4626
imo it was a copy & paste error when porting this code.
In case this goes through, will it be backported to 5.x or should this be a separate PR?

@ghost ghost added the 🚧 work in progress Work that is current in progress label Mar 4, 2021
@ghost ghost removed the 🚧 work in progress Work that is current in progress label Mar 4, 2021
@RussKie RussKie added the 💥 regression-preview Regression from a preview release label Mar 4, 2021
@RussKie RussKie reopened this Mar 4, 2021
@arknu
Copy link
Author

arknu commented Mar 4, 2021

@1R053 Thank you for creating the PR. It certainly fixes the NullReference exception for me. However, there appears to be other issues resulting in the ActiveX control not being created - at least when running in Office (via the ICTPFactory.CreateCTP method). I don't get any exceptions or details, just "The ActiveX object could not be created". However, those issues are probably best discussed in a separate issue, which I'll create once I have done some more investigation.

@RussKie Are you interested in having ActiveX hosting work correctly in .NET 6? If so, I'll be happy to work with you to test this. I have a proof-of-concept Outlook addin, which I can provide the code for, if needed. My specific scenario is Office, as stated previously. I suspect this will be the most common case these days, but I don't think any of these issues are Office-specific.

@1R053
Copy link
Contributor

1R053 commented Mar 4, 2021

@arknu Thanks - I've observed this as well. It would be great if you can assist in identifying the next issues.

@1R053
Copy link
Contributor

1R053 commented Mar 4, 2021

after the fix I think the class type of the control cannot be identified somehow.
When looking into Internal.Runtime.InteropServices.ComActivator->FindClassType there seems to be no type returned in my case

@arknu
Copy link
Author

arknu commented Mar 4, 2021

@1R053 For me it actually gets as far as calling the constructor of the UserControl now. Definitely progress. But after executing the constructor successfully, it fails. I have a feeling that there may be another copy-paste bug lurking somewhere...

@1R053
Copy link
Contributor

1R053 commented Mar 4, 2021

@arknu
Can you register/unregister the comhost.dll with Core 6? I had also issues here.
If it was already registered before I also had a successful instantiation but no success with ICTPFactory.CreateCTP

@RussKie
Copy link
Member

RussKie commented Mar 4, 2021

Are you interested in having ActiveX hosting work correctly in .NET 6? If so, I'll be happy to work with you to test this. I have a proof-of-concept Outlook addin, which I can provide the code for, if needed. My specific scenario is Office, as stated previously. I suspect this will be the most common case these days, but I don't think any of these issues are Office-specific.

Our current team is lacking in understanding how Office interop and ActiveX are used in the wild, and we're very interested in sample apps that can demonstrate real scenarios. Having these scenarios we'll help us building a test suite and identifying gaps, and may aid in cross-org discussion about the future of the interop.

I can't make any promises on the future support of VSTO and Office interop, but the more we know how our users use (and abuse :)) this area - the more we may be able to help.

@arknu
Copy link
Author

arknu commented Mar 5, 2021

@RussKie As for my use case, I'm currently attempting a proof of concept to determine whether whether we can do an Outlook addin without VSTO, since there apparently no desire to update VSTO (which is a shame!). We currently have a fairly complex commercial VSTO-based addin, and no desire to be stuck on a dead .NET Framework with outdated libraries - and no desire to use a subpar language like JS. OfficeJS is not even remotely capable of doing what we need. So we are looking to do an addin using raw COM APIs directly from .NET. This should be possible and mostly is. The only issue is showing a task pane, which uses a WinForms UserControl hosted as an ActiveX object. So I have created a .NET 5-based COM addin that works perfectly - except for showing the task pane, where we run into various issues, as detailed above.

So if we get these issues fixed, we will be using Office interop and COM for a full-fledged commercial Outlook addin.

@RussKie
Copy link
Member

RussKie commented Mar 9, 2021

If you have any repros to share but don't want those to be public please feel free to email me at Igor.Velikorossov at microsoft.

@arknu
Copy link
Author

arknu commented Mar 9, 2021

@RussKie What I have is a proof of concept, so I have no issues sharing it publicly. I'll upload it to GitHub later this week and post a link here - I just need to clean it up slightly first.

@arknu
Copy link
Author

arknu commented Mar 13, 2021

@RussKie As promised, I have now uploaded my proof-of-concept to a GitHub repository: https://github.com/arknu/OutlookAddinNet5. It is pretty much as simple as can be - adding a few buttons to the ribbon and attempting to show a taskpane with a UserControl. This last part being what fails.

It is currently based on .NET 5, haven't tried the .NET previews yet.

I'm running it with a private patched build of WinForms, which includes @1R053's patch for the NullReferenceException.

@weltkante
Copy link
Contributor

I'll have a look

@1R053
Copy link
Contributor

1R053 commented Mar 16, 2021

if this does not work, it would be a big issue imo. Since this would mean that two addins from two vendors that are usually not synced with respect to their .NET version would crash each other.

@weltkante
Copy link
Contributor

See for example dotnet/runtime#12018 (which is quite old) so its probably worth to ask again and present your usecase.

if this does not work, it would be a big issue imo

definitely, you'd risk your addin not being loadable by relying on .NET core if someone else gets loaded earlier

would crash each other.

I'd hope it'd just not load, not crash the Office process. If it crashes it may be worth fixing that crash independently from adding side-by-side support.

@1R053
Copy link
Contributor

1R053 commented Mar 16, 2021

Maybe a custom shim instantiating the runtime with a custom assembly context could work
https://docs.microsoft.com/en-us/dotnet/api/system.runtime.loader.assemblyloadcontext?view=net-5.0

does somebody have an example for a custom shim with .NET Core?

@weltkante
Copy link
Contributor

weltkante commented Mar 16, 2021

Maybe a custom shim instantiating the runtime with a custom assembly context could work

No at the end of the issue I linked above they mention they have been using global resources. If they didn't change that yet it won't work at all.

You'd need to move your addin out of process to make the runtime resolve uniquely, and have a C++ COM addin as a shim (or any other language that can load side-by-side)

We actually do something very similar for different reasons, on Desktop Framework WinForms already is a shared resource and we ran into problems because other addins may have loaded the same 3rd party WinForms controls we use before us with a different global configuration.

Therefore we moved all our UI into an out-of-process COM object, keeping an in-process addin to communicate with Office and shim all calls that do UI to our out-of-process objects. This is nontrivial due to how COM behaves when crossing apartmant boundaries, especially message pumping happening to wait for results of the call requires low level workarounds (you don't want message pumping when the ribbon is being loaded for example, so we do the out-of-process COM calls on the thread pool, with special non-pumping code to wait for results, being careful that we don't have deadlocks).

If we were to port to .NET Core we could probably update that approach and make the in-process addin C++ only until .NET Core actually supports loading side-by-side (though I don't know if we would take that step, considering we prefer coding in C# where possible)

@weltkante
Copy link
Contributor

weltkante commented Mar 16, 2021

Personally I'd suggest opening another issue on the runtime, I mentioned COM addins as a major problem in the linked issue, and missed when that issue got closed later. Having the runtime unable to load side-by-side basically makes it useless for implementing COM objects.

@1R053
Copy link
Contributor

1R053 commented Mar 16, 2021

You'd need to move your addin out of process

@weltkante do you have a C++ example project that does this with .NET Core? That could be the base for a custom shim if I understand you correctly.

@arknu
Copy link
Author

arknu commented Mar 16, 2021

@weltkante Thank you for all your effort on this. I have now filed dotnet/runtime#49686 for discussing the issue of multiple versions of .NET with COM hosting.

@arknu
Copy link
Author

arknu commented Mar 16, 2021

To get a little more back on topic, I have now created a .NET 6 version of this proof-of-concept. It loads OK (once you disable the .NET 5 version), but the UserControl in the task pane is non-interactive. It looks like mouse messages or something are not being processed correctly. It loads fine, but does not react to mouse movement or clicking the button. It works in .NET 5. Should I file a new issue for this problem or is it perhaps related to the known issue with PropertyGrid for .NET 6 preview 2?

@weltkante
Copy link
Contributor

weltkante commented Mar 16, 2021

@weltkante do you have a C++ example project that does this with .NET Core?

No, nothing finished anyways, ours is currently C# Desktop Framework (which is able to load side-by-side) but it wouldn't be too hard to create one, its just a bit of work. You'd probably use ATL or WRL to implement those COM objects and IDL to define the interfaces (you need a TLB to have COM talk cross-process, which .NET does not support generating anymore, so you have to define all out-of-process accessible interfaces in IDL you compile to a TLB and [ComImport] them, instead of using [ComVisible(true)]).

I might consider making an example but it would probably take a while (time scale would probably be more than a week but less than a month)

It loads fine, but does not react to mouse movement or clicking the button.

I believe that worked for me when I tested, but I may mix up memory of testing in the MFC host which I also tried. Also note that there still is #4685 returning wrong flags to Outlook if you didn't patch it (I did in my tests) so it may be related to that regression.

I'll look at it again later today.

Should I file a new issue for this problem

Not speaking for the team (which may have a different opinion) but I'd probably just rename the issue to 'hosting UserControl via ActiveX does not work' and we should try getting the basic functionality working in this issue. Makes not much sense to leave it in a partially working state, we should try to get the basics functional again.

@arknu arknu changed the title NullReferenceException when instiating UserControl as ActiveX Object Hosting UserControl via ActiveX does not work correctly Mar 16, 2021
@arknu
Copy link
Author

arknu commented Mar 16, 2021

@weltkante I agree, I've renamed the issue accordingly.

I haven't applied your latest patch to .NET 6 yet, so that may certainly be related. That said, I didn't apply it to my custom .NET 5 build and that works fine...

@1R053
Copy link
Contributor

1R053 commented Mar 16, 2021

Thanks @weltkante - I think the COM / IDL side I could manage. What would help would be to understand how the equivalent of metahost->CLRCreateInstance, metahost->EnumerateInstalledRuntimes or metahost->GetRuntimes is properly handled for .NET Core in a C++ project.

@weltkante
Copy link
Contributor

weltkante commented Mar 16, 2021

What would help would be to understand how the equivalent of metahost->CLRCreateInstance, metahost->EnumerateInstalledRuntimes or metahost->GetRuntimes is properly handled for .NET Core in a C++ project.

You don't need to, out of process hosts are an .exe so you can just make a normal .NET exe and it'll activate .NET Core itself.

You specifically wouldn't want to load the .NET runtime in the C++ shim since it'd fail anyways if someone already loaded a different version. The C++ shim would just mediate between COM objects (Office and your out-of-process COM objects) and not need to be aware of .NET at all.

PS: if you want to discuss more of that I'm also on gitter, might be getting a bit off topic for this issue.

@merriemcgaw
Copy link
Member

Is there anything else we can do with this issue? Is there more work that we need to do?

@RussKie RussKie added the 📭 waiting-author-feedback The team requires more information from the author label Aug 11, 2022
@RussKie RussKie modified the milestones: .NET 7.0, Future Aug 11, 2022
@weltkante
Copy link
Contributor

weltkante commented Aug 11, 2022

Personally I'd suggest to move "implementing ActiveX controls in WinForms and hosting them in external environments" into "unsupported" territory

  • doing it properly requires generating a type library (TLB) which is no longer supported by the dotnet ecosystem. You can get it to work by using the Desktop Framework to compile against a similarly shaped API and generate the TLB, then use that TLB with your .NET Core code (is that the current terminology?) But this is already doing things on your own, the tooling itself doesn't support it.
  • in the Office scenario vital undocumented infrastructure was removed/lost in the migration process and there is no intention to restore it (as mentioned in other issues), so hosting user controls in Office this way will not be supported by your roadmap regardless of what you do with ActiveX controls
  • the non-desktop dotnet runtime doesn't support side-by-side hosting, making COM a bad API/ABI to base new controls on, as multiple different such controls cannot coexist inside the same process

PS: the reverse scenario, hosting external ActiveX controls inside a WinForms control or form, is supportable.

also if there are bugs reproed/identified they can still of course be fixed, but the overall scenario itself cannot be supported given the current capabilities of dotnet, its always going to be a hack job for the one using this capability

@1R053
Copy link
Contributor

1R053 commented Aug 11, 2022

What is then the recommended architecture to host user controls in taskpanes of Office COM addins?

@weltkante
Copy link
Contributor

weltkante commented Aug 11, 2022

Office COM addins are no longer supported in context of .NET Core, and there's no plan to restore support

So if this is a requirement you have to remain with Desktop Framework or chose a different language like C/C++ for implementing the panel.

See also here and here for mentioning that VSTO is discontinued and will not be updated for dotnet core. Generally the Office team doesn't seem to like that people still use these old technologies.

I don't like the situation myself, as I maintain a COM Office Addin myself, but I can only report what the current situation is.

@RussKie
Copy link
Member

RussKie commented Aug 11, 2022

Adding @AaronRobinsonMSFT for general awareness.

@1R053
Copy link
Contributor

1R053 commented Aug 11, 2022

Office COM addins are still supported by MS Office and I do not expect that to go away soon, especially since the JavaScript based Office addin infrastructure lacks relevant features for more advanced scenarios.
That .NET Core does not want to support it is very unfortunate for this not unimportant use case.
The lack of multiple runtime support also means probably, that any vendor trying to establish an add-in ecosystem cannot really use .NET Core to develop their software, as they inevitably would run into similar issues. That seriously limits .NET Core usefulness for more advanced scenarios.

Anyway, thanks for pointing that out.

@ghost
Copy link

ghost commented Aug 26, 2022

This submission has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 14 days.

It will be closed if no further activity occurs within 7 days of this comment.

@ghost ghost closed this as completed Sep 2, 2022
@ghost ghost removed this from the Future milestone Sep 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-COM area-Interop 💥 regression-preview Regression from a preview release 📭 waiting-author-feedback The team requires more information from the author 💤 no-recent-activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants