Address Book vanishes if it is renamed as blank

VERIFIED FIXED

Status

SeaMonkey
MailNews: Address Book & Contacts
VERIFIED FIXED
13 years ago
13 years ago

People

(Reporter: malika.jaiswal, Assigned: standard8)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050319
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050319

Address Book vanishes if its name is made to blank or null. 

Reproducible: Always

Steps to Reproduce:
1.Create an Addressbook named "AD".
2.Create some mailing lists and cards under it.
3.Now rename the addressbook name as " ".
4.Close the browser and restart again.

Actual Results:  
The Addressbook which was renamed to " " vanishes along with its entries.

Expected Results:  
The Addressbook which was renamed to " " should not have vanished along with its
entries.

Comment 1

13 years ago
confirming the bug on lates mozilla1.7.6 for OS/2.

Comment 2

13 years ago
This seems a serious bug because I am able to create addressbook with no name and
I can also rename some existing address book with blank name. Next time whenever
you restart the browser the address books vanishes. You cant access those mailing
lists and cards which were there in that address book.

Another problem is this leaves all entries in prefs.js. If you go on creating no
named address book, IT WILL UNNECESSARILY BLOATs the file.

Comment 3

13 years ago
I think we should stop creation of Address book with no names.
This can be done by checking the names entered for address book in 
AbOnCreateNewAddressBook(aName) and AbOnRenameAddressBook(aName) name routines.

328 function AbOnCreateNewAddressBook(aName)
329 {
330   var properties =
Components.classes["@mozilla.org/addressbook/properties;1"].createInstance(Components.interfaces.nsIAbDirectoryProperties);
331   properties.description = aName;
332   properties.dirType = kPABDirectory;
333   top.addressbook.newAddressBook(properties);
334 }

replace this with

328 function AbOnCreateNewAddressBook(aName)
329 {
330   var name=aName.replace(/(^\s+)|(\s+$)/g, '');//abhi    
331   var properties =
Components.classes["@mozilla.org/addressbook/properties;1"].createInstance(Components.interfaces.nsIAbDirectoryProperties);
332   if(name!="")//abhi
333      {
334        properties.description = name;//abhi
335        properties.dirType = kPABDirectory;
336        top.addressbook.newAddressBook(properties);
337       }   
338 }


and aslo 
adding  var name=aName.replace(/(^\s+)|(\s+$)/g, '');//abhi this to function 
336 function AbOnRenameAddressBook(aName)
along with the following changes

364   properties.description = aName;
365 
366   // Now do the modification.
367   addressbook.modifyAddressBook(addressbookDS, parentDir,
selectedABDirectory, properties);

modified to

364   properties.description = name;//abhi
365   if(name!="") //abhi
366   // Now do the modification.
367   addressbook.modifyAddressBook(addressbookDS, parentDir,
selectedABDirectory, properties);

is working fine for me
(Assignee)

Comment 4

13 years ago
Confirming on linux build 20050412, and taking.
Assignee: sspitzer → bugzilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 5

13 years ago
Created attachment 180630 [details] [diff] [review]
Patch v1.

This patch works for both the suite & thunderbird, and stops address books
being created as or renamed with an empty name or with just whitespace
characters.

I decided to add the non-whitespace character requirement after talking to
various people on IRC and realising that with a whitespace character only name
some of the address book lists would show up with blank entries which could be
confusing to the user.

At the moment I'm only limiting it at the user-end. LDAP and other ways of
creating address books may need to be considered later.
Attachment #180630 - Flags: superreview?(mscott)
Attachment #180630 - Flags: review?(neil.parkwaycc.co.uk)

Comment 6

13 years ago
Comment on attachment 180630 [details] [diff] [review]
Patch v1.

How about saying something like:

"An address book name must contain at least one letter or digit."
(Assignee)

Comment 7

13 years ago
Comment on attachment 180630 [details] [diff] [review]
Patch v1.

Revising text.
Attachment #180630 - Flags: superreview?(mscott)
Attachment #180630 - Flags: review?(neil.parkwaycc.co.uk)
(Assignee)

Comment 8

13 years ago
Created attachment 180635 [details] [diff] [review]
Patch v2

I agree, scott's suggestion for the message error text is better than my
original one.
Attachment #180630 - Attachment is obsolete: true
Attachment #180635 - Flags: superreview?(mscott)
Attachment #180635 - Flags: review?(neil.parkwaycc.co.uk)

Comment 9

13 years ago
The other approach would be to disable the OK button if the name was invalid.

Note that !/\S/.test(abName) will be true anyway if abName == "".
(Assignee)

Comment 10

13 years ago
Scott, would you prefer to have the ok button disabled if the name is invalid, 
or a dialog that came up when the user pressed ok?
(Assignee)

Comment 11

13 years ago
Comment on attachment 180635 [details] [diff] [review]
Patch v2

Ok, I'm going with the flow and Neil's suggestion - if the name is invalid,
then we will disable the ok button. This is what happens with folders in mail
accounts,  so we'll apply that here as well.
Attachment #180635 - Attachment is obsolete: true
Attachment #180635 - Flags: superreview?(mscott)
Attachment #180635 - Flags: review?(neil.parkwaycc.co.uk)
(Assignee)

Comment 12

13 years ago
Created attachment 180907 [details] [diff] [review]
Patch v3

Ok revised patch so that now we only enable the button when it's valid. TB &
Suite versions included.
Attachment #180907 - Flags: review?(neil.parkwaycc.co.uk)

Comment 13

13 years ago
Comment on attachment 180907 [details] [diff] [review]
Patch v3

Unfortunately you forgot the case when you're creating a new address book.

Bug 290023 requests that leading and trailing spaces are removed from mailing
list names, so perhaps that would be a good idea here too?
Attachment #180907 - Flags: review?(neil.parkwaycc.co.uk) → review-
(Assignee)

Comment 14

13 years ago
Created attachment 182164 [details] [diff] [review]
Patch v4

This patch fixes the new address book case and adds stripping of spaces from
the start & end of address book names.
Attachment #180907 - Attachment is obsolete: true
Attachment #182164 - Flags: review?(neil.parkwaycc.co.uk)

Comment 15

13 years ago
Comment on attachment 182164 [details] [diff] [review]
Patch v4

>+	gNameInput = document.getElementById('name');
Oops, this is a tab, it should be two spaces :-P r=me with this fixed.
Attachment #182164 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Comment 16

13 years ago
Created attachment 182180 [details] [diff] [review]
Patch v5 (Checked in)

new patch to fix tab nit, carrying forward neil's r, requesting sr.
Attachment #182164 - Attachment is obsolete: true
Attachment #182180 - Flags: superreview?(bienvenu)
Attachment #182180 - Flags: review+

Updated

13 years ago
Attachment #182180 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 17

13 years ago
Comment on attachment 182180 [details] [diff] [review]
Patch v5 (Checked in)

Requesting approval. This is a low risk patch for both suite & tb, and stops us
"loosing" data by not allowing blank address book names.
Attachment #182180 - Flags: approval1.8b2?
Attachment #182180 - Flags: approval-aviary1.1a?

Comment 18

13 years ago
Comment on attachment 182180 [details] [diff] [review]
Patch v5 (Checked in)

a=chofmann
Attachment #182180 - Flags: approval1.8b2?
Attachment #182180 - Flags: approval1.8b2+
Attachment #182180 - Flags: approval-aviary1.1a?
Attachment #182180 - Flags: approval-aviary1.1a+

Comment 19

13 years ago
Comment on attachment 182180 [details] [diff] [review]
Patch v5 (Checked in)

Checking in mailnews/addrbook/resources/content/abAddressBookNameDialog.js;
/cvsroot/mozilla/mailnews/addrbook/resources/content/abAddressBookNameDialog.js
,v  <--  abAddressBookNameDialog.js
new revision: 1.9; previous revision: 1.8
done
Checking in mailnews/addrbook/resources/content/abAddressBookNameDialog.xul;
/cvsroot/mozilla/mailnews/addrbook/resources/content/abAddressBookNameDialog.xu
l,v  <--  abAddressBookNameDialog.xul
new revision: 1.17; previous revision: 1.16
done
Checking in mail/components/addrbook/content/abAddressBookNameDialog.js;
/cvsroot/mozilla/mail/components/addrbook/content/abAddressBookNameDialog.js,v 
<--  abAddressBookNameDialog.js
new revision: 1.4; previous revision: 1.3
done
Checking in mail/components/addrbook/content/abAddressBookNameDialog.xul;
/cvsroot/mozilla/mail/components/addrbook/content/abAddressBookNameDialog.xul,v
 <--  abAddressBookNameDialog.xul
new revision: 1.3; previous revision: 1.2
done
Attachment #182180 - Attachment description: Patch v5 → Patch v5 (Checked in)
(Assignee)

Comment 20

13 years ago
Patch checked in by Ian this is now fixed on TB & Suite.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Created attachment 183001 [details]
Screenshot, showing OK button disabled when empty string exists.
Verified FIXED using build 2005-05-08-05 on Windows XP Seamonkey trunk.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.