[Differential] [Commented On] D3085: RFC: Use DBusMenu if available

[graesslin (Martin Gräßlin)]

graesslin added inline comments.

INLINE COMMENTS

> CMakeLists.txt:10-13
> +  find_package(XCB COMPONENTS XCB)
> +  set_package_properties(XCB PROPERTIES DESCRIPTION "Required for exposing the global menu on X11"
> +                         TYPE REQUIRED
> +                        )

HAVE_X11 is just trying to find XLib. There is no need to bind find XCB to find XLib. You can just move it out of the if.

Also the HAVE_X11 is I think wrong, but that's unrelated ;-)

> CMakeLists.txt:62
>  if(HAVE_X11)
> -  target_link_libraries(KDEPlasmaPlatformTheme PRIVATE Qt5::X11Extras ${X11_Xcursor_LIB})
> +  target_link_libraries(KDEPlasmaPlatformTheme PRIVATE Qt5::X11Extras ${X11_Xcursor_LIB} ${XCB_XCB_LIBRARY})
>  endif()

XCB::XCB

> kdeplatformtheme.cpp:115
> +        case QEvent::ApplicationStateChange:
> +            // FIXME isn't actually triggered when engaging Alt+left-click window moving
> +            m_timer->stop();

Maybe FocusOut delivers it?

> x11integration.cpp:59-60
> +
> +    const xcb_intern_atom_cookie_t cookie = xcb_intern_atom(c, false, name.length(), name.constData());
> +    QScopedPointer<xcb_intern_atom_reply_t, QScopedPointerPodDeleter> reply(xcb_intern_atom_reply(c, cookie, Q_NULLPTR));
> +    if (!reply.isNull()) {

can we cache the atom? That's causing a roundtrip every time it's invoked.

REPOSITORY
  rPLASMAINTEGRATION Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D3085

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma
Cc: graesslin, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161028/44dc614c/attachment-0001.html>