User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.159 Safari/537.36

Steps to reproduce:

I have enabled the setting "When thunderbird is minimised, move to tray" in Windows 10. This work good, thanks for that.

Actual results:

But, as soon as you click on this icon, to open thunderbird, it immediately disappears. This is not a problem when you just single click.
But many of systray icons requires a double click (Steam for example) to open the app, so i'm used to double click.
And now, and that only happens for thunderbird in all my systray icons, when i double click thunderbird, it opens with the first click, instantly disappears, another icon comes into place of thunderbird icon, and my double click fires the action on the other icon.

In my case, Nvidia Systray is beside Thunderbird, which in case of a double click on Thunderbird opens thunderbird and Nvidia Syspanel.

I don't know, but does this happen only to me? It is so annoying :) I can't get the double click out of my head, as i said earlier, many systray applications requires double clicks to open an app.

Expected results:

This all can be prevented in multiple ways.

1. Do not hide the systray, even if thunderbird is opened (As almost every other application does it). Systray icons, as soon as they activated, never hide. (My preferred option)
2. Hide the systray icon after some delay, example after 1s instead of instantly. This prevent double click fails also.
3. Change single click to double click action (Not suggested)

Thanks for your attention. Maybe i am the only one with this problem, but i thought i give it a try here in bugzilla.

See also bug 1727890

This request echoes the one in this feature request on Mozilla.Connect

The option described in #1 should work. And this behaviour - leave the icon in tray on open is, in fact a default behaviour for most (all?) windows applications (e.g.: Slack, qBitTorrent, OneDrive and TheBat mail client)

(I will take a stab on the code, this should be simple)

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/5ee80510cdd7
Added an option to always show the tray icon on Windows. r=mkmelin

This is breaking a bunch of tests such as comm/mail/test/browser/junk-commands/browser_junkCommands.js comm/calendar/test/browser/browser_todayPane_dragAndDrop.js comm/mail/components/extensions/test/browser/browser_ext_menus.js
You can run a test like ./mach test comm/mail/test/browser/junk-commands/browser_junkCommands.js

Backout: https://hg.mozilla.org/comm-central/rev/7173869f5939dd5cd7d011d21b44cf38e43a35a2

Please add your new pref here:
https://searchfox.org/comm-central/rev/55a0a0eea8460301559b4dd58468a273da7d972b/mailnews/mailnews.js#849

Prefs which are not in the UI are already hard to find, people need to inspect
https://searchfox.org/comm-central/source/mailnews/mailnews.js or
https://searchfox.org/comm-central/source/mail/app/profile/all-thunderbird.js
https://searchfox.org/comm-central/source/mail/extensions/am-e2e/prefs/e2e-prefs.js

Pref which are not in those files are impossible to find, you'd have to read through the entire codebase to find those prefs ... or know about them by being subscribed to the respective bug.

Not giving the pref a default value also led to the test failures. You don't really need your latest code changes if the pref has a default value.

Also, would you mind explaining why you need to listen to the profile-after-change event now?

If some action really needs to be taken at app startup, can't you add the processing to final-ui-startup? Also, if you register a listener for the profile-after-change event, you'd need to deregister it here:
https://searchfox.org/comm-central/rev/55a0a0eea8460301559b4dd58468a273da7d972b/mail/components/MailGlue.jsm#333

(In reply to b8 from comment #9)

Thanks for helping out!

Please add your new pref here:
https://searchfox.org/comm-central/rev/55a0a0eea8460301559b4dd58468a273da7d972b/mailnews/mailnews.js#849

Added & committed.

You don't really need your latest code changes if the pref has a default value.

Leaving them anyways? Just for safety's sake.

Also, would you mind explaining why you need to listen to the profile-after-change event now?

As you correctly pointed it out - to show the tray icon upon the app startup. (Otherwise user would need to minimize the app once for the icon to show). As there is no documentation or comments on a correct approach (or it is really hard to find :) - I took this approach from one of the unit tests, which was testing for actions on app startup/shutdown.

Also, if you register a listener for the profile-after-change event, you'd need to deregister it here:

Could you please clarify on that - I see a lot of other listeners registered in MailNotificationManager.jsm, e.g windows-refresh-badge-tray, profile-before-change - but none of them de-registered in MailGlue.jsm?

First of all, something appears to be wrong with https://phabricator.services.mozilla.com/D165149, it only shows the latest changes but not the ones in MailGlue.jsm.

OK, so you need to trigger displaying the badge at startup and hence added code to MailGlue.jsm. Why can't that be added to final-ui-startup which already has a listener? We tried to locate the exact condition under which profile-after-change triggers but didn't find it. Do you know when it's triggered? Why did you choose this event? In general it's preferable not add another listener if there is one already that can be used.

As for deregistering: As you can see in MailGlue.jsm, all the ones that are registered are also deregistered, so this scheme should be followed (if at all necessary in case final-ui-startup can't be used).

As for windows-refresh-badge-tray (https://searchfox.org/comm-central/search?q=windows-refresh-badge-tray&path=&case=false&regexp=false) and profile-before-change (https://searchfox.org/comm-central/search?q=profile-before-change&path=&case=false&regexp=false): Mostly, but not always, there is also code to deregister. We can't do a line by line analysis of all the cases where the deregistration is missing, but for good housekeeping especially in MailGlue.jsm, a call for deregistering should happen.

The deregistration always happens in the code that registers as you can see studying those references.

(In reply to b8 from comment #12)

First of all, something appears to be wrong with https://phabricator.services.mozilla.com/D165149, it only shows the latest changes but not the ones in MailGlue.jsm.

I didn't do any changes to MailGlue.jsm yet. Those 3 files are the ones changed in my commits (and rolled back later after test failures):

• /source/mailnews/base/src/MailNotificationManager.jsm
• /source/mailnews/base/src/nsMessengerWinIntegration.cpp
• /source/mailnews/mailnews.js (added default pref as per your comment)

Could it be a rollback caused only the latest changes to appear on D165149?

OK, so you need to trigger displaying the badge at startup and hence added code to MailGlue.jsm. Why can't that be added to final-ui-startup which already has a listener?

I haven't added any code to MailGlue.jsm -- the code that updates the badge (and adds it on start) is all in MailNotificationManager.jsm and MailNotificationManager doesn't add a listener for final-ui-startup, however it has one for profile-before-change as well as a method _updateUnreadCount() which should be called for profile-after-change too (allowing minimal code changes).

We tried to locate the exact condition under which profile-after-change triggers but didn't find it. Do you know when it's triggered? Why did you choose this event?

I've studied the code from unit tests (e.g. test_startup_service.js#40 among other tests), which looks like profile-after-change and profile-before-change are called on startup/shutdown. The code in MailNotificationManager.jsm#119 confirms profile-before-change as correct choice for shutdown and the actual test runs confirmed profile-after-change to be called on startup (method is called - and the badge/tray icon is added).

As for deregistering: As you can see in MailGlue.jsm, all the ones that are registered are also deregistered, so this scheme should be followed (if at all necessary in case final-ui-startup can't be used).

Sorry, still not sure how MailGlue.jsm related here... On the other hand I've noticed that there are no de-registration at all for any of the listeners added in MailNotificationManager.jsm in that file - but as you can see I'm rather unfamiliar with the entire code base here. So if you say that de-registration needs to be added to MailGlue.jsm - I'll follow that advice.

The deregistration always happens in the code that registers as you can see studying those references.

That was my concern, since I didn't see any de-registration in MailNotificationManager - and I guess I just followed the pattern.

Sorry again - those were my very first commits for Thunderbird codebase, so I guess pretty much everything that could go wrong - went wrong. :)

There seems to be quite a bit of confusion here. Sorry about that.

Apologies for mentioning MailGlue.jsm. Listeners are registered and deregistered there but that has nothing to do with your patch. Comment #10 was completely wrong, and so was most of comment #12 :-(

The changes which were landed (commited to the comm-central repository) and later backed out (removed) are here https://hg.mozilla.org/comm-central/rev/5ee80510cdd7 in MailNotificationManager.jsm and nsMessengerWinIntegration.cpp.

However, https://phabricator.services.mozilla.com/D165149 in the latest version of the patch now only shows changes in nsMessengerWinIntegration.cpp and mailnews.js, the changes in MailNotificationManager.jsm are now missing. Maybe you should remove all your local commits (hg out, hg strip), pull/update the repos again and then apply the full changes again before submitting them to Phab. We're attaching the full changes as a patch which you can just import or apply. We've also tested the changes.

(In reply to b8 from comment #14)

However, https://phabricator.services.mozilla.com/D165149 in the latest version of the patch now only shows changes in nsMessengerWinIntegration.cpp and mailnews.js, the changes in MailNotificationManager.jsm are now missing. Maybe you should remove all your local commits (hg out, hg strip), pull/update the repos again and then apply the full changes again before submitting them to Phab. We're attaching the full changes as a patch which you can just import or apply. We've also tested the changes.

The confusion is fully understandable - considering the state of D165149 - still not sure how it got to that :)

To avoid any further mess (since I don't fully understand what's going on with D165149) - I'm going to abandon it altogether and create a completely new revision with all the correct changes patched in:

https://phabricator.services.mozilla.com/D165584

It passes all tests and lint checks on my local.

Comment on attachment 9309791 [details] [diff] [review]
1727874-try-icon-always.patch

Thanks!

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/0342d0aeb14f
Added an option to always show the tray icon on Windows. r=mkmelin