Recent

Author Topic: Problem with TrayIcon: mantis 35723  (Read 3241 times)

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3644
  • I like bugs.
Re: Problem with TrayIcon: mantis 35723
« Reply #15 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.

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #16 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
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #17 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
   
« Last Edit: October 06, 2019, 02:13:35 pm by dbannon »
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3644
  • I like bugs.
Re: Problem with TrayIcon: mantis 35723
« Reply #18 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.
« Last Edit: October 06, 2019, 05:36:57 pm by JuhaManninen »

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #19 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
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

Alextp

  • Hero Member
  • *****
  • Posts: 874
    • UVviewsoft
Re: Problem with TrayIcon: mantis 35723
« Reply #20 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(..)

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #21 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
« Last Edit: October 08, 2019, 07:40:55 am by dbannon »
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

Alextp

  • Hero Member
  • *****
  • Posts: 874
    • UVviewsoft
Re: Problem with TrayIcon: mantis 35723
« Reply #22 on: October 08, 2019, 08:45:56 am »

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #23 on: October 08, 2019, 10:18:13 am »
OK, ReadFileToString(), thats useful. Not come across it before.

Thanks.
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

Alextp

  • Hero Member
  • *****
  • Posts: 874
    • UVviewsoft
Re: Problem with TrayIcon: mantis 35723
« Reply #24 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

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3644
  • I like bugs.
Re: Problem with TrayIcon: mantis 35723
« Reply #25 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.

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #26 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
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3644
  • I like bugs.
Re: Problem with TrayIcon: mantis 35723
« Reply #27 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.

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #28 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
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

dbannon

  • Hero Member
  • *****
  • Posts: 748
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Problem with TrayIcon: mantis 35723
« Reply #29 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
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng