Closed Bug 1630184 Opened 5 months ago Closed 5 months ago

Enforce mailing list name restrictions for all code paths

Categories

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

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 78.0

People

(Reporter: darktrojan, Assigned: abdallah.khaled.ali.93)

Details

Attachments

(2 files)

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.

Hello,

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

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

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

Flags: needinfo?(mkmelin+mozilla)
Attachment #9143486 - Flags: feedback?(mkmelin+mozilla)
Assignee: nobody → abdallah.khaled.ali.93
Status: NEW → ASSIGNED
Flags: needinfo?(mkmelin+mozilla)
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
Attachment #9143486 - Flags: feedback?(mkmelin+mozilla) → feedback+

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

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

Any update?

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

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.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/6860fc805603
Add name validation for address books and mailing lists. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 78.0
You need to log in before you can comment on or make changes to this bug.