Closed Bug 969903 Opened 7 years ago Closed 7 years ago

Improve thread pane display name handling


(Thunderbird :: Folder and Message Lists, defect)

Not set


(Not tracked)

Thunderbird 30.0


(Reporter: squib, Assigned: squib)




(1 file, 2 obsolete files)

Attached patch display-name.patch (obsolete) — Splinter Review
This patch does three things to the From/Recipients columns in the thread pane:

1) Properly strips angle brackets when the from name is of the form <>
2) Add a space between multiple names in the recipients column
3) Adds much more thorough Mozmill tests for this code

I didn't add tests for the address book lookup for display names, but I could do that too.
Attachment #8372869 - Flags: review?(Pidgeot18)
Comment on attachment 8372869 [details] [diff] [review]

Review of attachment 8372869 [details] [diff] [review]:

::: mail/test/mozmill/folder-display/test-display-name.js
@@ +32,5 @@
> +  },
> +  { name: "from_email_in_angle_brackets",
> +    headers: { From: "<whudson@uscmc.invalid>" },
> +    expected: { column: "from", value: "whudson@uscmc.invalid" },
> +  },

I'd also include tests for empty From headers and ones with multiple addresses. Actually, come to think of it, you probably ought to have one with a From and a Sender and one with no From but a Sender as well.

@@ +58,5 @@
> +  { name: "to_display_name_multiple",
> +    headers: { To: "Carter Burke <cburke@wyutani.invalid>, " +
> +                   "Dwayne Hicks <dhicks@uscmc.invalid>" },
> +    expected: { column: "recipients", value: "Carter Burke, Dwayne Hicks" },
> +  },

A part of me wants there to be tests with group addresses in here. But the other part reminds me that our handling of groups right now is complete and utter crap.
Attached patch Add more tests (obsolete) — Splinter Review
I've added some more tests, testing address book contacts, mixing From/Sender, and mixing To/Cc. This should cover pretty much everything except groups.
Attachment #8372869 - Attachment is obsolete: true
Attachment #8372869 - Flags: review?(Pidgeot18)
Attachment #8373038 - Flags: review?(Pidgeot18)
Even more tests! Now there are tests for missing From/To headers, empty From/To headers, and invalid From/To headers.
Attachment #8373038 - Attachment is obsolete: true
Attachment #8373038 - Flags: review?(Pidgeot18)
Attachment #8373039 - Flags: review?(Pidgeot18)
Comment on attachment 8373039 [details] [diff] [review]
Add more tests, part 2

Review of attachment 8373039 [details] [diff] [review]:

This is the kind of tests I like seeing. Especially helpful in knowing that I'm not going to regress code in one of my major rewrites.
Attachment #8373039 - Flags: review?(Pidgeot18) → review+
Duplicate of this bug: 678719
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 30.0
Flags: in-testsuite- → in-testsuite+
You need to log in before you can comment on or make changes to this bug.