300968 - Replicating (downloading) from a LDAP directory server while adding fails

Status: RESOLVED FIXED

Description

[Bram van Leur]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050712 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050712 Firefox/1.0+

When adding a LDAP directory server to the directories the "Offline" tab and the
"Download Now" button are available. Download now does result in the replication
progress dialog with the dialog title ending with (null) instead of the current
description and the process fails at all.

This is probably caused by the fact that the replication service uses a
preference name and the preference (and addressbook entry) isn't created yet. I
guess it's reasonably solved by replacing the "Download Now" button with a label
explaining that the entry must be accepted first by hitting OK as long as no
preference is available yet (gCurrentDirectory still is null).



Reproducible: Always

Steps to Reproduce:

Comment 1

[Mark Banner (:standard8)]

Confirming on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051006
SeaMonkey/1.1a.

We also get this on the console:
replication failed.  ex=[Exception... "Component returned failure code:
0x8000ffff (NS_ERROR_UNEXPECTED) [nsIAbLDAPReplicationService.startReplication]"
 nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame ::
chrome://messenger/content/addressbook/replicationProgress.js :: Replicate ::
line 113"  data: no]

Also moving to core address book as it affects both Thunderbird & SeaMonkey.

Comment 2

[Mark Banner (:standard8)]

I'm working on a patch for this.

Comment 3

[Mark Banner (:standard8)]

Created attachment 199094 [details] [diff] [review]
Patch v1

This patch does the following:

* Disables the download now button for new LDAP address books.
* Adds text saying that you have to save the properties first (only for the new
LDAP address books)
* Fixes a direct alert call to use the prompt service.
* Returns a value from OnAccept so that if a value is invalid, the user has a
chance to change it and not loose all their settings so far...

Comment 4

[[e-mail]]

Comment on attachment 199094 [details] [diff] [review]
Patch v1

>+              <description id="downloadDisabledMsg" hidden="true" style="color: #FF0000;"/>
You should put an entity for the text in here rather than using the JS to
retreive it from a string bundle. You should also style this element in themes.

Comment 5

[Mark Banner (:standard8)]

Created attachment 199820 [details] [diff] [review]
Patch v2

This patch does the following on top of v1:

* defines the style in the themes
* defines the description in the dtd.

Comment 6

[[e-mail]]

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

>+      var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].getService(Components.interfaces.nsIPromptService);
Nit: line is a bit long :-P

>+                          addressBookBundle.getString("invalidItemTitle"),
Using document.title might suffice?

Comment 7

[Mark Banner (:standard8)]

Created attachment 200101 [details] [diff] [review]
Patch v3

Addressed Neil's nits and carrying forward his r+.

Comment 8

[Mark Banner (:standard8)]

Patch v3 checked in:
/cvsroot/mozilla/mailnews/addrbook/prefs/resources/content/pref-directory-add.xul,v
 <--  pref-directory-add.xul
new revision: 1.23; previous revision: 1.22
done
Checking in mailnews/addrbook/prefs/resources/content/pref-directory-add.js;
/cvsroot/mozilla/mailnews/addrbook/prefs/resources/content/pref-directory-add.js,v
 <--  pref-directory-add.js
new revision: 1.26; previous revision: 1.25
done
Checking in mailnews/addrbook/prefs/resources/locale/en-US/pref-directory-add.dtd;
/cvsroot/mozilla/mailnews/addrbook/prefs/resources/locale/en-US/pref-directory-add.dtd,v
 <--  pref-directory-add.dtd
new revision: 1.11; previous revision: 1.10
done
Checking in mail/locales/en-US/chrome/messenger/addressbook/pref-directory-add.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/addressbook/pref-directory-add.dtd,v
 <--  pref-directory-add.dtd
new revision: 1.4; previous revision: 1.3
done
Checking in mail/themes/pinstripe/mail/messenger.css;
/cvsroot/mozilla/mail/themes/pinstripe/mail/messenger.css,v  <--  messenger.css
new revision: 1.3; previous revision: 1.2
done
Checking in mail/themes/qute/mail/messenger.css;
/cvsroot/mozilla/mail/themes/qute/mail/messenger.css,v  <--  messenger.css
new revision: 1.2; previous revision: 1.1
done
Checking in themes/classic/global/mac/global.css;
/cvsroot/mozilla/themes/classic/global/mac/global.css,v  <--  global.css
new revision: 1.42; previous revision: 1.41
done
Checking in themes/classic/global/unix/global.css;
/cvsroot/mozilla/themes/classic/global/unix/global.css,v  <--  global.css
new revision: 1.10; previous revision: 1.9
done
Checking in themes/classic/global/win/global.css;
/cvsroot/mozilla/themes/classic/global/win/global.css,v  <--  global.css
new revision: 1.44; previous revision: 1.43
done
Checking in themes/modern/communicator/profile/profile.css;
/cvsroot/mozilla/themes/modern/communicator/profile/profile.css,v  <--  profile.css
new revision: 1.12; previous revision: 1.11
done
Checking in themes/modern/global/global.css;
/cvsroot/mozilla/themes/modern/global/global.css,v  <--  global.css
new revision: 1.52; previous revision: 1.51

Comment 9

[Mark Banner (:standard8)]

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

Requesting 1.8.1 branch approval for this patch. It will not let the user try to replicate an ldap directory until it has actually been created. This will also be needed to allow bug 133883 onto the branch as well.

Comment 10

[Ian Neal]

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

a=me for SM1.1