1652885 - Dynamic registration and unregistration of calendar providers

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect, P1)

Description

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

Attachment: Fix - v0

Calendar Providers are currently xpcom components, coming from the era of legacy xul add-ons. We need a way to dynamically register these, so that a WebExtension can register a provider on enable and remove it on disable/uninstall.

When a provide is unregistered, the calendars for that provider need to turn into calDummyCalendars, which gives them the look like the provider is missing.

I'm uploading v0 of the patch so others can test, this is missing tests.

Comment 1

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

The patch in bug 1652888 might be necessary for this to fully work.

Comment 2

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

Attachment: Bug 1652885 - Dynamic registration and unregistration of calendar providers. r?pmorris!

Comment 3

[Wayne Mery (:wsmwk)]

We will want this for 78.1.1

Comment 4

[Paul Morris [:pmorris]]

I reviewed this on Monday (27th), but I forgot to set "request changes" in phabricator when I did it. Just fixed that now so the status is clearer.

Comment 5

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

Try run with review items fixed: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=edf2dfeffb05c23a31027297e141279a92b570da

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to Philipp Kewisch [:Fallen] [:📆][:🧩] from comment #5)

Try run with review items fixed: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=edf2dfeffb05c23a31027297e141279a92b570da

Looks successful and with Paul's comment "With a successful try run and the comments addressed, this is ready to land." we can take this and bug 1652888 for a beta 80.0b1 rebuild.

Can you land this on c-c Sunday morning?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7ed9c0ba9c40
Dynamic registration and unregistration of calendar providers. r=pmorris

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9166239 [details]
Bug 1652885 - Dynamic registration and unregistration of calendar providers. r?pmorris!

Approved for beta - needed for gdata provider

Rob will be landing on c-c

Comment 9

[Rob Lemley [:rjl]]

Comment on attachment 9166239 [details]
Bug 1652885 - Dynamic registration and unregistration of calendar providers. r?pmorris!

[Triage Comment]
Per Wayne in Matrix.

Comment 10

[Rob Lemley [:rjl]]

Thunderbird 80.0b1 (build4):
https://hg.mozilla.org/releases/comm-beta/rev/ebd9239357fc

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/df4787c209e6
follow-up - Mark second argument of calICalendarManager.unregisterCalendarProvider as optional. rs=bustage-fix

Comment 12

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

Comment on attachment 9166239 [details]
Bug 1652885 - Dynamic registration and unregistration of calendar providers. r?pmorris!

[Approval Request Comment]
Developer impact if declined: Developers will need to copy/paste a lot of lines from the CalCalendarManager that replicate a core component of the calendar code, registering and unregistering calendars. If we update this code in the future, developers may still stick to the original code which could cause unknown side effects for those and other calendars.
User impact if declined: Less likelihood of provider extension developers upgrading to 78, which means less users can benefit from these extensions.
Testing completed (on c-c, etc.): Testing with the Provider for Google Calendar on Thunderbird 80.0b1 went well. No know reports for calendars being broken, though you probably have a better idea of it.
Risk to taking this patch (and alternatives if risky): This patch makes changes to calendar registration, if something goes wrong it could mean users don't have access to their calendars. The patch was written with backporting in mind so I deem the risk to be low.

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9166239 [details]
Bug 1652885 - Dynamic registration and unregistration of calendar providers. r?pmorris!

[Triage Comment]
Approved for esr78

(by the time this gets released on esr, beta will have been live for several days - but will want to recheck BMO before releasing 78.1.1)

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 78.1.1:
https://hg.mozilla.org/releases/comm-esr78/rev/7f2dc693ba52
https://hg.mozilla.org/releases/comm-esr78/rev/cbbe23f3d23f

Comment 15

[Rob Lemley [:rjl]]

Thunderbird 80.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/8428a04b2d4a

Comment 16

[Lasana Murray]

I don't think the dummy calendar implemented in this was a good idea. It leaves users with invalid albeit disabled calendars without warning. Users can enable those calendars and attempt to use them but it results in an error being thrown with no visual indication.