280743 - Headers lack chrome - gCollapsedHeaderList has no properties

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

1. Install a new build.
2. Create a profile.
3. Read mail (I'm using IMAP).
4. Note the complete lack of chrome mail headers due to:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult:
"0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame ::
chrome://messenger/content/msgHdrViewOverlay.js :: <TOP_LEVEL> :: line 79" 
data: no]

Error: gCollapsedHeaderList has no properties
Source File: chrome://messenger/content/msgHdrViewOverlay.js
Line: 189

79 var abAddressCollector =
Components.classes[abAddressCollectorContractID].getService(Components.interfaces.nsIAbAddressCollecter);

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

[01:10] <timelyx> you're missing 
"@mozilla.org/addressbook/services/addressCollecter;1"
[01:11] <timelyx> file a bug
[01:11] <timelyx> the code should try { getService() }
[01:11] <timelyx> instead of dying

Comment 2

[[e-mail]]

Why are you missing the address collecter (sic)?

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

Because my address book is empty ;-(  As soon as I populate it, shut down Moz
and restart, I get my header chrome back again.

Comment 4

[[e-mail]]

OK, so the address collector is trying to collect to the personal address book
which hasn't been created yet? Who actually creates the personal address book?

Comment 5

[Mark Banner (:standard8)]

hmm, look like my fix for bug 66410 (deleting address book should also delete
corresponding mab) caused this. The errors start to occur before the mailnews
account manager starts up.

Does anyone know of a way where we could hook a create off part of the
installation procedure or something when mozilla first starts up?

I'll have a look into it, but if I haven't got a fix by the weekend then I'll
get bug 66410 backed out so that we are clear for beta.

Comment 6

[[e-mail]]

So what do we want to do when the collected address book is deleted?

Comment 7

[Mark Banner (:standard8)]

I think this is what we want to do (pref = any pref for collecting addresses):

1) If pref is disabled - don't use/load address collector (which
msgHdrViewOverlay.js tries to at the moment)

2) If pref is enabled and user tries to delete the address book it points to
then we should either not allow the delete full stop (telling them why probably)
or if we do then we should let them know we are turning the pref off.

3) If pref is enabled, and the database file doesn't exist on start up, then we
should try and create it (move create option to be specified in call to
GetAbDatabaseFromURI so only address collector sets it to true).

Any comments?

Should we also be trying to bullet-proof address collector so it can be run
without a valid database file?

(Apart from how do we create the PAB at the moment - I've still got to look that up)

Comment 8

[Mark Banner (:standard8)]

Attachment: Patch

This patch allows the collector to create the database if required, and stops
msgHdrViewOverlay.js trying to use the collector when it isn't needed.

Note that I did find out that there is a part of the code that will create the
PAB (and currently the CAB) on startup via a different route than
GetAbDatabaseFromURI. Unfortunately, mailnews normally kicks in before this and
hence gives us the problem we see.

I've tried various different scenarios with this patch and will keep trying
over the weekend.

I haven't addressed the deletion of an address book which is pointed to by the
collected addresses preference, as it looks like it has possibly been in there
for a while we can keep this bug open (or raise another one) to address later.

Neil will you be able to consider this for review before the freeze? If not
I'll get one of the others to or get the other patch backed out.

Comment 9

[[e-mail]]

Comment on attachment 173423 [details] [diff] [review]
Patch

I think I would prefer deferring getting the addresss collector until we have
an address to collect.

Comment 10

[Mark Banner (:standard8)]

Attachment: Patch v2

Altered patch so that msgHdrViewOverlay only gets abAddressCollector when it is
really needed.

Comment 11

[[e-mail]]

Comment on attachment 173456 [details] [diff] [review]
Patch v2

Won't the address book have been created by the time you need it?

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 173456 [details] [diff] [review]
Patch v2

Neil's right, we don't need to have the extra options to create if we are
holding the Msg Header code from getting the address collector until we need
it.

Comment 13

[Mark Banner (:standard8)]

Attachment: Patch v3

This patch fixes both mailnews and tb. Whilst it is sufficient for mailnews to
not create the file via the address collector, TB tries to load the files
elsewhere for other things before the address book is initialised. Hence I've
modified the has card for email address and associated functions, so that if
the file doesn't exist, we return false as it's not a possiblity anyway.

I've tested both of them in various ways and haven't found a problem yet.

Comment 14

[[e-mail]]

Comment on attachment 173531 [details] [diff] [review]
Patch v3

Note: I'm not convinced that this is the best approach.

>   nsresult rv = NS_OK;
>   *aCardExists = PR_FALSE;
> 
>   if (!mDatabase)
>     rv = GetAbDatabase();
>+  if (rv == NS_ERROR_FILE_NOT_FOUND)
>+  {
>+    // If file wasn't found, the card cannot exist.
>+    *aCardExists = PR_FALSE;
>+    return NS_OK;
>+  }
>   NS_ENSURE_SUCCESS(rv, rv);
aCardExists has already been set to PR_FALSE :-P

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 173531 [details] [diff] [review]
Patch v3

David, if I promise to deal with Neil's nit before check-in, will you SR this
in time for freeze? (I'm not at home where my dev environment is).

Comment 16

[David :Bienvenu]

Comment on attachment 173531 [details] [diff] [review]
Patch v3

+    if (NS_SUCCEEDED(rv))
+    {
+      rv = mDatabase->AddListener(this);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+    else
+      return rv;
   }

this can all be simplified. 

if (NS_SUCCEEDED(rv))
  rv = mDatabase->AddListener;
return rv;

and since we're always returning from the if (!mDatabase) clause, we don't need
the code after it, so we can move the return rv to after if (!mDatabase).

+var abAddressCollector = null;
you don't need to init js globals to null, do you? Neil would 
know for sure...

sr=bienvenu, modulo those comments.

Comment 17

[[e-mail]]

They default to "undefined" so an explicit null is neater.

Comment 18

[Mark Banner (:standard8)]

Attachment: Patch for checkin

Patch for checkin that incorporates the comments from Neil and David.

Comment 19

[Mark Banner (:standard8)]

Fix checked in by timeless:

2005-02-07 11:09	timeless%mozdev.org

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED with the 2005-02-09-05 Seamonkey trunk build on Windows XP.

Thanks, Mark.