[Differential] [Changed Subscribers] D4301: [System Tray] SNI fallback to context menu on failing Activate

Kai Uwe Broulik noreply at phabricator.kde.org
Fri Jan 27 08:25:41 UTC 2017


broulik added inline comments.

INLINE COMMENTS

> statusnotifieritemsource.cpp:472
>      if (m_statusNotifierItemInterface && m_statusNotifierItemInterface->isValid()) {
> -        m_statusNotifierItemInterface->call(QDBus::NoBlock, QStringLiteral("Activate"), x, y);
> +        m_statusNotifierItemInterface->call(QDBus::BlockWithGui, QStringLiteral("Activate"), x, y);
> +        return m_statusNotifierItemInterface->lastError().isValid();

This spins its own event loop which is dangerous when dealing with QML

> statusnotifieritemsource.cpp:473
> +        m_statusNotifierItemInterface->call(QDBus::BlockWithGui, QStringLiteral("Activate"), x, y);
> +        return m_statusNotifierItemInterface->lastError().isValid();
>      }

I think that method should return true for success, not failure.

> statusnotifieritemsource.cpp:475
>      }
> +    return true;
>  }

Can this lead to false positives? Maybe we need a tri-state here, ie. activate succeeded, activate failed (fall back to menu), nothing to activate (do nothing).

REPOSITORY
  R120 Plasma Workspace

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

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

To: subdiff, #plasma, davidedmundson
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170127/0d91571c/attachment-0001.html>


More information about the Plasma-devel mailing list