1481777 - Remove "Thunderbird now contains calendaring functionality…" message at startup

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[Geoff Lankow (:darktrojan)]

MakeMyDay, you said to keep the code for this. What bits do actually want?

Comment 1

[[:MakeMyDay]]

I reconsidered since it would require to make that thing more generally usable first, so feel free to remove what was implemented with bug 1130852 including the prefs for mail and SM. I will re-implement it when i need it.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1481777-startup-notification-1.diff

Comment 3

[Martin Schröder [:mschroeder]]

(In reply to Geoff Lankow (:darktrojan) from comment #2)
> Created attachment 8999481 [details] [diff] [review]
> 1481777-startup-notification-1.diff

Should the preference "mail.calendar-integration.opt-out" also be removed from mail/app/profile/all-thunderbird.js and suite/browser/browser-prefs.js?

Comment 4

[Geoff Lankow (:darktrojan)]

Attachment: 1481777-startup-notification-prefs-1.diff

(In reply to Martin Schröder [:martinschroeder] from comment #3)
> Should the preference "mail.calendar-integration.opt-out" also be removed
> from mail/app/profile/all-thunderbird.js and suite/browser/browser-prefs.js?

Probably, yes.

Comment 5

[[:MakeMyDay]]

We're about to remove the notification bar about the Lightning integration in this bug. That was primarily intended to allow the users with existing profiles to decide about to keep Lightning when the bundeled Lightning was rolled out for the first time (for TB, this was 38), so we can consider it's job to be done.

When doing this we need to decide how to deal with the persisted user decission for TB and SM.

I would suggest to use the information from mail.calendar-integration.opt-out - currently not used otherwiese - to migrate those users in beta/esr channels, who have opted out but currently only disabled Lightning, and uninstall the addon while making sure extensions.installedDistroAddon.{e2fda1a4-762b-4020-b5ad-a41df1933103} is enabled (that pref would survive an addon uninstall) to respect the user's decision also in future.

That way we would also get rid of the complaints that Lightning got re-enabled, which we regularly receive after TB updates.

In turn, one could think about adding a UI control to the main products to give the user an easy way to get back a once uninstalled bundled addon if he changed his mind.

Most of the above wouldn't apply to Daily channel users since Lightning is bundled there as a system addon, which just can be disabled but not uninstalled.

Comment 6

[Magnus Melin [:mkmelin]]

I'm all for removing the startup message. 

> Most of the above wouldn't apply to Daily channel users since Lightning is bundled there as a 
> system addon, which just can be disabled but not uninstalled.

Since this bug is about trunk, does the uninstall discussion above apply?
In general I think just leaving it disabled and not uninstalling is ok.

Comment 7

[[:MakeMyDay]]

It does when 65 (or whenever this lands) goes to beta - next merge day is at beginning of September, so it's near-term.

The problem is it _does not_ keep disabled across updates, which is an issue in the toolkit code.

Comment 8

[Magnus Melin [:mkmelin]]

So you mean we bundle differently specifically for daily? (Not that bundling changed.) 
If so is there a reason for this?

Comment 9

[Frank-Rainer Grahl (:frg)]

SeaMonkey in comm-central is badly broken right now. We just keep it building for later. So I am fine either way. 

Personally I think Lightning should be fully integrated and no longer being build as an add-on which could be disabled. With the destruction of the classic add-on code this will probably be impossible soon anyway and I don't see that it would be possible as a web extension. For users who do not want it a disable pref for at least the menu items might do the job.

Comment 10

[[:MakeMyDay]]

Please(In reply to Magnus Melin from comment #8)
> So you mean we bundle differently specifically for daily? (Not that bundling
> changed.) 
> If so is there a reason for this?

We collected all the pro and cons in an etherpad in the 38 days, but this got lost in the etherpad migration. So from my memory, those days the way we do it for Daily was considered to go way in a foreseeable time, so we decided to go the from that pov more stable way for beta and esr.

But I would appreciate to not tranform this bug into a discussion about whether and how to change the way we bundle Lightning. Let's keep it with the remaining deciding about how to deal with te collected opt-out decisions.

I am not thrilled if we finally don't respect the users opt-out decision although we technically could do. We can add such a migration code to Lightning, but that would just mean the user would have to live with the current situation for one additional update cycle, which we could save when putting it into /common.

Comment 11

[Geoff Lankow (:darktrojan)]

Attachment: 1481777-startup-notification-prefs-2.diff

If the opt-out pref is true, set the installed pref to true. This is a little bit difficult to test, but AFAICT it works.

Comment 12

[[:MakeMyDay]]

Comment on attachment 9005938 [details] [diff] [review]
1481777-startup-notification-prefs-2.diff

Review of attachment 9005938 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/mailGlue.js
@@ +58,5 @@
> +    // If the user has opted out of Lightning, make sure it doesn't get reinstalled.
> +    if (Services.prefs.getPrefType("mail.calendar-integration.opt-out") == Services.prefs.PREF_BOOL &&
> +        Services.prefs.getBoolPref("mail.calendar-integration.opt-out")) {
> +      Services.prefs.setBoolPref("extensions.installedDistroAddon.{e2fda1a4-762b-4020-b5ad-a41df1933103}", true);
> +    }

You additionally need to check the current status of Lightning here.

If a user opted out out once and later reinstalled Lightning, the opt-out pref isn't reset, so for those users you would trigger to uninstall an active Lightning on restart.

Also, you should reset mail.calendar-integration.opt-out once the migration is done to get rid of the custom pref value, since we only need to run this once per profile.

Comment 13

[Worcester12345]

Wouldn't this be made easier if calendar were just integrated into Thunderbird?
https://bugzilla.mozilla.org/show_bug.cgi?id=401779#c36

and:
https://bugzilla.mozilla.org/show_bug.cgi?id=401779#c70

Comment 14

[Geoff Lankow (:darktrojan)]

(In reply to [:MakeMyDay] from comment #12)
> Also, you should reset mail.calendar-integration.opt-out once the migration
> is done to get rid of the custom pref value, since we only need to run this
> once per profile.

I think I must've read the code wrong before. I was under the impression that something reset the installedDistroAddon pref at upgrade, hence why I left the other pref behind, but now I can't find it.

Comment 15

[Geoff Lankow (:darktrojan)]

Attachment: 1481777-startup-notification-prefs-3.diff

Comment 16

[Geoff Lankow (:darktrojan)]

Comment on attachment 9007442 [details] [diff] [review]
1481777-startup-notification-prefs-3.diff

Hang on, let's address ALL the review comments first. :-/

Comment 17

[Geoff Lankow (:darktrojan)]

My recent patches don't achieve anything.

extensions.installedDistroAddon.{e2fda1a4-762b-4020-b5ad-a41df1933103} is true if Lightning was ever installed from the distribution. Nothing sets it to false.

If the user has *removed* Lightning, it won't be reinstalled if the above pref is true. If the pref is set to false then Lightning will be installed.

If the user has *disabled* Lightning, it will be upgraded on the next app update, but doesn't remain disabled. I've filed bug 1490957 because I think that's wrong. If the bug is not fixed we could check the opt-out pref and disable Lightning ourselves, *but* it would need to happen after the upgrade and before the Add-On Manager startup completes and that will be very tricky.

My suggestion is to go back to the first version of the prefs patch, and just remove the default values.

Comment 18

[[:MakeMyDay]]

Comment on attachment 8999536 [details] [diff] [review]
1481777-startup-notification-prefs-1.diff

Review of attachment 8999536 [details] [diff] [review]:
-----------------------------------------------------------------

All right, let's go with that, since a custom values of mail.calendar-integration.opt-out is kept even if we remove the deafult value, so we could pick it up if we're doing a fix for the re-enabling bug in comm-central. I'm defering the review to the mail and suite peers, though, since it changes their code.

Comment 19

[[:MakeMyDay]]

Re the bug 1490957: that has been an issue since we started bundling. It's related to the soft-disabling that is used while updating, iirc, but we never manage to pin that enough to propose a patch. And now it's probably too late to do so, since that could be an issue for non-restartless legacy addons, which isn't supported by FF anymore.

If we could find a way to prevent that in comm code, it would be nice, otherwise we stick with our recommendation to uninstall instead [1].

[1] https://support.mozilla.org/en-US/kb/calendar-updates-issues-thunderbird#w_lightning-reappears-after-a-thunderbird-update-although-i-opted-out-release-and-beta-versions

Comment 20

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8999536 [details] [diff] [review]
1481777-startup-notification-prefs-1.diff

Thanks++

Comment 21

[Wayne Mery (:wsmwk)]

I can see this general direction resulting in more issues, and perhaps this is tb-planning material and other bugs. For starters:

* What product adjustments and publicity/messaging is needed for users who have disabled or removed lightning, and future new users who might have opted to do the same?  (we know a stink will be made by some users who "don't want lightning")

* How do we provide for situations where we want to test thunderbird without lighting, without resorting to a different profile, either for performance issues or reliability issues?

Comment 22

[Magnus Melin [:mkmelin]]

(In reply to Wayne Mery (:wsmwk) from comment #21)
>   (we know a stink will be made by some users who "don't want lightning")

Not sure I ever heard from such users. 

> * How do we provide for situations where we want to test thunderbird without
> lighting, without resorting to a different profile, either for performance
> issues or reliability issues?

There is always -safe-mode

Comment 23

[[:MakeMyDay]]

What's removed here is just the lower notificationbar. This will only impact users with a new profile and those who haven't decided yet whether to keep Lightning (and who hasn't yet decided, probably doesn't care), so I don't see an issue here.

And those users can still opt-out by uninstalling (on beta or esr) or disabling (Daily). Only the place to take action just moves from the main window to the addons manager.

I don't understand the concerns of the last point, but I think the notification bar would provide much help here.

Comment 24

[Geoff Lankow (:darktrojan)]

I've made a Try run, as MakeMyDay said there were mozmill issues when the feature went in, so there might be when it comes out.

Comment 25

[Geoff Lankow (:darktrojan)]

And now I've remembered to paste a link to the Try run.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=af76d83d5de764f8cbca6453a2e9a25961c759ed

Comment 26

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #22)
> (In reply to Wayne Mery (:wsmwk) from comment #21)
> >   (we know a stink will be made by some users who "don't want lightning")
> 
> Not sure I ever heard from such users. 

Regardless, there have been bug reports, support and newsgroup reports, etc. (and hinted at in comment 7)


> > * How do we provide for situations where we want to test thunderbird without
> > lighting, without resorting to a different profile, either for performance
> > issues or reliability issues?
> 
> There is always -safe-mode

I am referring to comment 9 where it has been suggested to change to not be an add-on.  Are you saying there is no likelihood of going in that direction?

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8c63ea2dd667
Remove "Thunderbird now contains calendaring functionality..." message at startup. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/eaea65290635
Remove startup notification default pref from Thunderbird and SeaMonkey. r=mkmelin,frg,MakeMyDay

Comment 28

[Magnus Melin [:mkmelin]]

(In reply to Wayne Mery (:wsmwk) from comment #26)
> Regardless, there have been bug reports, support and newsgroup reports, etc.
> (and hinted at in comment 7)

There will always be reports when things aren't working as intended. 
But that's a different thing compared to having "forced integrated lightning". Look at chat - went in pretty unnoticed, is far less used, and still there was no outcry.

> I am referring to comment 9 where it has been suggested to change to not be
> an add-on.  Are you saying there is no likelihood of going in that direction?

Well, it's not the current plan.

Comment 29

[Worcester12345]

(In reply to Magnus Melin [:mkmelin] from comment #28)

> But that's a different thing compared to having "forced integrated
> lightning". Look at chat - went in pretty unnoticed, is far less used, and
> still there was no outcry.

> > an add-on.  Are you saying there is no likelihood of going in that direction?
> 
> Well, it's not the current plan.
Listen, a calendar is a LOT more useful to most people than "chat". Chat rooms, and newsgroups are more 80's/90's tech, wouldn't you agree?  Time to add this to the current plan. I'd say there is an outcry for the poorly integrated and currently nonfunctional calendar we have now.

Comment 30

[Worcester12345]

(In reply to Frank-Rainer Grahl (:frg) from comment #9)
> Personally I think Lightning should be fully integrated and no longer being
> build as an add-on which could be disabled. With the destruction of the
> classic add-on code this will probably be impossible soon anyway and I don't
> see that it would be possible as a web extension. For users who do not want
> it a disable pref for at least the menu items might do the job.


(In reply to Wayne Mery (:wsmwk) from comment #26)
> I am referring to comment 9 where it has been suggested to change to not be
> an add-on.  Are you saying there is no likelihood of going in that direction?


I just opened this bug to address this very thing:
https://bugzilla.mozilla.org/show_bug.cgi?id=1493008

Comment 31

[Magnus Melin [:mkmelin]]

To be clear I'm not really opposed to integrating lightning if the calendar people would like to go that route (but they don't). On the other hand there are some benefits with having it be an add-on as well. For instance, that way we are more likely to dogfood add-on support so other add-ons are also possible. 

As for integration level, I don't think there's anything user-facing that would change by integrating it lower level. It could do whatever integrations needed already.

Comment 32

[[:MakeMyDay]]

Geoff, why do we need to uplift this? It's not urgent, it has string changes and even if it just removes strings, it could be considered a late string change from l10n perspective.

Comment 33

[Geoff Lankow (:darktrojan)]

Well that's true, I hadn't considered the L10n point of view. I was coming from a "let's get rid of this for the first beta in a new cycle" angle.

Comment 34

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8999481 [details] [diff] [review]
1481777-startup-notification-1.diff

Review of attachment 8999481 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm I am not quite sure what happens with string changes on beta since the unified l10n thing. It should be ok since they are removals? Maybe you can check with Pike before you push.

Comment 35

[Jorg K (CEST = GMT+2)]

Axel, any comment in late string removals on beta? I removed two strings on ESR the other day, so I'm the even worse offender.

Comment 36

[Axel Hecht [:Pike]]

Yeah, removals are OK, thanks for asking.

Side note, we'll start tracking ESR in cross-channel with the next ESR branch. So the current one isn't in scope of x-channel.

Comment 37

[Jorg K (CEST = GMT+2)]

Beta (TB 63, Cal 6.5):
https://hg.mozilla.org/releases/comm-beta/rev/daa5920d9ee477ae72cbf9262ed21279928443f7
https://hg.mozilla.org/releases/comm-beta/rev/6ca93a79d47165d771bab829d0db22d07333fdb4

Comment 38

[Worcester12345]

I'm on 60 right now, and things are broken. What is the first pairing of Thunderbird/Lightning post 60 that works?

Comment 39

[Wayne Mery (:wsmwk)]

(In reply to Worcester12345 from comment #38)
> I'm on 60 right now, and things are broken. What is the first pairing of
> Thunderbird/Lightning post 60 that works?

Lightning is not working is an exception - it works in every beta and every release.  If you have trouble, consult the release notes https://www.thunderbird.net/en-US/thunderbird/60.0/releasenotes/

Comment 40

[Jorg K (CEST = GMT+2)]

TB 60 ESR works great with Lightning. Troubleshooting suggestions here:
https://support.mozilla.org/en-US/kb/calendar-updates-issues-thunderbird