1630184 - Enforce mailing list name restrictions for all code paths

Status: RESOLVED FIXED

(MailNews Core :: Address Book, task, P2)

Description

[Geoff Lankow (:darktrojan)]

Right now we enforce that mailing lists be correctly named in the UI, but there are other ways to create a mailing list. We should prevent invalid list names no matter how the list is created.

Comment 1

[Magnus Melin [:mkmelin]]

https://searchfox.org/comm-central/search?q=dirName+%3D&case=true&regexp=false&path=mail

Comment 2

[Abdallah Afify]

Hello,

Can I work on this bug or it is not suitable for first timers ?

Comment 3

[Magnus Melin [:mkmelin]]

Sure, get started on a patch and I'll assign the bug to you then.

Comment 4

[Abdallah Afify]

Attachment: RestrictMailingListsNames.diff

Hello,

I uploaded a diff file to show where I think it should be implemented. All I need is to test. The only test I know is to go through the normal way using the address book window. I do not know any other ways to add or update lists. Would give me some pointers on how further I can test.

Thank you

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9143486 [details] [diff] [review]
RestrictMailingListsNames.diff

Review of attachment 9143486 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/addrbook/jsaddrbook/AddrBookDirectory.jsm
@@ +898,5 @@
>        throw Cr.NS_ERROR_UNEXPECTED;
>      }
>  
> +    // Check if the new name is empty.
> +    if (list.dirName.length == 0) {

we usually check falsy, so
if (!list.dirName) {

::: mailnews/addrbook/jsaddrbook/AddrBookMailingList.jsm
@@ +173,5 @@
>          );
>        },
>        editMailListToDatabase(listCard) {
> +        // Check if the new name is empty.
> +        if (self._name.length == 0) {

if (!self._name) {

::: mailnews/addrbook/jsaddrbook/AddrBookManager.jsm
@@ +225,4 @@
>        }
>      }
>  
> +    if (!dirName || 0 == dirName.length) {

not needed. if dirName.length is 0, dirName is already falsy

Comment 6

[Magnus Melin [:mkmelin]]

Except for the UI the code path I think Geoff was after is creation through the MailExtension APIs. Please adapt the test_ext_addressBook.js test to test this case.

./mach test comm/mail/components/extensions/test/xpcshell/test_ext_addressBook.js

Comment 7

[Abdallah Afify]

Thank you for the feedback, I will correct the comments and update the tests

Comment 8

[Magnus Melin [:mkmelin]]

Any update?

Comment 9

[Abdallah Afify]

I was just waiting for the other bug of lists renaming to be closed first to make a commit for this one to avoid amending commits because I cannot do it easily on hg

Comment 10

[Abdallah Afify]

Attachment: Bug 1630184 - Add name validation for address books and mailing lists. r=mkmelin

Add name validation for both address books and mailing lists and add tests for the mail extensions to ensure it works
for both normal creation in the TB Address book window and for the mail extension.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6860fc805603
Add name validation for address books and mailing lists. r=darktrojan