Recent

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

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3646
  • I like bugs.
Re: Problem with TrayIcon: mantis 35723
« Reply #30 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.
« Last Edit: October 10, 2019, 02:56:48 pm by JuhaManninen »

dbannon

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

dbannon

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

dbannon

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

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3646
  • I like bugs.
Re: Problem with TrayIcon: mantis 35723
« Reply #34 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.
« Last Edit: October 14, 2019, 02:45:23 pm by JuhaManninen »

dbannon

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

dbannon

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

JuhaManninen

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

dbannon

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

Cyrax

  • Hero Member
  • *****
  • Posts: 758
Re: Problem with TrayIcon: mantis 35723
« Reply #39 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.

JuhaManninen

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

dbannon

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

JuhaManninen

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

dbannon

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