appmenu-qt5

Merge lp://staging/~3v1n0/appmenu-qt5/avoid-x11-calls-in-other-envs into lp://staging/appmenu-qt5

  1. avoid-x11-calls-in-other-envs
  2. Merge into trunk
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Andrea Azzarone
Approved revision: 45
Merged at revision: 45
Proposed branch: lp://staging/~3v1n0/appmenu-qt5/avoid-x11-calls-in-other-envs
Merge into: lp://staging/appmenu-qt5
Prerequisite: lp://staging/~3v1n0/appmenu-qt5/x11-usertime-on-activate
Diff against target: 45 lines (+14/-3)
1 file modified
src/appmenuplatformmenubar.cpp (+14/-3)
To merge this branch: bzr merge lp://staging/~3v1n0/appmenu-qt5/avoid-x11-calls-in-other-envs
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Approve
Review via email: mp+311677@code.staging.launchpad.net

This proposal supersedes a proposal from 2016-11-24.

Commit message

AppMenuPlatformMenuBar: Don't initialize X11 related functions in other environments

To post a comment you must log in.
Revision history for this message
Andrea Azzarone (azzar1) wrote :

+1

review: Approve
Revision history for this message
Dmitry Shachnev (mitya57) wrote :

Can you please provide more details on how this was failing?

XSetErrorHandler should be safe to call on non-x11 environments, because it only sets an libX11-internal variable, and does not interact with the server.

Or maybe the real reason is that GTK+ 2.x does not interact well with Mir? Then the commit message is misleading: the X11 related functions to blame are not in appmenu-qt5, but in GTK+ 2.

If you are sure there is some kind of failure, then we may need to fix the mainline Qt too: it has similar code in http://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platformthemes/gtk3/qgtk3theme.cpp#n78.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

The problem here was not in XSetErrorHandler (that was not needed anyway at this point), but in the call `QGnomeTheme::themeHint(hint);`

I don't remember the details now, but it was somewhat caused by some code in gtk2 yes.
Upstream qt has not this kind of issues. Also gtk3 is not X11-only toolkit, thus it self-protects when not running in such environments

Revision history for this message
Dmitry Shachnev (mitya57) wrote :

OK. Now that we have Qt 5.7 in Zesty, we can port appmenu-qt5 to GTK+ 3, though I would prefer to remove it altogether instead (bug 1634941).

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Yeah, I do agree that removing appmenu-qt5 is way better (considering also that shipping this in a snap world, would just be annoying).

We should take care of changes going to be needed for unity8 appmenus too, though.
You might get in touch with Nick Dedekind.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches