DeaDBeeF-Player / deadbeef

DeaDBeeF Player
https://deadbeef.sourceforge.io/
Other
1.64k stars 178 forks source link

Use libappindicator for tray icon #1170

Closed Oleksiy-Yakovenko closed 8 years ago

Oleksiy-Yakovenko commented 9 years ago

Original issue 1268 created by Alexey-Yakovenko on 2015-02-13T13:38:29.000Z:

I am using plasma next on archlinux. It is known that plasma next has dropped support for xembed tray icons. GTK applications should use libappindicator to display a tray icon in plasma next. Is there any plan to implement tray icon with libappindicator?

vovochka404 commented 9 years ago

Don't know how this is done in a deb world, but in an rpm based something like this:

./configure ... \
%if %suse_version >= 1310
  --enable-sni \
%else
  --disable-sni \
%endif

But okay, хозяин - барин :)

How this trayicon replacement can be done via a plugin?

Oleksiy-Yakovenko commented 9 years ago

Don't know how this is done in a deb world, but in an rpm based something like this:

It's probably done the same way, but I will repeat 3rd time -- you misunderstanding my point. You're talking about compile time configuration, while I'm talking about run-time.

FYI: It has nothing to do with deb vs rpm, or anything related.

How this trayicon replacement can be done via a plugin?

The code would be exactly the same I guess. Just put into a separate plugin. See how other plugins work.

Oleksiy-Yakovenko commented 9 years ago

You should not make your plugin replace the system tray icon provided by GTKUI.

In fact, your plugin doesn't have to assume that GTKUI is even used.

Are there any functions in the statusnotifier code that have to interact with other windows?

vovochka404 commented 9 years ago

The code would be exactly the same I guess. Just put into a separate plugin. See how other plugins work.

But i'll have to remove current hardcoded GtkStatusIcon. Right?

you misunderstanding my point.

Yes... That's true... from my point of your position looks like: all code must work in old distors, but if it's not working in a modern DE - it doesn't metter.

Are there any functions in the statusnotifier code that has to interact with other app windows?

How about basic one: show/hide main window? :) And as i understand, current popup menu is also a part of a gtkui. So it must be reimplemented in a separate plugin?

The other case: gtkui knows nothing about sni plugin. i'm disabling tray icon (because it's duplicates sni) and what is the default behavior for gtkui mainwindow on close? Yep... Exit. Not to hide to tray.

How can i control this behavior via plugin?

vovochka404 commented 9 years ago

Also about current GtkStatusIcon - it's deprecated. How long you will be able to use it?

Oleksiy-Yakovenko commented 9 years ago

But i'll have to remove current hardcoded GtkStatusIcon. Right?

No. Why would you? Just leave it alone.

Yes... That's true... from my point of your position looks like: all code must work in old distors, but if it's not working in a modern DE - it doesn't metter.

This is not correct. First of all, I disagree that it "doesn't work". As I explained at the top of this thread, it works correctly in Unity and OSX, the way it's designed to be, without using the tray icon.

KDE way is different from everything else, because it doesn't have the dock, but removes the system tray at the same time (which no one else was doing AFAIK). Since this feature is only useful in KDE -- it needs to be in a specialised kde-specific plugin.

Another thing you're wrong about is that "when it's not working it doesn't matter". No. My point is that when it's not working -- it needs to fixed, but not at the cost of breaking other things.

How can i control this behavior via plugin?

By using the same config variables as GTKUI is using.

How long you will be able to use it?

It's not going to disappear from GTK2 and GTK3. It will most likely be removed in GTK4. Hope that answers your question.

vovochka404 commented 9 years ago

Ok, I'll go the plugin way...

By using the same config variables as GTKUI is using.

But how about menu re-implementation and MainWindow show/hide? I really don't know how to make this show/hide functionality via plugin.

Do you have any proposals how to control that there is no 2 icons at the same time? Do not show sni when there is a common tray icon enabled? But in case of plasma5 it's just not seen. Show sni just only in case of plasma5? :)

Oleksiy-Yakovenko commented 9 years ago

But how about menu re-implementation and MainWindow show/hide?

If you only care about GTKUI, then you can use the GTKUI API for that. It exposes GtkWidget pointer of the main window, so you can query its state, and work off that.

#include <deadbeef/gtkui_api.h>

See more info about this on the wiki here, the "Example2: deadbeef 0.6 only" is what you want, I believe.

Do you have any proposals how to control that there is no 2 icons at the same time?

Do you have a repro how to get into this situation? are there DEs which have both tray and the indicators at the same time?

Show sni just only in case of plasma5? :)

I suppose it would show as soon as your plugin is installed. Otherwise not shown. And user could go in the plugin settings, and switch it off.

vovochka404 commented 9 years ago

Do you have a repro how to get into this situation? are there DEs which have both tray and the indicators at the same time?

KDE4 as minimum. Supports old tray and SNI.

Oleksiy-Yakovenko commented 9 years ago

Well, in this case, it's unlikely that the users will want to install the SNI in the first place...

But if they just want to have SNI instead of tray - the best for them would be to open Preferences, and disable the tray icon.

vovochka404 commented 9 years ago

If you only care about GTKUI, then you can use the GTKUI API for that. It exposes GtkWidget pointer of the main window, so you can query its state, and work off that.

include <deadbeef/gtkui_api.h>

See more info about this on the wiki here, the "Example2: deadbeef 0.6 only" is what you want, I believe.

Thanks for this.. Will investigate further and ask if something will be unclear to me :)

vovochka404 commented 9 years ago

Hi, again :) I've almost finished with my plugin and i have 2 questions.

1) would you mind to place two strings somewhere in your project for localization purpose? I don't think it's wise to have separate *.po files.

2) I have many glib/gtk warnings, mainly related to:

  1. (deadbeef:654): Gtk-WARNING *_: Screen for GtkWindow not set; you must always set
  2. css related stuff: (process:654): Gtk-CRITICAL *_: _gtk_css_rgba_value_get_rgba: assertion 'rgba->class == &GTK_CSS_VALUE_RGBA' failed

(process:654): Gtk-CRITICAL **: _gtk_css_rgba_value_get_rgba: assertion 'rgba->class == &GTK_CSS_VALUE_RGBA' failed

I do not see that you are specifying anything screen related for menu in gtkui. And i don't understand where from this css related warnings are coming..

Can you help me with this?

Oleksiy-Yakovenko commented 9 years ago
  1. In general, no. Plugin devs are supposed to localize their stuff on their own.
  2. Looks like some GTK3 theme issue to me.
vovochka404 commented 9 years ago

But what about "screen" stuff? Can you help with this?

You can check plugin here

Oleksiy-Yakovenko commented 9 years ago

I know managing localization of your project is a huge task. I'd say just keep it english-only for now.

But what about "screen" stuff? Can you help with this?

I never seen this problem before. I think it's caused by your GTK3 theme.

vovochka404 commented 9 years ago

And also, can you check sni_update_tooltip? I'm afraid that there may be something wrong with memory managment... may be you can advise something?

Oleksiy-Yakovenko commented 9 years ago

There's a tool called valgrind, which can find memory problems.

vovochka404 commented 9 years ago

i'm a little bit afraid of that tooltip can be lager than 1000 bytes..

Oleksiy-Yakovenko commented 9 years ago

then use snprintf instead of sprintf, to make sure buffer doesn't overflow. or calculate the precise buffer size from data, and allocate exactly how much is needed.

snevolin commented 9 years ago

В Fedora 22 plasma 5 по дефолту, и иконка в deadbeef просто не видна всем пользователям KDE. Без иконки пользоваться deadbeef неудобно. Как-то это всё-таки можно исправить?

Oleksiy-Yakovenko commented 9 years ago

@snevolin готового решения пока нет, но если собрать deadbeef из ветки statusnotifier -- там есть поддержка kde-трея.

vovochka404 commented 9 years ago

@snevolin Попробуй использовать мой плагин: https://github.com/vovochka404/deadbeef-statusnotifier-plugin Но он может и не завестись с последней версией gtk По крайней мере у меня не заводится в opensuse tumbleweed, никак руки не дойдут переписать. Но если ты согласен быть моим тестером/пользователем, я постараюсь исправить эту проблему побыстрее :)

snevolin commented 9 years ago

@vovochka404 Отписал ошибку в твой репозиторий

vovochka404 commented 8 years ago

@Alexey-Yakovenko Можешь добавить ссылку на мой плагин к себе в wiki? :) Может так же в G+ рассказать? :)

Oleksiy-Yakovenko commented 8 years ago

@vovochka404 сделал

Oleksiy-Yakovenko commented 8 years ago

@vovochka404 вообще в deadbeef уже есть штатный плагин для этого дела, но он немножко недоделан до релизного состояния. но в master, например, уже можно бы и смержить.

vovochka404 commented 8 years ago

@Alexey-Yakovenko спасибо :) Я видел что шла какая-то активность в этом направлении и надеялся что в 7 версии он уже будет :) Но его не оказалось, а актуальность с каждым днём растёт :)

Oleksiy-Yakovenko commented 8 years ago

@vovochka404 в связи с тем, что оно не доделано -- его еще и в 0.7.1 не будет.

vovochka404 commented 8 years ago

Жаль, жаль... А можно в вики небольшое описание к плагину? А то ведь кто поймет что Status Notifier - это иконка в трее, а не какая-то уведомляшка о каких-то состояниях? Что-нибудь типа: provices alternative tray icon (mainly for plasma5 DE)

Oleksiy-Yakovenko commented 8 years ago

Status Notifier / Alternative status icon / Appindicator -- так будет норм?

vovochka404 commented 8 years ago

Большинству пользователей эти слова не о чем не говорят конечно. Но хоть в одном из вариантов есть слово icon :)

Может все же добавить слово tray?

Oleksiy-Yakovenko commented 8 years ago

никто и никогда в линуксе не использует слово System Tray (в оригинале) - это перебежчики с винды притащили. но ок, добавлю.

vovochka404 commented 8 years ago

Спасибо большое :)

Oleksiy-Yakovenko commented 8 years ago

App indicator type status icons are now supported on master branch.

xgdgsc commented 6 years ago

This is supported? I 'm on archlinux with plasma desktop and doesn' t see tray icon.

Oleksiy-Yakovenko commented 6 years ago

@xgdgsc It used to work, but then something changed in how newer linux distributions handle the notification area, and the library which deadbeef used for this stopped working, so I had to disable it.

I'm waiting until linux distributions / DEs come to some common conclusion about how they want to do system tray / notification area, and provide some API to support it - I can't keep up with these changes.

vovochka404 commented 6 years ago

@xgdgsc , can u try my plugin?

xgdgsc commented 6 years ago

Thanks. Your plugin works well, I added an archlinux package.

koroki commented 4 years ago

@xgdgsc , can u try my plugin?

Thanks!