1585512 - Remove deprecated nsIMsgHeaderParser::parseHeadersWithArray

Status: RESOLVED FIXED

(MailNews Core :: General, task)

Description

[Ben Campbell]

parseHeadersWithArray() in nsIMsgHeaderParser is marked deprecated and should be removed.
parseEncodedHeader() is the appropriate replacement.

Occurences:
https://searchfox.org/comm-central/search?q=parseHeadersWithArray&path=

Comment 1

[Ben Campbell]

Attachment: 1585512-remove-parseHeadersWithArray-1.patch

xpcshell unit tests seems good locally with this patch applied.

I've got a try build running to check out the mozmill tests (the patch has a few more modifications to user-facing code than I'm totally comfortable with :- ):

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bc3ec9b93184838e727821e8e4da92af2e61bded

Comment 2

[Ben Campbell]

Hmm. comm/calendar/test/unit/test_providers.js looks borked... (I didn't pick it up locally, had calendar disabled).
Hold off on this patch - I'll investigate in the morning.

Comment 3

[Magnus Melin [:mkmelin]]

The test_provider.js failure was due to the bug 1583330 landing. Now fixed.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9097841 [details] [diff] [review]
1585512-remove-parseHeadersWithArray-1.patch

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

::: mail/base/content/mailWindowOverlay.js
@@ +1637,3 @@
>      uniqueAddresses,
> +    undefined,
> +    false

Looks like giving a charset should also be optional, and the only param you should pass in is uniqueAddresses

(here and the other places)

@@ +3473,1 @@
>    if (adrCount > 0) {

No need for the temp I think

::: mail/components/compose/content/MsgComposeCommands.js
@@ +4209,2 @@
>        return;
>      }

doesn't look like this would ever happen. not in the old code either

Comment 5

[Ben Campbell]

Attachment: 1585512-remove-parseHeadersWithArray-2.patch

Tweaked patch, with suggestions applied.
I left the adrCount temporary in mail/base/content/mailWindowOverlay.js as it's used a couple of times further down the function (doesn't show up in the diff).

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9098414 [details] [diff] [review]
1585512-remove-parseHeadersWithArray-2.patch

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

::: mailnews/db/gloda/modules/utils.js
@@ +53,2 @@
>      return {
> +      names: addresses.map(a => (a.name ? a.name : null)),

Can I change that to: a => a.name || null
?

Comment 7

[Ben Campbell]

(In reply to Jorg K (GMT+2) from comment #6)

Comment on attachment 9098414 [details] [diff] [review]
1585512-remove-parseHeadersWithArray-2.patch

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

::: mailnews/db/gloda/modules/utils.js
@@ +53,2 @@

 return {

•  names: addresses.map(a => (a.name ? a.name : null)),
  

Can I change that to: a => a.name || null
?

Yes, that'd probably be more idiomatic.
(although I had to run some quick tests in a javascript console before answering, to reassure my C++ instincts ;- )

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b22962d53db9
Remove deprecated nsIMsgHeaderParser::parseHeadersWithArray. r=mkmelin