290024 - Address Book vanishes if it is renamed as blank

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

Description

[malika.jaiswal]

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

[ABHIJEET BHATTACHARYA]

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

Comment 2

[ABHIJEET BHATTACHARYA]

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

[ABHIJEET BHATTACHARYA]

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

Comment 4

[Mark Banner (:standard8)]

Confirming on linux build 20050412, and taking.

Comment 5

[Mark Banner (:standard8)]

Attachment: 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.

Comment 6

[Scott MacGregor]

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."

Comment 7

[Mark Banner (:standard8)]

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

Revising text.

Comment 8

[Mark Banner (:standard8)]

Attachment: Patch v2

I agree, scott's suggestion for the message error text is better than my
original one.

Comment 9

[[e-mail]]

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 == "".

Comment 10

[Mark Banner (:standard8)]

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?

Comment 11

[Mark Banner (:standard8)]

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.

Comment 12

[Mark Banner (:standard8)]

Attachment: Patch v3

Ok revised patch so that now we only enable the button when it's valid. TB &
Suite versions included.

Comment 13

[[e-mail]]

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?

Comment 14

[Mark Banner (:standard8)]

Attachment: Patch v4

This patch fixes the new address book case and adds stripping of spaces from
the start & end of address book names.

Comment 15

[[e-mail]]

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.

Comment 16

[Mark Banner (:standard8)]

Attachment: Patch v5 (Checked in)

new patch to fix tab nit, carrying forward neil's r, requesting sr.

Comment 17

[Mark Banner (:standard8)]

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.

Comment 18

[chris hofmann]

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

a=chofmann

Comment 19

[Ian Neal]

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

Comment 20

[Mark Banner (:standard8)]

Patch checked in by Ian this is now fixed on TB & Suite.

Comment 21

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

Attachment: Screenshot, showing OK button disabled when empty string exists.

Comment 22

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

Verified FIXED using build 2005-05-08-05 on Windows XP Seamonkey trunk.