Move QT_X11_NO_NATIVE_MENUBAR to code

[jasp00]

Enable QT_X11_NO_NATIVE_MENUBAR by default. Clean up the desktop entry.

[PhysSong]

• I don't think the error message is descriptive enough.
• I found that the environment variable is meaningless when using Qt5. See MDI Interface bug with Ubuntu Unity #488 for details.

[jasp00]

• I don't think the error message is descriptive enough.

Make a proposal.

[tresf]

@jasp00 thanks for the trick, this will come in useful. For this particular setting, I recommend we remove the env and use code instead, thanks to @PhysSong's findings here: #488 (comment)

[tresf]

@jasp00 Qt::AA_DontUseNativeMenuBar is part of the Qt4 documentation. Is there a reason why you did not use it?

[jasp00]

I do not see a way to override Qt::AA_DontUseNativeMenuBar.

[PhysSong]

I do not see a way to override Qt::AA_DontUseNativeMenuBar.

I think @tresf meant QCoreApplication::setAttribute. Both ways work the same to me, though they internally work slightly different.
Quoting part of the source code of appmenu-qt5:

void AppMenuPlatformMenuBar::createMenuBar()
{
    static bool firstCall = true;
    static bool envSaysNo = !qgetenv("QT_X11_NO_NATIVE_MENUBAR").isEmpty();
    static bool envSaysBoth = qgetenv("APPMENU_DISPLAY_BOTH") == "1";

    if (!m_menuBar->parentWidget()) {
        return;
    }

    m_adapter = 0;

    if (!firstCall && !envSaysBoth && QApplication::testAttribute(Qt::AA_DontUseNativeMenuBar)) {
        return;
    }

    if (envSaysNo) {
        if (firstCall) {
            firstCall = false;
            m_nativeMenuBar = NMB_DisabledByEnv;
            QApplication::setAttribute(Qt::AA_DontUseNativeMenuBar, true);
        }
        return;
    }

bool AppMenuPlatformMenuBar::isNativeMenuBar() const
{
    if (m_nativeMenuBar == NMB_DisabledByEnv) {
        return false;
    }
    if (m_nativeMenuBar == NMB_Auto) {
        return !QApplication::instance()->testAttribute(Qt::AA_DontUseNativeMenuBar);
    }
    return m_nativeMenuBar == NMB_Enabled;
}

• I don't think the error message is descriptive enough.

Make a proposal.

Something like this?

Failed to set QT_X11_NO_NATIVE_MENUBAR environment variable. MDI interface might be buggy.

[tresf]

... but the error message is irrelevant because we should be using the API, or remove it entirely since it does nothing for Qt5.

[PhysSong]

... but the error message is irrelevant because we should be using the API, or remove it entirely since it does nothing for Qt5.

Okay. Now I'm fine with keeping the error message as-is.

[tresf]

I'm confused. The error message wouldn't be needed if using QCoreApplication::setAttribute which is more consistent with the hidpi PR. Can we just use that and settle this? How does an error message fit into that?

[PhysSong]

@tresf I was talking about the error message we should use if we stick to setenv.

[jasp00]

Ready.

[tresf]

Testing with Qt4:

Before:

After:

[tresf]

Travis-CI shows failed, but that's just the Debian jobs. Appears to be failing, not related to the codebase at all. Merging.