563170 - Lightning 1.1a1pre broken, views remain empty - update to new addon manager API required

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Stefan Sitter [:ssitter]]

Lightning 1.1a1pre with Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100501 Shredder/3.2a1pre

Current Lightning nightly builds doesn't work in Shredder 3.2a1pre anymore due to changes in the extension manager. Observed behavior is that views remain empty and no events tasks are shown. Error Console shows messages similar to 

> Error: No timezones found! Please install calendar-timezones.xpi.
> 
> Error: Can't create calendar for e0791341-a2d5-4c0e-aae7-7eaea4029ff4
> (storage, moz-profile-calendar://): [Exception... "'<error>' when
> calling method: [calICalendar::id]"  nsresult: "0x804a0008
> (<unknown>)"  location: "JS frame ::
> file:///[...]/calendar-js/calCalendarManager.js :: cmgr_assureCache
> :: line 793"  data: no]
> 
> Error: uncaught exception: [Exception... "'[JavaScript Error:
> "this.mSelectByTzid is undefined" {file:
> "file:///[...]/calendar-js/calTimezoneService.js" line: 238}]' when
> calling method: [calITimezoneService::defaultTimezone]"  nsresult:
> "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location:
> "JS frame :: chrome://calendar/content/calUtils.js ::
> calendarDefaultTimezone :: line 212"  data: yes]

Reason is that calendar code uses extension manager to e.g. search for the timezone extension and probe for Lightning. This fails now with

> TypeError: Components.classes['@mozilla.org/extensions/manager;1'] is
> undefined

The timezone code falls back to the unitest case and tries to open the database from nonexistent location, causing another error. Code in question can be found at: http://mxr.mozilla.org/comm-central/search?string=extensions/manager&find=/calendar/

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

I believe a new addons manager landed. We need to find out about the new interface.

Comment 2

[Philip Chee]

There are some examples of how to use the new API here:
<http://www.oxymoronical.com/blog/tag/extension-manager>

Comment 3

[Stefan Sitter [:ssitter]]

It should work again with the next Shredder nightly build because the new addon manager has been backed out with Bug 563202 from comm-central.

We should keep this bug open to update to new addon manager API when it lands again.

Comment 4

[Daniel Kies]

This bug seems to be back since the new Add-ons Manager has landed again on the SeaMonkey trunk.  

See http://forums.mozillazine.org/viewtopic.php?f=6&t=1876345&p=9388795#p9388795 for some Error Console details.

Comment 5

[Peter Lairo]

Shouldn't this bug be importance = *blocker*? The calendar doesn't work at all, and thus "blocks development and/or testing work".

Comment 6

[Peter Lairo]

... and keyword "Regression"?

Comment 7

[Simon Paquet [:sipaq]]

This is not a regression, as we didn't regress anything. Source in the core changed.

And this is also not a blocker, as our main focus currently is not on 1.1a1pre builds.

Comment 8

[Daniel Kies]

pij has found an easy workaround for Shredder.  I can confirm it works with the SeaMonkey trunk too.  See

http://forums.mozillazine.org/viewtopic.php?f=6&t=1876345&p=9428309#p9428309

for the details.

Comment 11

[Tony Mechelynck [:tonymec]]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a6pre) Gecko/20100614 Lightning/1.1a1pre SeaMonkey/2.1a2pre - Build ID: 20100614000629

The forum posts linked in comment #8 are from Windows users, but the procedure works on Linux too, and I'm summarizing it here: create a directory <installdir>/extensions/[e-mail] then copy to it the file <profiledir>/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/timezones.sqlite where <installdir> and <profiledir> are the install directory and your profile directory respectively, for example on Linux they might be /usr/local/seamonkey and ~/.mozilla/seamonkey/aq2wz59x.default

If you're doing this from a bash prompt the braces around the Lightning directory-name GUID may need to be backslash-escaped.

Comment 12

[Tony Mechelynck [:tonymec]]

P.S. Of course the procedure has to be repeated every time you recreate the installdir (e.g. after installing the next mailer nightly) but even if you forgot and started SeaMonkey or Thunderbird first, no restart is necessary: the events appear the next time the Calendar tab is displayed or a different view (e.g. Multiweek) selected.

This comment might be regarded as bugspam at first thought, but (after some soul-searching) I'm posting it to avoid misguided bugs or comments saying "Your ?*!$ workaround stops working at the next install". Dear developers, maybe I'm the one misguided: please bear with me.

Comment 13

[Philipp Kewisch [:Fallen][📱📆]]

mschroeder, do you already have code for this? If not, I should look into this soon to get the nightlies back on track.

Comment 14

[Martin Schröder [:mschroeder]]

(In reply to comment #13)
> mschroeder, do you already have code for this? If not, I should look into this
> soon to get the nightlies back on track.

There are three places we use the old API. One place is the migration from old calendar extension. I suppose to remove this option completely, because nobody should have this installed in a Thunderbird 3.1 build. ;-) Second place we use the API is the detection of removing calendar provider addons. I'll attach some untested code to this bug. The third place is the most complicated one because we use the synchronous API for initializing the timezone service. But the new API works asynchronous. So, we have to think of another ways to safely initialize and ensure this with the new API. I'll also attach some code to show the usage of the new API which is once more untested and doesn't solve the (a)synchronous problem.

Comment 15

[Martin Schröder [:mschroeder]]

Attachment: Untested code for new addon API

Comment 16

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v1

This patch should take care. I've created a new calstartupService that takes care of starting up our services in order. We might be able to re-use this service for future command line handler(s) too. Requesting review from mschroeder since he's also contributed code, I'll take anyone's review though.

It would be nice to get this in in the next few days, since the massive changes to the component registration are also going to land soon.

Comment 17

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v2

Something's not right with my upload extension, where did the rest of that patch go?

Comment 18

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v3

This patch fixes a few issues that v2 had.

Comment 19

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v4

This patch fixes an issue with the migration dialog that the last patch would regress.

Comment 20

[Martin Schröder [:mschroeder]]

Comment on attachment 463921 [details] [diff] [review]
Fix - v4

r=mschroeder (but almost no testing!) with following considered:

diff --git a/calendar/base/src/calCalendarManager.js b/calendar/base/src/calCalendarManager.js
--- a/calendar/base/src/calCalendarManager.js
+++ b/calendar/base/src/calCalendarManager.js

[...]

+var gCalendarManagerAddonListener = {

[...]

+    queryUninstallProvider: function(aExtension) {

> I would prefer naming the argument 'aAddon'.

[...]

-    shutdown: function ccm_shutdown() {
+    shutdown: function ccm_shutdown(aCompleteListener) {

[...]

+        AddonManager.removeAddonListener(aListener);

> This needs to be 'gCalendarManagerAddonListener' as argument!

[...]

+            // Disable Lightning
+            AddonManager.getAddonByID("{e2fda1a4-762b-4020-b5ad-a41df1933103}", function getLightningExt(addon) {
+                addon.userDisabled = true;
+                startup.quit(Components.interfaces.nsIAppStartup.eRestart |
+                    Components.interfaces.nsIAppStartup.eForceQuit);
+            });

> Better use aAddon as function argument.

diff --git a/calendar/base/src/calItemModule.js b/calendar/base/src/calItemModule.js
--- a/calendar/base/src/calItemModule.js
+++ b/calendar/base/src/calItemModule.js
+    // This should be last, or at least after all services it calls
+    {cid: Components.ID("{2547331f-34c0-4a4b-b93c-b503538ba6d6}"),
+     contractid: "@mozilla.org/calendar/startup-service;1",
+     script: "calStartupService.js",
+     constructor: "calStartupService",
+     categories: {"app-startup": "calendar-startup-startup"},
      service: true}

> 'app-startup' is no longer available to addons. The earliest is 'profile-after-change'.

diff --git a/calendar/base/src/calTimezoneService.js b/calendar/base/src/calTimezoneService.js
--- a/calendar/base/src/calTimezoneService.js
+++ b/calendar/base/src/calTimezoneService.js

In general:
> Better use cal-prefixed version of calGetString in calTimezoneService.js.

[...]

+    // nsIStartupService
+    startup: function startup(aCompleteListener) {
+        this.initialze(aCompleteListener);
+    },

> Function declaration and this call should be named 'initialize'.

[...]

+    _initDB: function _initDB(sqlTzFile) {

[...]

+            let dbService = Components.classes["@mozilla.org/storage/service;1"]
+                                       .getService(Components.interfaces.mozIStorageService);

> Align on '.'.

[...]

+    initialze: function calTimezoneService_initialze(aCompleteListener) {

> This declaration and function call should be named 'initialize'.

[...]

+        function wereDone() {
+            aCompleteListener.onResult(null, Components.results.NS_OK);
+        }

> I would prefer that you use 'isDone' as function name.

[...]

+        AddonManager.getAddonByID(kCalendarTimezonesXpiId, function getTimezoneExt(addon) {

> Better use aAddon as argument.

[...]

For every occurrence:
+                let sqlTzFile = toFile(addon.getResourceURI(''));

There has not to be an argument to getResourceURI, and we should not need an extra 'toFile' function because the getResourceURI code calls Services.io.newFileURI() which returns a nsIURI and so nsIURI.file should give the nsIFile. But I do not have the time to verify this now.

Comment 21

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to comment #20)
> +    // This should be last, or at least after all services it calls
> +    {cid: Components.ID("{2547331f-34c0-4a4b-b93c-b503538ba6d6}"),
> +     contractid: "@mozilla.org/calendar/startup-service;1",
> +     script: "calStartupService.js",
> +     constructor: "calStartupService",
> +     categories: {"app-startup": "calendar-startup-startup"},
>       service: true}
> 
> > 'app-startup' is no longer available to addons. The earliest is 'profile-after-change'.

This will be fixed with bug 576746. If I change it now then I have to fix the patches to that bug too.


> +        function wereDone() {
> +            aCompleteListener.onResult(null, Components.results.NS_OK);
> +        }
> 
> > I would prefer that you use 'isDone' as function name.

Hehe I was thinking "we're done" when I wrote this function, but now I see that it looks like "(we) were done" :-) I've changed it to simply "done", since isDone sounds like a function to check if we are done.

> For every occurrence:
> +                let sqlTzFile = toFile(addon.getResourceURI(''));
> 
> There has not to be an argument to getResourceURI, and we should not need an
> extra 'toFile' function because the getResourceURI code calls
> Services.io.newFileURI() which returns a nsIURI and so nsIURI.file should give
> the nsIFile. But I do not have the time to verify this now.

A pure nsIURI doesn't have a .file attribute per se. While the code does seem to call newFileURI (which returns an nsIFileURI), I think we should rather rely on the outer API, which advertises an nsIURI.

Also, per API there is an argument "in string path" to getResourceURI, so I do think it needs an argument.

Comment 22

[Philipp Kewisch [:Fallen][📱📆]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/87bf451a8077>
-> FIXED

Comment 23

[Rob S]

Who says this is fixed?

I spent a week trying to get any of the recent nightlies of Lightning to work with either 2.1B1 or recent versions of 2.1B2 and could not - see comments here.

Others having same problem, so I reverted back to 2.0.

http://forums.mozillazine.org/viewtopic.php?f=6&t=2040389

This one is much more important than the size display issue!!!

Comment 24

[Stefan Sitter [:ssitter]]

Rob, this issue has been fixed and trunk builds were working for some time. But new problems were raised by new changes in the Mozilla toolkit. The newer problems are tracked by newer bugs.