133883 - No error msg when user tries to do replication when offline

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[yulian chang]

20020327 trunk builds on Wins

Error message, "You must be online to replicate a directory", is expected when
user tries to replicate LDAP Directory server while offline.

Comment 1

[Rajiv Dayal]

Attachment: initial patch to fix this

patch with all related code changes to catch the error and display appropriate
message if the user is offline when doing AB LDAP replication

Jennifer could you please review the below string :
+offlineError=You are offline, please go online for Address Book replication

Comment 2

[jglick]

Name of dialog should be the ProductName.

If possible: 
"You must be online in order to replicate a directory server. Would you like to 
go online now?" Yes - attempt to go online and then replicate. No - close dialog 
and return to Directory Server Properites dialog. 

Otherwise:
"You must be online in order to replicate a directory server." OK button. Close 
dialog and return to Dir Server Prop dialog.

Comment 3

[robinf]

Good suggestions.

Comment 4

[Mark Banner (:standard8)]

I'm guessing Rajiv isn't around or working on this bug any more -> taking. I have an idea for fixing this.

Comment 5

[Mark Banner (:standard8)]

Attachment: Indicate when offline.

This patch will disable the download button and notify the user with some text to say that they can't download because they are offline (and dynamically update it if the situation changes). I think this is better than the previous suggested fixes on this bug as they were suggesting extra dialogs to warn the user - we may as well tell them straight away if we know they are offline.

Comment 6

[[e-mail]]

Comment on attachment 204966 [details] [diff] [review]
Indicate when offline.

>+function setDownloadOfflineOnlineState(isOffline)
>+{
>+  if (!gPrefInt.prefIsLocked(gCurrentDirectoryString + ".disable_button_download"))
Then why bother adding the observer? Aside: I get the feeling that gPrefInt should be a branch.

>+      document.getElementById("downloadDisabledMsg").value = document.
>+        getElementById("bundle_addressBook").
>+        getString("addressbookofflinewarning");
Double trouble: the "new directory" error, stored in the .dtd, is the text content of the label, while the "offline" error, stored in the .properties, is the value of the label.

Nit: I hadn't notice that it was possible to lock the button, in that case I guess it would be pointless to show the "new directory" error.

Comment 7

[Mark Banner (:standard8)]

(In reply to comment #6)
> Aside: I get the feeling that gPrefInt
> should be a branch.

Agreed, I'll do that in a follow up patch.

> Nit: I hadn't notice that it was possible to lock the button, in that case I
> guess it would be pointless to show the "new directory" error.

The lock is only possible for a specific directory in prefs, i.e. one that already exists, so the new directory error is still always needed.

Comment 8

[Mark Banner (:standard8)]

Attachment: Indicate when offline v2

Addresses Neil's review comments (see also my comment 7).

Comment 9

[[e-mail]]

Comment on attachment 206939 [details] [diff] [review]
Indicate when offline v2

Nit: you may wish to change .value to .textContent in case other locales require wrapped text.

Comment 10

[Mark Banner (:standard8)]

Attachment: Indicate when offline v3

addressed Neil's nit, carrying forward his r, requesting sr.

Comment 11

[David :Bienvenu]

Comment on attachment 207067 [details] [diff] [review]
Indicate when offline v3

+    document.getElementById("downloadWarningMsg").hidden = false;
+    document.getElementById("download").disabled = true;
+  }
+  else
+  {
+    // Enable the download button and remove the text
+    document.getElementById("downloadWarningMsg").hidden = true;
+    document.getElementById("download").disabled = false;


this could just be

document.getElementById("downloadWarningMsg).hidden = !isOffline;
document.getElementById("download").disabled = isOffline;


sr=bienvenu with that fixed.

Comment 12

[Mark Banner (:standard8)]

Attachment: Indicate when offline v4 (checked into trunk)

v4 with David's nits addressed.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 207084 [details] [diff] [review]
Indicate when offline v4 (checked into trunk)

Carrying forward bienvenu's sr and checked into trunk:

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.27; previous revision: 1.26
done
Checking in mailnews/addrbook/prefs/resources/content/pref-directory-add.xul;
/cvsroot/mozilla/mailnews/addrbook/prefs/resources/content/pref-directory-add.xul,v  <--  pref-directory-add.xul
new revision: 1.24; previous revision: 1.23
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.12; previous revision: 1.11
done
Checking in mailnews/addrbook/resources/locale/en-US/addressBook.properties;
/cvsroot/mozilla/mailnews/addrbook/resources/locale/en-US/addressBook.properties,v  <--  addressBook.properties
new revision: 1.38; previous revision: 1.37
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.5; previous revision: 1.4
done
Checking in mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties,v  <--  addressBook.properties
new revision: 1.10; previous revision: 1.9

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 207084 [details] [diff] [review]
Indicate when offline v4 (checked into trunk)

Requesting 1.8.1 branch approval. This is a low risk patch that correctly indicates the offline state for ldap when attempting to download. Bug 300968 will be needed to be checked in before this one (approval also requested)

Comment 15

[Mark Banner (:standard8)]

This was checked into trunk a while ago, therefore I'm marking as fixed. I'll
check into branch if the patch eventually gets approval.

Comment 16

[Scott MacGregor]

Comment on attachment 207084 [details] [diff] [review]
Indicate when offline v4 (checked into trunk)

I also approved Bug 300968 which this bug needs.

Comment 17

[Ian Neal]

Comment on attachment 207084 [details] [diff] [review]
Indicate when offline v4 (checked into trunk)

a=me for SM1.1