Comment 1 Samuel Mannehed 2024-07-17 15:40:33 CEST

Windows' add & remove programs looks at the registry HKLM Software\Microsoft\Windows\CurrentVersion\Uninstall\tlclient. More info in NSIS documentation:

https://nsis.sourceforge.io/Add_uninstall_information_to_Add/Remove_Programs#Computing

Comment 3 Samuel Mannehed 2024-07-17 15:41:26 CEST

Created attachment 1212 [details]
Screenshot of Add & Remove programs after patch

Comment 4 Samuel Mannehed 2024-07-19 12:39:45 CEST

(In reply to Samuel Mannehed from comment #0)
> This looks bad, and also causes our update to the Microsoft Store to fail:
> 
> > Products need to clearly communicate how they can be removed.
> > The Add or Remove Programs entry for your product shows a blank or unrelated Name or Publisher.
> > Please update your product so users can correctly identify it.

A 4.17.0 client with the patch in attachment 1211 [details] applied was built and successfully uploaded to Microsoft Store.

Comment 6 Samuel Mannehed 2024-07-19 14:23:01 CEST

Created attachment 1213 [details]
Screenshot of old Add & Remove programs UI after patch

On Windows 10 you can open the "Add and remove programs" from the Controlpanel, and get the old interface. This interface shows a bit more information, for example the UrlInfoAbout and HelpLink fields.

Comment 11 Samuel Mannehed 2024-07-19 14:41:58 CEST

I split the patches into four pieces, since they add four different things for different purposes.

Comment 12 Samuel Mannehed 2024-07-22 17:59:16 CEST

(In reply to Samuel Mannehed from comment #1)
> Windows' add & remove programs looks at the registry HKLM
> Software\Microsoft\Windows\CurrentVersion\Uninstall\tlclient. More info in
> NSIS documentation:
> 
> https://nsis.sourceforge.io/Add_uninstall_information_to_Add/
> Remove_Programs#Computing

As seen in the above link, very few of these registry keys are actually required for the uninstaller. Microsoft Store adds the "Publisher" requirement.

For a complete list of available registry keys for Add/Remove Programs (ARP) UI, see these two pages:

https://learn.microsoft.com/en-us/windows/win32/msi/configuring-add-remove-programs-with-windows-installer
https://learn.microsoft.com/en-us/windows/win32/msi/uninstall-registry-key

However, testing shows that the ARP UI on Windows 11 will only show these things:

 * Icon
 * DisplayName
 * DisplayVersion
 * Publisher
 * InstallDate
 * EstimatedSize

From the attached patches on this bug, the odd-men out are UrlInfoAbout and HelpLink. These fields are not shown in Windows 11 ARP.

I added them because it was simple, and they are likely useful to users on Windows 10.

Comment 13 Adam Halim 2024-08-01 10:45:30 CEST

Many properties are listed in [1, 2], and the ones added by the patches are:
* Publisher
* DisplayVersion
* UrlInfoAbout
* HelpLink
* EstimatedSize

The documentation for how exactly these values are used by windows is poorly
documented, but I found an interesting dev blog (from 2004!) [3] which mentioned
that if "EstimatedSize" is set explicitly, Windows won't do any magic to try
and guess the program's size.

Searching for "EstimatedSize" in their documentation gives two results, [2] and
[4], which don't provide much detail.

Beyond "EstimatedSize", the other added options look good, but I think the URLs
for "UrlInfoAbout" and "HelpLink" should be switched? These properties
translate to:
UrlInfoAbout -> Link to support: https://www.cendio.com
HelpLink     -> Link to help:    https://www.cendio.com/thinlinc/support

I looked into what other programs do, and there doesn't seem to be a clear
consensus, and most apps don't even list these. Here are some apps that populate
several fields:

* Firefox:
    Product version:    128.0.2
    Help link:          https://support.mozilla.org
    Support link:       https://www.mozilla.org
    Update information: https://www.mozilla.org/firefox/<version>/releasenotes
    Size:               243 MB
    Comment:            Mozilla Firefox 127.0 (x64 sv-SE)

* Notepad++
    Product version:    8.1.4
    Help link:          https://notepad-plus-plus.org/
    Support link:       https://notepad-plus-plus.org/
    Size:               12,8 MB

* GIMP
    Product version:     128.0.2 
    Help link:           https://www.gimp.org/docs/
    Support link:        https://www.gimp.org/
    Update information:  https://www.gimp.org/
    Size:                1,19 GB

* WinSCP
    Product version:    5.17.5
    Help link:          https://winscp.net/forum/
    Support link:       https://winscp.net/
    Update information: https://winscp.net/eng/download.php
    Size:               96,6 MB    

From the properties listed in [1, 2], we might want to consider adding
"UrlUpdateInfo". I think it would be cool to add a URL to the release notes.

Three other interesting ones are "Description", "Comment" and "Contact". I
don't really see the value in adding anyone of these, except for maybe "Contact".

On Windows 10, the "Publisher" property is not listed in the ARP view, but is
seen in the old control panel view. The property is displayed correctly on
Windows 11.

[1] https://learn.microsoft.com/en-us/windows/win32/msi/configuring-add-remove-programs-with-windows-installer
[2] https://learn.microsoft.com/en-us/windows/win32/msi/uninstall-registry-key
[3] https://devblogs.microsoft.com/oldnewthing/20040709-00/?p=38493
[4] https://learn.microsoft.com/en-us/windows/win32/msi/fastoem

Comment 14 Adam Halim 2024-08-01 10:53:32 CEST

Tested applying all patches and ran into an error building the client
customizer. The GetSize function needs to be included from FileFunc.nsh, so I
simply included it.

On Windows 11, I could see all properties listed properly in both the ARP UI,
and in the old control panel view.

On Windows 10, I could not see the Publisher in the ARP view, but the version
and size were displayed as expected. All fields were visible in the old control
panel view, identically to Windows 11.

Comment 15 Adam Halim 2024-08-01 12:34:43 CEST

Sanity checked the size value, and it was indeed identical to the size of the
install directory (C:\Program Files\ThinLinc Client).

I also tested writing bogus hard-coded values in the NSIS file for
EstimatedSize and saw that the value was reflected when installed.

In the registry [1], the values were identical between Windows 10 and 11, and
it is difficult to tell why the Publisher isn't listed on Windows 10. The only
thing I could find was a Stack Overflow post [2] where someone mentioned that
Publisher is only listed when installing an MSIX/APPX package.

I tested installing our 4.17.0 client from the Microsoft store, which is
signed, and it also did not show the Publisher.

[1]Computer\HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\tlcustomizer
[2] https://stackoverflow.com/questions/57432858/show-publisher-name-in-list-of-installed-programs

Comment 16 Adam Halim 2024-08-01 14:10:24 CEST

Looking at the software listed in "Apps & Features" on Windows 10, only
UWP-apps had a publisher (and an "Advanced options" setting). It seems like
Microsoft changed it in Windows 11 so that the publisher is listed regardless
whether the program is a UWP-app, or a desktop app.

Comment 21 Adam Halim 2024-08-02 10:39:28 CEST

Tested installing client & customizer 4.17.0 on Windows 10 and 11, and then
upgrading to 4.17.0post with the patches applied. After the upgrade, I could
see that all added fields were displayed in both the new and old uninstall UI,
on both Windows 10(*) and 11.

Uninstalling the client/customizer removes all uninstall entries in the
registry.

A clean installation of 4.17.0post with the patches applied also yields the same
results.

When downgrading from 4.17.0post to 4.17.0, the fields added by these patches don't get removed, so if someone in the future downgrades from 4.18.0 to 4.17.0 (or older), the new fields won't get removed. For example, it will still say version 4.18.0 (and the old file size) in the UI. Since we can't go back in time and change our old installers, there is nothing we can do about this. The consequences are not severe, and downgrades are probably not that commit, so this should be fine.

(*) Except for Publisher in the "Apps & Features" on Windows 10, as explained in 
    previous comments.

Comment 22 Adam Halim 2024-08-02 10:40:39 CEST

(In reply to Adam Halim from comment #21)
> The consequences are not severe, and downgrades are probably
> not that commit, so this should be fine.
not that common*

Comment 23 Adam Halim 2024-08-02 10:48:00 CEST

> MUST:
>   ✅ The ThinLinc client must be able to be
>      uploaded to Microsoft Store without
>      failing.
Indeed, we have a version with these patches live on Microsoft Store.
> SHOULD:
>   ✅ The ThinLinc client/customizer installation 
>      should behave similarly to how other Windows
>      programs behave when installed.
What to write in some of the added fields, "UrlInfoAbout" and "HelpLink in particular, is inconsistent between different Windows applications. Reading Microsoft's documentation [1], I think the strings we display are in line with the descriptions from Microsoft. Some other applications also follow this, while others have chosen to use them more freely. I don't think we do anything unconventional, while at the same time we could be more liberal (for example switching the strings for "UrlInfoAbout" and "HelpLink", or maybe having a URL to our TAG/community).

Overall, the changes look good.

[1] https://learn.microsoft.com/en-us/windows/win32/msi/uninstall-registry-key

Comment 24 Alexander Zeijlon 2024-08-13 10:21:13 CEST

I have looked through the commits, and they look good.

The most important addition to the Windows installer was the "Publisher" entry that is required if we want to upload Thinlinc client to the Microsoft store.

Most other additions look sensible to me. The choice of URL for "HelpLink" may be a bit confusing, since cendio.com/thinlinc/support does not actually contain information about client support. On the other hand, we don't have any documentation intended for client users (the TAG is intended for administrators), and as such it at least makes sense to provide a URL that indirectly directs users to the ThinLinc community, where help can be found.

I have installed the client from the latest Jenkins build (4.17.0post), and via the Microsoft store (4.17.0). In both cases, all the new properties are set in
> Regedit-> HKLM/SOFTWARE/Microsoft/Windows/CurrentVersion/Uninstall/tlclient,
and they are visible e.g. when looking in appwiz.cpl, on both Windows 10 and Windows 11.

Closing!

Comment 25 Alexander Zeijlon 2024-08-14 09:13:25 CEST

Follow-up on comment 24.

I installed tlcustomizer and verified that the newly added fields are displayed properly on both Windows 11 and Windows 10. I can also see that the corresponding properties are set for tlcustomizer in Regedit.