Closed Bug 133883 Opened 19 years ago Closed 15 years ago

No error msg when user tries to do replication when offline

Categories

(MailNews Core :: LDAP Integration, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: yulian, Assigned: standard8)

References

Details

(Keywords: fixed-seamonkey1.1a, fixed1.8.1)

Attachments

(1 file, 4 obsolete files)

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.
Attached patch initial patch to fix this (obsolete) — Splinter Review
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
Blocks: 154225
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.
Good suggestions.
QA Contact: yulian → gchan
Product: MailNews → Core
I'm guessing Rajiv isn't around or working on this bug any more -> taking. I have an idea for fixing this.
Assignee: rdayal → bugzilla
OS: Windows NT → All
Hardware: PC → All
Attached patch Indicate when offline. (obsolete) — Splinter Review
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.
Attachment #84313 - Attachment is obsolete: true
Attachment #204966 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #204966 - Flags: review?(neil.parkwaycc.co.uk) → review-
(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.
Attached patch Indicate when offline v2 (obsolete) — Splinter Review
Addresses Neil's review comments (see also my comment 7).
Attachment #204966 - Attachment is obsolete: true
Attachment #206939 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #206939 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attached patch Indicate when offline v3 (obsolete) — Splinter Review
addressed Neil's nit, carrying forward his r, requesting sr.
Attachment #206939 - Attachment is obsolete: true
Attachment #207067 - Flags: superreview?(bienvenu)
Attachment #207067 - Flags: review+
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.
v4 with David's nits addressed.
Attachment #207084 - Flags: review+
Attachment #207067 - Flags: superreview?(bienvenu) → superreview+
Attachment #207067 - Attachment is obsolete: true
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
Attachment #207084 - Attachment description: Indicate when offline v4 → Indicate when offline v4 (checked into trunk)
Attachment #207084 - Flags: superreview+
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)
Attachment #207084 - Flags: approval1.8.1?
Depends on: 300968
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.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 207084 [details] [diff] [review]
Indicate when offline v4 (checked into trunk)

I also approved Bug 300968 which this bug needs.
Attachment #207084 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 207084 [details] [diff] [review]
Indicate when offline v4 (checked into trunk)

a=me for SM1.1
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.