Lazarus

Programming => General => Topic started by: bonmario on September 30, 2019, 07:54:09 pm

Title: Problem with TrayIcon: mantis 35723
Post by: bonmario on September 30, 2019, 07:54:09 pm
Hi,
this is the mantis link: https://bugs.freepascal.org/view.php?id=35723

Quote
Ah, thanks Mario. I tested against so many Distros, I did not even know that Ubuntu made a Cinnamon one. What is it called ?

I'm using Ubuntu 19.04 64 bit, and i have installed Cinnamon from Synaptic.



Quote
Mario, can you please tell me if it has libappindicator.so or libappindicator3.so installed ?

Code: Text  [Select]
  1. bonmario@bonmario-desktop:~$ ldconfig -p | grep libapp
  2.         libappindicator3.so.1 (libc6,x86-64) => /usr/lib/x86_64-linux-gnu/libappindicator3.so.1
  3.         libappindicator.so.1 (libc6,x86-64) => /usr/lib/x86_64-linux-gnu/libappindicator.so.1
  4.  


Hi, Mario
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 01, 2019, 12:30:11 am
Ah, well done Mario. Much easier here.

Your system has both libappindicator and libappindicator3, so prior to release 61820 it would load libappindicar3 and be happy. If you remove libappindicator3, then it would fall back to the old SystemTray model. Few OS now support that model, even less will as time goes by. While we could spend days discussing why thats a bad thing, no point, its happening. See -

https://goodies.xfce.org/projects/panel-plugins/xfce4-statusnotifier-plugin

https://blog.linuxmint.com/?p=3795

https://developer.gnome.org/gtk3/stable/GtkStatusIcon.html - "gtk_status_icon_new has been deprecated since version 3.14 and should not be used in newly-written code. Use GNotification and GtkApplication to provide status notifications"

https://blogs.gnome.org/aday/2017/08/31/status-icons-and-gnome/


To see what the current state of play is, please See my table here - https://wiki.freepascal.org/Talk:How_to_use_a_TrayIcon#Linux_Problems

In particular, note that the changes made recently have disabled any system tray in both what I consider mainstream Linux distros. Its mainly about Wayland, I found you could get sensible menu placement if you disable Wayland but am reluctant to suggest my users do that. And, whether you like it or not, Wayland is apparently the way we are heading.

So, I have to ask, what is it you like about the old SystemTray model ?  Is it being able to detect left clicks separately from popup menu ?  Even if you can do it now, the evidence is, it won't be there in the next few years.

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 01, 2019, 04:35:59 pm
In particular, note that the changes made recently have disabled any system tray in both what I consider mainstream Linux distros. Its mainly about Wayland, I found you could get sensible menu placement if you disable Wayland but am reluctant to suggest my users do that. And, whether you like it or not, Wayland is apparently the way we are heading.
Then we must test for more desktops than just Unity. Maybe also Cinnamon?
Wayland is not a desktop, it is an X-Window system replacement. Do the desktops behave differently when they are under Wayland?
Here is my latest note in issue:
 https://bugs.freepascal.org/view.php?id=35723
---
r61758 in 0035983 solved the issue on my Manjaro Linux with KDE. It now works with both GTK2 and QT(5) bindings.
What should be tested in function UnityAppIndicatorInit to make it work in every distro?
Now it tests for "GetEnvironmentVariableUTF8('XDG_CURRENT_DESKTOP') <> 'Unity' ".
Should we test for Cinnamon?
My Manjaro has libappindicator3.so installed but apparently it is not needed with KDE. Instead, loading it screws things up.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 01, 2019, 10:14:56 pm
JuhaManninen, sorry, perhaps I am confusing you.
Unity is dead (thank the Lord), Ubuntu abandoned it several releases ago.

The Gnome developers decided that the SystemTray was too complicated and stopped (just like that) supporting it. While RedHat ewent along with that decision (as you would expect) Cannonical and KDE both decided they could at least find a solution that would get people over the transition to the newer approach. In Ubuntu they ported their own Libappindicator over to the Gnome Desktop. KDE came up with something similar, I suspect KDE went first ....

Meanwhile people started making things like TopIcons, probably just for Redhat flovoured system ?

Maybe this history is a bit out of order but basically OK.

I suspect Cinnamon  that Mario is using is 'just' a Cinnamon package installed on top of an existing Desktop. Not a tested and fine tuned complete package.

When you say r61758 in 0035983 solved the issue on my Manjaro Linux with KDE". what problem ?

All the System Tray need do is pop up a popup menu ?  it worked before 0035983 !

Davo




Title: Re: Problem with TrayIcon: mantis 35723
Post by: bonmario on October 02, 2019, 08:18:17 am
I suspect Cinnamon  that Mario is using is 'just' a Cinnamon package installed on top of an existing Desktop. Not a tested and fine tuned complete package.

Cinnamon is the principal desktop environment of the Linux Mint distribution and is available as an optional desktop for other Linux distributions and other Unix-like operating systems as well.
https://en.wikipedia.org/wiki/Cinnamon_(desktop_environment)

I've installed it from Ubuntu's repositories

Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 02, 2019, 11:49:43 am
Cinnamon is the principal desktop environment of the Linux Mint distribution and is available as an optional desktop for other Linux distributions and other Unix-like operating systems as well.
https://en.wikipedia.org/wiki/Cinnamon_(desktop_environment)

Yep, I have tested Linux Mint, (please) see my table at https://wiki.freepascal.org/Talk:How_to_use_a_TrayIcon#Linux_Problems. It works with the pre 61821. And it works with the post 61821 too, probably because it has not yet adopted wayland. The Linux Mint people were discussing this last year, not very positively.

Given that Gnome (and wayland as its Display Server) is the default desktop on Ubuntu, RedHat (inc Fedora) and Suse, my guess is that its far more common than any other particular desktop and probably represents about half of installed desktop Linux systems. Personally, I don't use it (much prefer Mate) but my users do. And there can be no denying its the a very important part of the Linux (and FreeBSD etc) world. Next most popular is KDE I suppose.

I am still unsure why the change in 35983 was made, any hint ?

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 03, 2019, 04:46:12 am
When you say r61758 in 0035983 solved the issue on my Manjaro Linux with KDE". what problem ?
All the System Tray need do is pop up a popup menu ?  it worked before 0035983 !
No, the System Tray also needs to call its OnClick handler when left-clicked.
I reopened the relevant issue:
 https://bugs.freepascal.org/view.php?id=35983
Please test with the provided demo application.
An uploaded patch gives special treatment for KDE only. Works for me. Do we need the same treatment for Cinnamon?
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 03, 2019, 09:14:42 am
Quote
No, the System Tray also needs to call its OnClick handler when left-clicked.
Ahh, finally I understand.
Looking at my table, there seems to be a number of combinations. Note that Mate gives you System Panel menu, some give us a popup menu with either left or right click, some pass back to a click handler. I guess thats the "complicated interface" that Shuttleworth and the Gnome developers complain about.  :(
I always ensure both left or right click give me the popup menu because thats what I believe users expect. Maybe ....

Juha, my table says Cinnamon does work in both SystemTray mode and LibAppIndicator3 mode and yes, I see how its a touch more useful in SystemTray one. So, good idea for now. However, thats just the immediate problem. But from what I have read, the trend is going to be away from the SystemTray, did you read the XFCE blog for example ? And even libappindicator3 is just a transitional thing....

So, if programmers do write code that depends on being able to both trigger a Popup Menu and detect, separately, a left click, they will have a diminishing number of possible Linux OSs that will do so. And we'll have the situation where end users will upgrade their OS and find something no longer works. And more Distros might drop SystemTray without supporting the transitional LibAppIndicator3 (as RedHat and Suse have done).

I will keep testing and add a note on the wiki page that warns programmers about this situation. Trying to get a release of my app out right now too !

The real solution is, I am afraid, a new model for Linux depending on the AppNotifier instead. But the AppNotifier appears to be very poorly documented and, obviously there are not yet any Pascal bindings to it. It works through DBus but I cannot workout if we talk directly through dbus of if there is a common API closer to user spaces. Sigh ....

Thanks Juhu, sorry we are giving you a hard time !

Davo

EDIT: OK, I really understand now. If you don't assign a popup menu, then there are cases where 61820 does not show its icon !  And they can join the other cases where that icon is not visible. Sigh, what a mess. Juha's method of filtering out on a desktop by desktop basis seems an even better one now. 

Sorry, I never thought to test when there is no popup menu assigned.

If it was not just a temporary fix, I'd suggest it needs to be made some sort of compile time option because we need different rules for applications that will assign a menu compared to ones that want to deal with the click in their own code. I have altered my table to and will start mapping that extra behaviour. Sigh.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 03, 2019, 02:04:38 pm
EDIT: OK, I really understand now. If you don't assign a popup menu, then there are cases where 61820 does not show its icon !  And they can join the other cases where that icon is not visible. Sigh, what a mess. Juha's method of filtering out on a desktop by desktop basis seems an even better one now. 
I guess there is no perfect solution but we should commit something to make it work with as many distros as possible. My patch essentially reverts r61758 except for KDE. What else is needed?
I must confess I have no deep knowledge of this issue. I have not really used SystemTray in my apps. I haven't even read the XFCE blog (yet). I must trust you guys for a valid solution.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 04, 2019, 01:58:37 pm
Juha, I have tested a few distros today and I confess to be quite unhappy. Truth is, it all depends on what the programmer wants to achieve. If all they want is a popup menu ist pretty easy to sort by distributions. But if the programmer wants to see that LeftClick event and maybe a right click menu, wow, the combinations possible is way out of scope.

I have data from six or seven distros at present, will need a lot more before coming up with a conclusive answer but I don't think whatever we do is likely to be pretty.

A wild card, how would you react to bringing out a new (Linux only) property for TrayIcon, allows the programmer to decide what is more important -
?

Neither Windows not Mac need such fussing.

Anyway, perhaps you need to wait until I have tested a few more distros.

Davo

Title: Re: Problem with TrayIcon: mantis 35723
Post by: kupferstecher on October 04, 2019, 02:47:36 pm
Hello,
I ran into the same problem with Linuxmint 19.2 Mate. Under Lazarus 2.0.4 the left click wasn't firing and the icon had rendering issues (Looked like aliasing or wrong size bounds). I went back to 2.0.2 where it worked correctly.
I guess its what you already know, I just wanted to tell in case there is a further helping detail.

The left click behaviour is quite important for me. I sometimes implement a "tray mode" in my apps. I.e. when the tray mode is activated, the app will appear when clicking on the tray icon. Taking the focus away from the app, e.g. by clicking outside the app, the app window will disappear together with the taskbar item.


A wild card, how would you react to bringing out a new (Linux only) property for TrayIcon, allows the programmer to decide what is more important
Perhaps a setting for the target desktop environment would be more self explanatory. Different fall-back mechanisms still could be possible.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 04, 2019, 05:11:45 pm
Juha, I have tested a few distros today and I confess to be quite unhappy. Truth is, it all depends on what the programmer wants to achieve. If all they want is a popup menu ist pretty easy to sort by distributions. But if the programmer wants to see that LeftClick event and maybe a right click menu, wow, the combinations possible is way out of scope.
I applied the test for KDE and Cinnamon in r61980 which I plan to merge for Lazarus 2.0.6. I understand the issue depends also on other things than desktop and LibAppIndicator3 lib so my commit is not perfect but improves things.
What else does the issue depend on?
You should update the wiki page:
 https://wiki.freepascal.org/Talk:How_to_use_a_TrayIcon#Linux_Problems
Now you have the revisions wrong. I guess you meant r61620 and r61621.
There are now other revisions affecting, namely r61758 and r61980. Maybe you should just list what the current trunk does and then plan for a more robust solution.

Quote
A wild card, how would you react to bringing out a new (Linux only) property for TrayIcon, allows the programmer to decide what is more important -
A property cannot be "Linux only" in a cross-platform component. It can be documented to have effect only on Linux though.

Quote
  • Unset, we'll do the default
Default means not to load any LibAppIndicator libs, right?

Quote
  • Prefer the OldSystemTray except where we know it won't work - Good choice when we want to handle LeftClicks.
This means to load LibAppIndicator (v1) lib, right?

Quote
  • Prefer the LibAppIndicator3 except where we know it won't work - Good choice is all we want is a popup menu.
This is the current situation. Not loaded for KDE and Cinnamon where it does not work.

Take your time and then please provide a patch for a robust solution with a new property. It is a new feature and will not be backported.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 05, 2019, 05:31:21 am
OK Juha, I have made extensive (far too extensive) additions to https://wiki.freepascal.org/Talk:How_to_use_a_TrayIcon

It includes a summary of how we got here and some suggestions to a programmer intending to use TrayIcon. This probably needs to make it on to the main page. My raw data is also there and it needs to be enlarged with data from more distros but I think there is enough now for us to see whats happening.

....
What else does the issue depend on?
It is primarily about what the programmer wants to do. If they just want to popup a menu its quite a different solution from someone who wants to get a LeftClick handler called. Further complicated by potential to use (eg) TopIconsPlus to force new Gnome system to do a real SystemTray. Messy install will make many end users uncomfortable.

Quote
You should update the wiki page:
 https://wiki.freepascal.org/Talk:How_to_use_a_TrayIcon#Linux_Problems
Now you have the revisions wrong. I guess you meant r61620 and r61621.

61820 and 61821 is the change that prevents loading LibAppIndicator3.  I understand you have made changes since then but people like me need to wait until those changes make it int at least Fixes. Trunk is a bit too scary for people trying to make production quality software.
Quote
....
Take your time and then please provide a patch for a robust solution with a new property. It is a new feature and will not be backported.

Yeah, will involve some work. Have to decide if its justified given that this whole approach is wrong, maybe we need to put what ever effort is available into AppNotifier ?  Personally, I'm happy to manually edit UnityWSCrls.inc but thats an unproffesional approach. And it means apps like mine cannot make it into any repository that needs a source build.

We'll see !
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 05, 2019, 08:39:53 am
61820 and 61821 is the change that prevents loading LibAppIndicator3.  I understand you have made changes since then but people like me need to wait until those changes make it int at least Fixes. Trunk is a bit too scary for people trying to make production quality software.
Ok, r61821 merged trunk revision 61758 into fixes_2_0 branch.
r61820 is an unrelated revision in trunk:
 "Translations: Slovak translation update by LacaK, bug 36050"
All branches in the Subversion server share the same number space.

Trunk r61621 (issue #35723) was merged to fixes branch in r61634, together with 4 other trunk revisions.

Your wiki page says:
Quote
In this section, '820' means using Lazarus Fixes_2_0 release 81620 or earlier. '821' means 61821 or later up to a (soon) future release that tidies up this issue. Confused ?
Yes, it is partly wrong and very confusing. The fixes branch will become release 2.0.6 soon, maybe end of this month. You should refer to it instead of the merged revisions. The latest KDE/Cinnamon commit will be merged soon.

BTW, the wiki page talks about "Linux Problems" while actually the problems are with GTK2 bindings on Linux. UnityWSCtrls.pas is part of LCL-GTK2 binding code.
With QT(5) bindings everything works fine at least here. You could test it with your distros, too.

I still don't understand what makes the differences between distros that use the same desktop system, but I guess nobody does.
My common sense says that a desktop system + LibAppIndicator3 determine the behavior but apparently not.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 05, 2019, 01:26:44 pm
Ok, r61821 merged trunk revision 61758 into fixes_2_0 branch. r61820 is an unrelated revision in trunk:
Yes, of course, I was thinking how I isolated (what I saw as) the problem, it was not there in r61820, was there in r61821. Very careless wording on my part, will correct.

Quote
The fixes branch will become release 2.0.6 soon, maybe end of this month. You should refer to it instead of the merged revisions. The latest KDE/Cinnamon commit will be merged soon.

That I don't understand. "You should refer to it ..." - how do I refer to "it" without using revision numbers ? 

Quote
qt ?

I have, some time ago, tested against the QT libraries, from memory the results I achieved where disappointing, but I know a bit more  about the issue now, time I repeated those tests I guess. Yes, I agree, problem should be restricted to GTK.

Quote
I still don't understand what makes the differences between distros that use the same desktop system, but I guess nobody does.
My common sense says that a desktop system + LibAppIndicator3 determine the behavior but apparently not.
Most integrate their own ideas into a Desktop, Ubuntu for example add what ever is needed to make it work with libappindicator3. Redhat, because it works so close to Gnome, don't change as much. From memory, there is no libappindicator3 in the Redhat repositories. The Gnome Extensions, TopIcons* fill that role.

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 05, 2019, 04:37:44 pm
That I don't understand. "You should refer to it ..." - how do I refer to "it" without using revision numbers ? 
Just explain that Lazarus 2.0.6 behaves so and so compared to earlier versions like 2.0.4 and 2.0.2 etc.
It will be the reality in just few weeks.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 06, 2019, 11:54:45 am
OK, so I am really going to have to change my tune here ! But maybe its really good news !

I have now tested lots and lots of different distros.  And I have concluded that the issue that worries me is indeed confined to just Gnome desktops. And, I found that TopIconsPlus Gnome extension are now always needed for Lazarus TrayIcon and Gnome. So, if you want to use a Lazarus TrayIcon on Gnome, you must install TopIconsPlus.

And the newer version of TopIconsPlus seems to be all we need except for (so far) one exception, Mageia Gnome.  If I can detect when the user is running just that Distro/Desktop, I can have it use libAppIndicator3 and the rest just bounce back. I don't want all Gnome desktops to be forced to use libAppIndicator3 because, as Mario found, it does not call a click handler, its usable only with a menu.

Right now, when someone tries to use the TrayIcon Menu, it pops up no where near the spot the the user clicks.

Sigh.

How long do I have before you need to push that code into 2.0.6 ?  If you need an an immediate solution, then I would recommend changing the test (around #270 of UnityWSCtrl.pas) so that it exists if the Desktop is not 'GNOME', that will mean that RedHat, Mageia and possibly Suse distros running Gnome will not return a click handler. (Ubuntu won't be affected, its calls itself 'ubuntu:GNOME'.)  Thats significantly better than the current solution but still not good.

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 06, 2019, 12:59:11 pm
OK, Juha, here is my suggestion, have a look at it and see if you want me to make a patch.

Add this function within "function UnityAppIndicatorInit: Boolean;"  As a seperate function its easy to add more exceptions, as I am sure we will need to -

Code: Pascal  [Select]
  1.   function NeedAppIndicator() : boolean;
  2.   var
  3.     DeskTop,  VersionSt : String;
  4.     ProcFile: TextFile;
  5.   begin
  6.       AssignFile(ProcFile, '/proc/version');
  7.       try
  8.         reset(ProcFile);
  9.         readln(ProcFile, VersionSt);
  10.       finally
  11.         CloseFile(ProcFile)
  12.       end;
  13.       DeskTop := GetEnvironmentVariableUTF8('XDG_CURRENT_DESKTOP');
  14.       if DeskTop = 'Unity' then exit(True);
  15.       if ((DeskTop = 'GNOME') and (pos('mageia', VersionSt) > 0)) then exit(True);
  16.       if ((DeskTop = 'GNOME') and (pos('Red Hat', VersionSt) > 0)) then exit(True);
  17.       if ((DeskTop = 'GNOME') and (pos('SUSE', VersionSt) > 0)) then exit(True);
  18.       exit(False);
  19.   end;  
         

That gets called from within "function UnityAppIndicatorInit: Boolean;" where we previously had simpler test -

Code: Pascal  [Select]
  1.  begin
  2.     Result := False;
  3.     if Loaded then
  4.         Exit(Initialized);
  5.     Loaded := True;
  6.     if not NeedAppIndicator() then
  7.     begin
  8.         Initialized := False;
  9.         Exit;
  10.     end;
  11.     ....  
   

Like I said, if I have not broken too many rules here, I'll make you a patch.  Been tested against a whole lot of distros, Fedora (4 desktops), Ubuntu (4 desktops), Mageia (5 desktops), Debian (2 desktops) and Suse. I've tested against Gnome (always needs TopIcons), Gnome Classic, Mate, Cinnamon, XFCe, LXQt, KDE

Davo
   
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 06, 2019, 05:31:28 pm
OK, Juha, here is my suggestion, have a look at it and see if you want me to make a patch.
Add this function within "function UnityAppIndicatorInit: Boolean;"  As a seperate function its easy to add more exceptions, as I am sure we will need to -
...
Ok, I committed a modified version in r62003. Will be merged to 2.0.6.
I also resolved issue #35723 for now.
The name "Unity" is in a unit name, function name and class name (TUnityWSCustomTrayIcon). It is misleading now but I didn't change it.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 07, 2019, 01:10:30 am
Yes, the names are confusing, also misleading in that we are using Unity stuff in systems that have (thankfully) never had anything to do with (Cannonical) Unity. 

I would also like to incorporate some error checking, in both TUnityWSCustomTrayIcon and TGtk2WSCustomTrayIcon, if they returned an error when unable to display, a lot of this fuss could have been avoided.

I considered raising an exception if UnityAppIndicatorInit() decided it needed LibAppIndicator3 but could not find it. Its almost certain to fail in that case but "almost", sigh.  I check for the presence of the library further up, but the real problem is in the Gome systems, we cannot check that the end user has installed TopIcons, thats the hard task and right out of our sight.

I would also like to cleanup the comments in UnityWSCtrls, people who come later will find it quite confusing. Sigh...

when I see r63003 appear in Fixes, I update the wiki page (finally).

Thanks for your help here Juha, we have done something quite useful !

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: Alextp on October 07, 2019, 11:12:11 pm
Please fix the last patch. For ReadLn(f...).

{$PUSH}
{$IOCheck off}
Assign(..)
Reset(..)
if IOResult<>0 then exit;
{$POP}
Readln(...)
Close(..)
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 08, 2019, 05:40:57 am
Sure.  Should a new patch be based on
https://svn.freepascal.org/svn/lazarus/trunk/lcl/interfaces/gtk2/unitywsctrls.pas

which does not show the most recent patch or on

https://github.com/graemeg/lazarus/blob/upstream/lcl/interfaces/gtk2/unitywsctrls.pas

which does incorporate the recent patch.

Sorry, I do not understand the process ....

Mind if I ask why we don't want exception handling stuff in there ?  I am assuming there is a good reason, its just that I would like to know what it is.

Code: Pascal  [Select]
  1. begin
  2.   {$PUSH}
  3.   {$IOCheck off}
  4.   AssignFile(ProcFile, '/proc/version');
  5.   reset(ProcFile);
  6.   if IOResult<>0 then exit(false);
  7.   {$POP}
  8.   readln(ProcFile, VersionSt);
  9.   CloseFile(ProcFile)
  10.   .....
  11.  
No "try..finally..end" and no need to {$IOCheck on} ??

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: Alextp on October 08, 2019, 08:45:56 am
Hold on, I posted the patch
https://bugs.freepascal.org/view.php?id=36145#bugnotes
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 08, 2019, 10:18:13 am
OK, ReadFileToString(), thats useful. Not come across it before.

Thanks.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: Alextp on October 08, 2019, 02:39:53 pm
ReadFileToString cannot be used on '/proc/version'
Added new func: https://bugs.freepascal.org/view.php?id=36145
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 08, 2019, 09:35:07 pm
Sure.  Should a new patch be based on
https://svn.freepascal.org/svn/lazarus/trunk/lcl/interfaces/gtk2/unitywsctrls.pas
which does not show the most recent patch or on
https://github.com/graemeg/lazarus/blob/upstream/lcl/interfaces/gtk2/unitywsctrls.pas
which does incorporate the recent patch.
No, the trunk branch in svn.freepascal.org server is the master where all commits go. Graeme's GitHub mirror follows it with a small delay (15 minutes or something).

Quote
Sorry, I do not understand the process ....
You should concentrate on trunk when testing changes from other people or when creating patches yourself. The development always happens in trunk.
Bug fix revisions may then be merged to the fixes branch for the next dot-release. It is mostly usefull for people who don't participate in Lazarus development but want the latest stable version.
Lazarus developers must work with trunk. You are a Lazarus developer when you provide patches.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 10, 2019, 12:55:57 am
Are we making any progress here ?  Be really nice to get this fixed in 2.0.6.

And I am still puzzled as to where the authoritative 'trunk' version is.

https://svn.freepascal.org/svn/lazarus/trunk/lcl/interfaces/gtk2/unitywsctrls.pas

has Juha's  'fix kde and Cinnamon only' fix from several weeks ago. But Graham's github has my suggested "fix em all as best we can" model and has had it for several days. The real trunk should always be fresher than Graham's but that does not appear to be the case.

Am I looking at the wrong svn server for trunk ?  If I checked out trunk with SVN, would I get Juha's, mine or one of Alex's version ?

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 10, 2019, 08:04:50 am
Are we making any progress here ?  Be really nice to get this fixed in 2.0.6.
It will be there. See:
 https://wiki.freepascal.org/Lazarus_2.0_fixes_branch#Submitted_by_developer_.2F_committer.2C_tested.2C_waiting_to_be_merged

Quote
And I am still puzzled as to where the authoritative 'trunk' version is.

https://svn.freepascal.org/svn/lazarus/trunk/lcl/interfaces/gtk2/unitywsctrls.pas

has Juha's  'fix kde and Cinnamon only' fix from several weeks ago. But Graham's github has my suggested "fix em all as best we can" model and has had it for several days. The real trunk should always be fresher than Graham's but that does not appear to be the case.

Am I looking at the wrong svn server for trunk ?  If I checked out trunk with SVN, would I get Juha's, mine or one of Alex's version ?
Yes trunk is there:
 https://svn.freepascal.org/svn/lazarus/trunk
You have a browser cache issue. Press F5 and it will update.
Why don't you try checking out the trunk yourself. There are nice frontends for Subversion for any OS / platform.
I personally use Git-svn link which gives my local commit history + other goodies.
BTW, the mirror is Graeme's, not Graham's.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 10, 2019, 10:52:32 am
thanks for the reassurance Juha !

So, I was looking at the 'wrong' trunk !  I will bookmark the right one and sure won't make that mistake again ! 

I do struggle with svn, reasonably confident with git, use it for my own project, have used it professionally in the past. But I figure that svn is what FPC/Lazarus uses so I should make the effort. But Github sure does make looking at revisions easier! Happy at the command line...

Yep, I should play with trunk every now an again, especially as I now know where to get it.

And I do apologize to Graeme, everyone hates to see their name misspelled, its a bad thing.

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 10, 2019, 12:45:09 pm
Ahh sorry, missed one.

We have to add 'Debian' to the list of distros whose Gnome requires us to load the libappindicator3.

So, the list is now 'mageia', 'Red Hat', 'SUSE', 'Debian'

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 10, 2019, 02:28:11 pm
Ahh sorry, missed one.
We have to add 'Debian' to the list of distros whose Gnome requires us to load the libappindicator3.
So, the list is now 'mageia', 'Red Hat', 'SUSE', 'Debian'
I added it in r62020. Will be merged.
I also optimized the code. /proc/version needs to be read only for Gnome.
BTW, has anybody tested how the TrayIcon works with GTK3 bindings? Our changes affect only GTK2 bindings.

Quote
So, I was looking at the 'wrong' trunk !  I will bookmark the right one and sure won't make that mistake again !
No, you looked at the right trunk but your web browser had cached the file and showed an old version. Pressing F5 helps as I wrote.
Please use proper tools for viewing the revision history.

Quote
I do struggle with svn, reasonably confident with git, use it for my own project, have used it professionally in the past. But I figure that svn is what FPC/Lazarus uses so I should make the effort.
You can use Git if you prefer it, either through a Github mirror or through git-svn link. We take Git formatted patches, no problem.

Quote
But Github sure does make looking at revisions easier! Happy at the command line...
No, Git makes looking at revisions easier. You confuse things. IMO Github is overrated. Doing a pull request there is no way easier than creating and uploading a patch.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 11, 2019, 05:12:07 am
I added it in r62020. Will be merged.
I also optimized the code. /proc/version needs to be read only for Gnome.
BTW, has anybody tested how the TrayIcon works with GTK3 bindings? Our changes affect only GTK2 bindings.
Excellent !

Its quite messy really, take Debian for example, lots and lots of Debian users are still on 9.9, that are a very conservative bunch. It works fine without LibAppIndicator3 but we've just patched LCL to only LibAppIndictor3 on (all) Debian Gnome because Debian 10.0 and 10.1 does require it. I would consider checking each version of every OS just too hard so we have to accept that existing Debian9.9 users will now need to manually install LibAppIndicator3 and TopIcons. Sucks !

Really need to make it a run time choice. But that would do nothing on other OS and is just a temp fix I am afraid. Long term we will either have to drop the TrayIcon or make it work in the new mode.

 I have never tried GTK3, all my effort to keep my app going under GTK2, Windows, QT and Carbon/Cocoa. I was under impression it is quite incomplete ? Might have a play....

EDIT: Seems there is no TrayIcon implementation in GTK3, RegisterCustomTrayIcon() just returns False.

Quote
No, you looked at the right trunk but your web browser had cached the file and showed an old version. Pressing F5 helps as I wrote.
I had been pressing F5, made no diff. I just manually emptied my Firefox cache and now do see you most recent modes. Thats pretty strange. I'll trust what I see on github in future.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 12, 2019, 06:26:46 am
OK, sent my own app out to beta tester prior to its own next release (way overdue) based on code now in trunk->unityctrl.pas and first thing he comes back with is that he has lost his trayIcon !

Mageia Enlightenment but a beta version of that too. Hmm....

While we are still trying to establish why, thought I had better outline what I am doing with my app. It may well be against several Lazarus/FPC design rules and I am first to admit its as ugly as it gets but so is user apps failing when they update or change their distro.

In unityctrls.pas, I have code in the init function that looks for an environment variable called LAZUSEAPPIND, if its set to YES then we force an attempt to load Libappindicator3, if its NO we only rely on the old System TrayIcon and if its INFO we just log our decisions to stdout.

In this way, I can help an end user to identify what the problem is and, perhaps force one way or the other. Better than bringing out a new TrayIcon property that only Linux needs and, maybe only for a short while.

I'll make a patch and make it available ....

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 14, 2019, 12:52:31 pm
I see the note about 2.0.6 !

I am afraid that I have also found that Enlightenment should also be listed as a desktop that requires the LibAppIndicator3

There is bound to be more.   I am not sure what patches look like at the moment, Alex's suggestion is dependent on a separate patch providing a function to read the proc filesystem,  complicationing the issue a bit.  I would consider Juha  r62020 best bet but it need another line referring to Enlightenment.

What is the best approach ?  A new patch assuming r62020 is applied or a modified patch that extends 662020 ?

David
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 14, 2019, 02:17:15 pm
I am afraid that I have also found that Enlightenment should also be listed as a desktop that requires the LibAppIndicator3
Don't be afraid. Just add it into your coming patch. :)

Quote
I am not sure what patches look like at the moment, Alex's suggestion is dependent on a separate patch providing a function to read the proc filesystem,  complicationing the issue a bit.  I would consider Juha  r62020 best bet but it need another line referring to Enlightenment.
No, Alex's suggestion does not complicate anything because it is not applied to trunk or anywhere. He is working on non-issues or "nothing-burgers" as Americans say, which is a little irritating when there are so many open bugs.
Why would you consider r62020 the best bet? Just do your changes in trunk. It always has the latest sources. Yes, r62020 is the latest revision regarding unitywsctrls.pas but that is kind of irrelevant.
You are using SVN trunk now, aren't you?

Quote
What is the best approach ?  A new patch assuming r62020 is applied ...
Assuming ... WTF! r62020 is already applied, that is why it has a revision number. It will even be merged to the fixes_2_0 branch.

Quote
... or a modified patch that extends 662020 ?
That is far far future, maybe in year 22019.  :)

BTW, your environment variable LAZUSEAPPIND sounds like a good idea. If not found, the behavior would revert to what it does currently, thus making the change backwards compatible.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 15, 2019, 05:56:45 am
Quote
... or a modified patch that extends 662020 ?
That is far far future, maybe in year 22019.  :)
I am getting so excited I am starting to stutter.
Quote
BTW, your environment variable LAZUSEAPPIND sounds like a good idea. If not found, the behavior would revert to what it does currently, thus making the change backwards compatible.
OK, I am on it.  I have been using that idea in my own app so its tested and found useful already.  Will have that patch ready soon, I understand time is important right now ....

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 15, 2019, 07:26:52 am
OK, I have posted my patch on https://bugs.freepascal.org/view.php?id=35723

As I mentioned, I have used this approach in my own app, its due for a release and first beta tester I sent it to reported an incompatibility. And this approach made it very easy to confirm and even work around that problem.

Its an svn patch against current trunk.

EDIT: Note I have rearranged things a little, no point in doing file i/o if we don't need it, even /proc takes some time. And I have relocated the NeedAppIndicator function.

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 15, 2019, 03:08:18 pm
OK, I have posted my patch on https://bugs.freepascal.org/view.php?id=35723
Thanks. In future please also reopen the issue. Then I will notice it for sure in Mantis. I would have missed it without this forum post.

Quote
EDIT: Note I have rearranged things a little, no point in doing file i/o if we don't need it, even /proc takes some time. And I have relocated the NeedAppIndicator function.
NeedAppIndicator function is now nested inside UnityAppIndicatorInit although not using its parameters or local variables. As you know it imposes an extra frame pointer and a small performance penalty.
Your indentation is also different from the surrounding code.
Anyway I applied the patch as is because this TrayIcon stuff is now your project. It will be merged to fixes_2_0 branch, just in time for the release.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 16, 2019, 12:16:13 am
I did not know nesting a function like that incurred a performance penalty ! I did it like that to reduce its scope, sorry, next time, and I am certain there will be a next time, :-(  I will pop it back outside again.

Hmm, I do that 'nesting' quite a lot in my own code, guess I should review that.

I have updated the wiki page to expose that env var.

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: Cyrax on October 16, 2019, 01:43:35 am
I did not know nesting a function like that incurred a performance penalty ! I did it like that to reduce its scope, sorry, next time, and I am certain there will be a next time, :-(  I will pop it back outside again.

Hmm, I do that 'nesting' quite a lot in my own code, guess I should review that.

I have updated the wiki page to expose that env var.

Davo

There is no need for you to stop using nesting subroutines in your program. The performance penalty is minimal at least.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 16, 2019, 03:49:07 pm
There is no need for you to stop using nesting subroutines in your program. The performance penalty is minimal at least.
AFAIK the extra frame pointer is comparable to a Self pointer in methods of classes. Some people avoid object oriented programming because of the overhead caused by it.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 20, 2019, 08:18:45 am
Oh, Juha, you are going to laugh !  (Or at least I hope so...)

I have just, finally, got around to testing the newly released Ubuntu19.10 against the patches to keep TrayIcon going. I am afraid it too is now on the hit list of an distro that must be forced to use LibAppIndicator3.

Right now I am guessing that the 'other' Ubuntus will be OK, but we really won't know until each one is individually tested.  I will start now and post a patch to Mantis as soon as I can. If I can get it all together before 2.0.6, well and good, at least we have that debugging capability in there now that does allow us to override at run time.

The default install of Ubuntu also has a lot less GTK2 in there now too, creating a dependency problem for any Lazarus app. Fun eh ?

Davo
Title: Re: Problem with TrayIcon: mantis 35723
Post by: JuhaManninen on October 20, 2019, 12:44:32 pm
I have just, finally, got around to testing the newly released Ubuntu19.10 against the patches to keep TrayIcon going. I am afraid it too is now on the hit list of an distro that must be forced to use LibAppIndicator3.
Right now I am guessing that the 'other' Ubuntus will be OK, but we really won't know until each one is individually tested.  I will start now and post a patch to Mantis as soon as I can. If I can get it all together before 2.0.6, well and good, at least we have that debugging capability in there now that does allow us to override at run time.
No need to hurry with that. Let's keep it in trunk. Then it will be in the next major version 2.2.0 at some day. The environment variable LAZUSEAPPIND can solve problematic cases now, right?
Most Ubuntu users don't update immediately. Some of them use only LTS versions which 19.10 is not.

Quote
The default install of Ubuntu also has a lot less GTK2 in there now too, creating a dependency problem for any Lazarus app. Fun eh ?
GTK3 is the standard now. Even the desktop XFCE in version 4.14 got finally ported for it.
The solution is to improve the LCL-GTK3 bindings. You could maybe implement TrayIcon there.
Title: Re: Problem with TrayIcon: mantis 35723
Post by: dbannon on October 20, 2019, 01:32:56 pm
OK, makes sense. Its heaps better than it was before this sorry journey started.

Anyway, the mantis bug is 36199

I have not played with gtk3, no idea how complete it is but, as you say, time I found out.

Davo