nppShell
nppShell copied to clipboard
WORK IN PROGRESS - Install for all users
Changes for installing for all users
@ddomingos-encora Would you be able to take a look at this, and see if you can spot what I'm doing wrong? :)
It seems it is all good. I'll try to test it once I restore my system (probably tomorrow). I'll let you know
@GurliGebis In the other discussion, you mentioned this: "The Get-AppxPackage cmdlet shows it as installed, but it isn't registered". What field tells you it is not registered? I did not find such field. Also, do you see any difference in the output of the Get-AppxPackage command when you install the official Notepad++ package and when you install the package generated by the code in this PR?
@ddomingos-encora if you run that command, and it shows up in the list, it is installed 🙂 So that is what I meant by it showing it as installed. What I meant was basically, it is installed, but it doesn't look like it is working (no dllhost.exe process is spawned when a file is right clicked, so the code isn't even being run)
@ddomingos-encora could it be an ACL issue? (Just thinking aloud)
Hi @GurliGebis I managed to restore my system and test today. It "works" but requires a restart or sign off/sing in. Since the installation of notepad++ does not require a restart, it is probably not a final solution. Still, you might want to keep both this logic and the ensure registration logic, since the ensure registration logic also has a blind spot of not displaying the menu the first time when that code is executed (if was not installed before), so the chance of that happening for other users is reduced. At least the remove for all users flag I think you should keep. If you want to keep the provisioning code as well, I'd use both staging/provisioning and add code, so that it gets installed for the user running the installer. One idea could be: instead of changing the current code for registering, we would add a new function for provisioning. Then the installation path would call both but the ensure registration logic would call only the one for registration (as today). Let me know what you think. Regards
I totally agree @ddomingos-encora 🙂 I'll update the PR with it added back, can you take a look at it afterwards and see if it looks fine?
@ddomingos-encora There is a problem - it works fine for the first user now.
However, since the package has been provisioned, and is installed on all users - the EnsureRegistrationOnCurrentUser function thinks it is installed (which it technically is), and does nothing.
Any ideas?
Yes, I thought about this later yesterday, after I posted. I was wondering if there could be a difference between finding the package for all users or the current user but I just saw you already implemented this way. The only thing I can think of now would be the Package class itself: https://learn.microsoft.com/en-us/uwp/api/windows.applicationmodel.package?view=winrt-22621 Maybe some functions like Status can be helpful, or even InstalledDate. Maybe one of them can indicate us that the package is staged but not actually installed. I'll do some tests later today
Please do. From what I could come up with (didn't have long to test, so didn't spend a lot of time on it) - it just looks to be installed correctly, but not loading for some reason. I even tried setting "Everyone" to "Full control" on the folder, to ensure it isn't ACL related, but that doesn't seem to make any difference.
Look at this explanation here: https://github.com/microsoft/WindowsAppSDK/discussions/2099#discussioncomment-2392347 It seems we are doing the right thing. There is another comment where the same guy says that for the current user we should also Add (besides staging and provisioning). The only thing that is not clear to me is why the other users are not getting it installed.
I did a few more tests but could not find an attribute that could help us to differentiate that situation, nor even establish when the package gets installed to other users. Sometimes it seems to require a restart, sometimes it doesn't get installed at all. It seems just buggy to me. Unfortunately, I gave up. I think we should change the title of the PR to uninstall to all users and keep only the RemoveForAllUsers flag. Regards
@GurliGebis I was doing some tests and for a moment I thought that the RemoveForAllUsers flag was not working but it is. I think this is a good improvement we can take out of all these attempts. Regards
@ddomingos-encora I think so as well. I wonder if it works better on the current insider canary builds of Windows 11, since they might have made changes. I'll have to test that later.