Misc fixes for Windows 7

[sryze]

This fixes the following issues with the desktop app on Windows 7:

1. Registry key HKCU\Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace does not exist

2. Task bar position is not calculated properly because HKCU\SOFTWARE\Microsoft\Windows\CurrentVersion\Explorer\StuckRects3 does not exist, but StuckRects2 can be used instead

[er-vin]

nitpick: please don't remove those empty lines, this has nothing to do with the rest of your patch and reduced readability IMHO.

[er-vin]

I think it'd be better to do a check on the key before hand using the registryKeyExists function you introduced to determine if we're going with StuckRects3 or StuckRects2. And then making a single registryGetKeyValue call with that key.

A bit like this:

QString taskbarPositionSubkey = "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\StuckRects3";
if (!Utility::registryKeyExists(HKEY_CURRENT_USER, taskbarPositionSubkey)) {
    taskbarPositionSubkey = "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\StuckRects3";
}
auto taskbarPosition = Utility::registryGetKeyValue(HKEY_CURRENT_USER, taskbarPositionSubkey, "Settings");

I think it'd show the intent better.

[sryze]

OK, updated it

[er-vin]

Ah also: please make sure all your changes are in a single commit and also please ensure you please the DCO bot.

Thanks in advance.

[DominiqueFuchs]

1. Registry key HKCU\Software\Microsoft\Windows\CurrentVersion\Explorer\Desktop\NameSpace does not exist

I'm wondering what the issue was? Because the check of the return value of RegOpenKeyEx is already done in the Utility::registryGetKeyValue (just like you do in the new registryKeyExists now), thus it's duplicated that way. If no key is found, it returns an empty QVariant:

https://github.com/nextcloud/desktop/blob/ca1575b59f62d9e10ac2aff82e445365241517d9/src/common/utility_win.cpp#L137-L138

Don't mind the change though, extracting this into a separate check function that return a bool (which is easier to understand/harder to misuse) may be a good idea.

[sryze]

@DominiqueFuchs the issue was that registryWalkSubKeys has an assert that fails if the key is not found:

    REGSAM sam = KEY_READ | KEY_WOW64_64KEY;
    LONG result = RegOpenKeyEx(hRootKey, reinterpret_cast<LPCWSTR>(subKey.utf16()), 0, sam, &hKey);
    ASSERT(result == ERROR_SUCCESS);

Instead of adding registryKeyExists, maybe it would be better to make registryWalkSubKeys just return false in such case, I'm not entirely sure about this

[er-vin]

@DominiqueFuchs the issue was that registryWalkSubKeys has an assert that fails if the key is not found:

    REGSAM sam = KEY_READ | KEY_WOW64_64KEY;
    LONG result = RegOpenKeyEx(hRootKey, reinterpret_cast<LPCWSTR>(subKey.utf16()), 0, sam, &hKey);
    ASSERT(result == ERROR_SUCCESS);

Instead of adding registryKeyExists,

registryKeyExists is needed anyway for the other part of the patch. So definitely a keeper.

maybe it would be better to make registryWalkSubKeys just return false in such case, I'm not entirely sure about this

I like your patch as is, but I like this idea quite a bit as well. Your call. Just let me know what you want to do.

[sryze]

I like your patch as is, but I like this idea quite a bit as well. Your call. Just let me know what you want to do.

OK, let's keep it as is

[er-vin]

/rebase

[nextcloud-desktop-bot]

AppImage file: Nextcloud-PR-2589-18c1bc0bd66be5f70965a45ba106a855b1013e5a-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.