I'm on the fence about this, but great points regarding running old Qt API. Would it be possible to wrap those higher API calls in a runtime version check (in addition to the compile-time check)?

I'm not aware of a method to check which Qt version is installed on the machine at runtime.
I tried googling and I found a couple of posts suggesting to parse the output of qmake -v, but that feels a bit hacky to me; and probably won't work on Windows machines. Not to mention that it might be relatively slow.

That macro itself is pulling the current run version using qVersion() which returns the version number as a const char*.

So I can make every compile time check wrap a corresponding run-time check using qVersion(). How about that?

Since Qt doesn't make this very easy, I think you should do a one-time conversion of the qVersion() output into an integer that can be easily compared against the QT_VERSION_CHECK macro construction:

QString sq = QString::fromLatin1(qVersion()); \
int version = (sq.section(QChar::fromLatin1('.'),0,0).toInt()<<16) + (sq.section(QChar::fromLatin1('.'),1,1).toInt()<<8)+ sq.section(QChar::fromLatin1('.'),2,2).toInt()

This could be stored in the config class (ie, config()->qtVersion()) or config()->checkQtVersion(major, minor, patch)

Alright, I think I can add this. Shall I keep the compile time checks wrapping the run-time checks or replace all compile time checks with run-time checks?

Keeping the compile time checks as a wrapper would imply that the newer code would be cut off at compile time and the user will never be able to benefit from it, even if the app will be run with a higher version of Qt at a later time.

We need to keep the compile time checks so people can build using our posted minimum Qt version of 5.2.

I ended up putting the function in Tools.cpp because I thought it's more appropriate than Config.cpp, which takes care of the user preferences.

I replaced a couple of compile time checks with runtime checks, namely the workaround for Unity in MainWindow.cpp. I also slightly changed that code so that it checks if the desktop environment is actually Unity prior to applying the workaround, as opposed to applying it only based on the Qt version and regardless of the desktop environment.

EDIT: I made the changes because of your request but I feel like the code has become harder to maintain now. Also, I don't think it's idiomatic Qt checking for the version at runtime. Also, checking for the Qt version at runtime when using deprecated methods, effectively broke the dev build because the compiler can't prove that the "old" methods will be used only when a lower version of Qt is detected at runtime, so I think we should either gracefully exit if the app is launched with a lower version of Qt or leave the code as before. Just my opinion.

This is a common practice in the Android world due to the "fragmentation" issue. I feel like it should be common in Qt as well since they made so many API changes between 5.2 and 5.12.

I am going to sit on this until @phoerious reviews.

I understand. If we're going to use the runtime-check approach, I think I'm going to refactor this to use the QVersionNumber class, which I've just found out about. It seems to make this task more natural and less "hacky", thanks to the fromString and compare methods.
I'll also add some #pragma statements over the methods which have been deprecated, in order to tell the compiler to ignore those and restore the dev build.

QVersionNumber was introduced in Qt 5.6

Yeah, I hadn't noticed that before. I only added the two #pragma statements to get the dev build working again.

I still don't understand why somebody would downgrade from 5.11 to 5.2 at runtime. That should never happen. Version upgrades may occur, but shouldn't break anything since we are not using depcrated functionality.

Well, it doesn't seem so far-fetched to me that I may have compiled KeePassXC on my machine which has, say, Qt 5.12.0, and then I take the compiled binary to my friend's computer which only has Qt 5.6.0.
Now, I didn't actually try this but if something like this happens, the program will probably crash since it'll try to reference non-existent functions. That was my point with this PR; I'm sorry if that wasn't clear from my explanation. I've never meant downgrading Qt at runtime while the program is running; that of course should never happen!

I was just proposing a way to handle this case gracefully instead of just letting the app crash on the user.

Qt versions are not necessarily binary-compatible across major versions, so you should never do that. Use our AppImage instead.

I understand. Thanks :)

I think this PR is still useful, do we want to merge it in?

Personally I'm not too convinced. I think the static approach of aborting during startup is a fairer approach. Especially since, as @phoerious says, downgrading Qt is something that should never happen and the goal was to handle gracefully an event that should be extremely exceptional (well, at least more gracefully than a crash).
Penalizing users with runtime overhead for something like this feels a bit overkill.

OK, I agree with that. If you want to keep your QtVersionChecker function in that is fine by me. Otherwise you can revert back to the graceful exit approach.

Ok, what about this change? I'll restore the compile time check so that this entire block of code won't be dumped to the output binary at all if the user compiles with Qt >= 5.9 and I'll also leave the runtime check so that the fix applies only if needed (just in case the user does have Qt <= 5.9 at compile time but upgrades later on and the workaround is no longer necessary); if you're okay with that.

@@ -987,16 +993,21 @@ void MainWindow::toggleWindow()
     } else {
         bringToFront();
 
-#if defined(Q_OS_UNIX) && !defined(Q_OS_MACOS) && !defined(QT_NO_DBUS) && (QT_VERSION < QT_VERSION_CHECK(5, 9, 0))
+#if defined(Q_OS_UNIX) && !defined(Q_OS_MACOS) && !defined(QT_NO_DBUS)
         // re-register global D-Bus menu (needed on Ubuntu with Unity)
         // see https://github.com/keepassxreboot/keepassxc/issues/271
         // and https://bugreports.qt.io/browse/QTBUG-58723
         // check for !isVisible(), because isNativeMenuBar() does not work with appmenu-qt5
-        if (!m_ui->menubar->isVisible()) {
-            QDBusMessage msg = QDBusMessage::createMethodCall("com.canonical.AppMenu.Registrar",
-                                                              "/com/canonical/AppMenu/Registrar",
-                                                              "com.canonical.AppMenu.Registrar",
-                                                              "RegisterWindow");
+        const static auto isDesktopSessionUnity = QString::fromLatin1(qgetenv("XDG_SESSION_DESKTOP"))
+                                                    .contains(QStringLiteral("unity"), Qt::CaseInsensitive);
+
+        if (Tools::qtRuntimeVersion() < QT_VERSION_CHECK(5, 9, 0)
+                && isDesktopSessionUnity
+                && !m_ui->menubar->isVisible()) {
+            QDBusMessage msg = QDBusMessage::createMethodCall(QStringLiteral("com.canonical.AppMenu.Registrar"),
+                                                              QStringLiteral("/com/canonical/AppMenu/Registrar"),
+                                                              QStringLiteral("com.canonical.AppMenu.Registrar"),
+                                                              QStringLiteral("RegisterWindow"));

Good for me

Alright, it should be ok now.