Closed Bug 1585512 Opened 5 months ago Closed 5 months ago

Remove deprecated nsIMsgHeaderParser::parseHeadersWithArray

Categories

(MailNews Core :: General, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: benc, Assigned: benc)

References

Details

Attachments

(1 file, 1 obsolete file)

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=

Assignee: nobody → benc
Blocks: 1562158
Status: NEW → ASSIGNED

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

Attachment #9097841 - Flags: review?(mkmelin+mozilla)

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.

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

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
Attachment #9097841 - Flags: review?(mkmelin+mozilla) → review+

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

Attachment #9097841 - Attachment is obsolete: true
Attachment #9098414 - Flags: review+
Keywords: checkin-needed
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
?

(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 ;- )

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/b22962d53db9
Remove deprecated nsIMsgHeaderParser::parseHeadersWithArray. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0
No longer blocks: 1562158
Blocks: 1551704
You need to log in before you can comment on or make changes to this bug.