Latest Notepad++ replaces "Pin to Quick access" in the Windows 11 menu

[GurliGebis]

Description of the Issue

The latest version of Notepad++ replaces the "Pin to Quick access" in the Windows 11 right click menu.
This is due to it being added in the wrong way.

Instead of just overwriting something else, it should be added like described here:
#13170 (comment)

The correct way to do it is to use Sparse Manifests: https://blogs.windows.com/blog/2021/07/19/extending-the-context-menu-and-share-dialog-in-windows-11/

Also, please don't close this bug as "expected behavior", since you are not supposed to just overwrite the built-in menu options to add your own.

Steps to Reproduce the Issue

1. Install the latest Notepad++ version
2. Watch as it takes over the "Pin to Quick access" menu option

Expected Behavior

It shouldn't overwrite the default Windows behavior, but should instead add it's own menu option to the list.

Actual Behavior

It takes the easy (and wrong) way out, and just replaces the existing menu item, instead of doing it the right way.

[wallrik]

Please consider rolling back release 8.5 until this is fixed. 😣 Also, people have left comments on the commit and other places as well.

Rolling back the release would stop more people from getting messed up installs, and save a lot of headache.

[riverar]

@donho Would consider rolling back a003115

[ErikHumphrey]

Doesn't seem necessary to do it the way it's currently implemented. It is toggleable, however.

[donho]

Watch as it takes over the "Pin to Quick access" menu option

It's not true. this entry will be untouched if it has been taken.

Stop sharing the following useless link with me - it says it "should do" but not says "how to"
https://blogs.windows.com/blog/2021/07/19/extending-the-context-menu-and-share-dialog-in-windows-11/

If you have the concrete solution, please let me know, or you can do your contribution by opening a PR - it's an open source project, remember?

For people who don't want the context menu, just uncheck Context Menu Entry during the installation:

[donho]

If you have the concrete solution ("correct" way to install context menu entry manually), please let me know, or you can do your contribution by opening a PR.
It will be in the next release if it's accepted, but v8.5 won't be rolled back.

[riverar]

@donho Sounds fair, thanks!

[GurliGebis]

I think this is what is needed: It explains how to create the needed Sparse Package with an example: https://blogs.windows.com/windowsdeveloper/2019/10/29/identity-registration-and-activation-of-non-packaged-win32-apps

From what I can see, registrering it should not require too many changes.

[sylveon]

There's a full example here: https://github.com/microsoft/AppModelSamples/tree/master/Samples/SparsePackages/PhotoStoreContextMenu

[GurliGebis]

Also, there is a clean C++ example for VS Code here: microsoft/vscode#139640 - the one above requires C# it seems

[sylveon]

The context menu project in the sample I linked is all C++, but yeah that's also another good example.

[valinet]

@donho Why do you think it is okay to hijack the user’s preferences without notifying, at least? There is no mention of this app replacing some other entry in the context menu. Even if there was, it’s still a poor choice to do it like this when there’s documentation and examples available, you just have to look up. This is downright malware behavior - antivirus vendors better flag this release as such. This is unacceptable, the fact that you dare to ship such a mess brings a lot of question marks above the credibility and trust for this project going forward. There are already plenty comments advising not to push forward with this, yet you did it anyway.

I will uninstall this product for the time being, this is unacceptable.

[ToothyDev]

For everyone whose registry has already been messed with, deleting the HKLM\SOFTWARE\Classes\*\shell\\pintohome will make N++'s changes undone. This key doesn't exist in Windows by default and is created by N++, and thus safe to delete

[GurliGebis]

I have started working on implementing this, though nothing is even close to ready yet (the DLL compiles, but that is all - I haven't had time to finish it yet, and we also need to manifest and package files)

[Cmdr-Keene]

I really expected better from a project like this, than to hijack system entries in unsupported hacky methods.

[riverar]

@LtCmdrKeene Well to be fair, Microsoft didn't exactly document this convulted process properly, outside from a few random blog posts and some complex samples. Seems like the author had pure intentions here. Maybe at the MVP Summit or your PGIs you can discuss how terrible this process is.

[valinet]

@riverar Still, this is not a proper way of going about it. Period. As I said, this is malware behavior. Things indeed may be poorly documented or not documented at all sometimes. Trust me, I know how frustrating it feels. But just because someone does something bad, that doesn’t make it okay to do something else equally bad or even worse.

[donho]

@valinet
It's an open source project so you are welcome to do your contribution, instead of whining.
Or you can use v8.4.3 Unhappy Users' Edition and get a refund.

[GurliGebis]

Okay, I'm not normally the one doing this, but can we please get back on track with fixing the issue (see my PR above), instead of throwing things at each other and acting like children?
All this "he said, she said" isn't helping anyone.

[valinet]

@valinet It's an open source project so you are welcome to do your contribution, instead of whining. Or you can use v8.4.3 Unhappy Users' Edition and get a refund.

Inexplicably, you failed to understand what I wrote. For example, being open source doesn't make malware less "malwarey". The behavior in the latest update is by all definitions malware. Also, I was not questioning or "whining" about the lack of such feature - I use Windows 11 and live just fine without said feature. So yeah, please pay more attention and actually read what people write before throwing non-sense around. Since you mention it, free software means anything but non-transparency, disregard of feedback and user preferences and malware-like behavior, which, as I said, is exactly what this here is all about. My 2 cents regarding your remark.

[donho]

@GurliGebis
Thank you for your coding effort - very appreciated.
The non-ARM64 version (64 bits) has been tested - it works like charm.
I'm preparing the package to include your implementation into v8.5.1 release. The RC should be out this night or tomorrow.
Thank you again for your contribution!

[GurliGebis]

@donho you're welcome 🙂
Quick question, how do we cleanup for the people that has installed 8.5.0?
Their registry will have those keys, replacing the pin to quick access.
One way might be added a check on startup, and then use that to cleanup.

[donho]

@GurliGebis

Quick question, how do we cleanup for the people that has installed 8.5.0?

Via installer, this entry will be checked under Windows 11 - if it exists and, and contain the "Notepad++", then it'll be removed (even it's done by user manually). And the new "Edit with Notepad++" (your implementation) will be installed.
See 5645ca1

[GurliGebis]

@riverar Okay 🙂
@sylveon Would you be able to create a PR that sets that up?

[sylveon]

Unfortunately I am too busy to do so myself, but I imagine a lot of the infra for the previous DLL can be reused

[GurliGebis]

@sylveon if you can point me at some documentation on it, I might be able to take some time to look at it. (I haven't been able to find any, that's why I'm asking)

[sylveon]

From a quick look, it's pretty much exactly what NppShell does. It's all documented here.

A full example would then end up being (replacing placeholders of course)

HKEY_CLASSES_ROOT
   CLSID
      {00000000-1111-2222-3333-444444444444}
         InProcServer32
            (Default) = C:\MyDir\MyCommand.dll
            ThreadingModel = Apartment
   *
      Shellex
         ContextMenuHandler (this is the same for both IContextMenu and IExplorerCommand)
            ANotepad++64 (this is what NppShell uses)
               (Default) = {00000000-1111-2222-3333-444444444444}

So my suggestion would be to:

• Migrate the IExplorerCommand implementation to the NppShell project.
• Update the IExplorerCommand implementation code to honor existing settings registry keys (you can probably look at how the IContextMenu implementation does it).
• Remove the old IContextMenu implementation but re-use its CLSID for the new IExplorerCommand implementation.
• Migrate RegisterSparsePackage and UnregisterSparsePackage as well and export it from the DLL.
• Remove NppModernShell project.
• In the installer, when registering NppShell, detect Windows 11 and if so call RegisterSparsePackage instead of DllRegisterServer (and the other way around for uninstallation)

This seems a lot cleaner (no duplication of functionality, can't end up with two "Edit with Notepad++" entries, installer can treat this as a normal update of NppShell, etc.)

Also, to avoid needing to restart Explorer after registration, one needs to call SHChangeNotify with SHCNE_ASSOCCHANGED. This can probably be done within RegisterSparsePackage, UnregisterSparsePackage, DllRegisterServer and DllUnregisterServer.

[GurliGebis]

@sylveon thanks, and I agree 🙂
@donho you want me to give it a go?

[donho]

@GurliGebis
Yes, it'll be great to have only one dll to maintain.

[GurliGebis]

@donho okay, I'll take a look at it - would it be okay if I migrate it into the new project instead?
That way, it would be less moving around, since everything is in there, except for the registry registration.

I also have an idea for changing the Register/Unregister functions to new names, and have them do this:

• Detect which windows version we are on.
• If Windows 11, unregister using the old code on register and register the new way.
• If lower than Windows 11, register in the registry instead.
• Uninstall unregisters both.

That way, installation just has to call the register function, and it handles it correctly, depending on which Windows version you are installing on.

Would that be okay?

[GurliGebis]

By that I mean moving the new project into the NppShell folder and renaming stuff

[donho]

@GurliGebis

That way, installation just has to call the register function, and it handles it correctly, depending on which Windows version you are installing on.

Would that be okay?

Yes, sounds good to me.

By that I mean moving the new project into the NppShell folder and renaming stuff

I would say it deserved to be a independent project in Notepad++ organization.
What do you think?

[GurliGebis]

@donho it is too deeply integrated for that to make sense.
On its own, it doesn't serve a purpose, and it is too project specific to be easily used with something else.
I plan on making the unregistration do all the cleanup, so the installer can be cleaned up a bit🙂

[ozone10]

@GurliGebis
IMHO it should be independent project.
Modifying "NppModernShell" source code has no effect on notepad++.exe binary, appveyor is not building it and it blocks appveyor.

And we can use git submodules to include it here.

btw
It can also help other to learn how to implement Windows 11 context menu since it can be easier to find, instead of buried deep inside Notepad++ main repository.

[GurliGebis]

@ozone10 you got a compelling argument 🙂
I'll refactor it in this repo, and we can then look at moving it into another repo and add it back as a submodule.
@donho would that be okay with you?

[donho]

@GurliGebis

I'll refactor it in this repo, and we can then look at moving it into another repo and add it back as a submodule.
@donho would that be okay with you?

It sounds good to me - please mark it as work in progress as you did so it won't trigger the CI's compiling.

[GurliGebis]

@donho I will do.
@sylveon If I try and register the handler in the old menu with a IExplorerCommand, the object gets created just fine, but it is not added to the "classic" right click menu.
Any ideas on why that might be? (There seems to be absolutely 0 documentation on how to use IExplorerCommand for anything but APPX/MSIX packages.)

[donho]

@GurliGebis

Any ideas on why that might be? (There seems to be absolutely 0 documentation on how to use IExplorerCommand for anything but APPX/MSIX packages.)

Absolutely no idea.
I cloned long time ago the project which was the menu entry shell for SciTE:
https://burgaud.com/scite-context-menu

With the permission of Andre Burgaud (the author), I've modified the project slightly for meeting Notepad++'s need. You might find some document in the website or you may write to the author.

[sylveon]

There is documentation from the Windows 7 days mentioning IExplorerCommand for menus: https://learn.microsoft.com/en-us/windows/win32/shell/how-to-create-cascading-menus-with-the-iexplorercommand-interface

It says:

In Windows 7 and later, you can provide the same verb implementation using the IExplorerCommand interface as you can with the IContextMenu interface.

So I suspect something somewhere else is broken.

There's an old sample also from that era here, if that helps: https://github.com/microsoft/Windows-classic-samples/blob/main/Samples/Win7Samples/winui/shell/appshellintegration/ExplorerCommandVerb/ExplorerCommandVerb.cpp

[GurliGebis]

I think I have figured it out - I'll take a look at it later.
If my theory is correct, then it is facepalming time 😃

[mike000000000001]

My God, this program is a horror. Can't be uninstalled completely, leaves context menu doubled up. Uninstalling for good.

[GurliGebis]

@mike000000000001 well, that is what we are working on fixing, but sure, you are welcome to uninstall if that is what is right for you.
@donho and @sylveon Okay, I got a small demo project working, so now I know what changes are needed - yay 🙂

[alankilborn]

@mike000000000001 said:

Uninstalling for good.

Don't forget to apply for your refund by filling out the refund FORM on your way out...

[GurliGebis]

@donho I got it working - it seems like skipping WRL is not an option when the IExplorerCommand should work for both classic and modern context menus.
I'll make a PR for you with updated guide on building later today 🙂

[donho]

@GurliGebis
Sorry, I don't follow you - what's WRL ?

[GurliGebis]

@donho Windows Runtime Library - develop ment has shifted to WIL (Windows Interface Library), but both are fine to use.
WRL is the "old" way of writing COM object, but it seems it is needed here (the sample for sparse packages is also using it, so it is what Microsoft themselves uses)

[donho]

@GurliGebis Thank you for the explanation.
As far as the binary not over 2 MB, there's no problem to me :)

[GurliGebis]

@donho they are pretty small - the size difference on my machine on the x64 DLL is only 5kb larger than the old NppShell dll.

I have create a PR now with the changes, if you can take a look at it (and maybe create an installer for me to test with)? 🙂
#13389

@sylveon thank you for helping with this one, your notes above helped a lot with finding the right place to look.

[donho]

The thread becomes huge so let's talk in #13389 then.

[GurliGebis]

@donho I agree, can you lock this issue?