Closed Bug 1585512 Opened 5 years ago Closed 5 years ago

Remove deprecated nsIMsgHeaderParser::parseHeadersWithArray

Categories

(MailNews Core :: General, task)

Product:
MailNews Core
Component:
General
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 71.0

People

(Reporter: benc, Assigned: benc)

References

Details

Attachments

(1 file, 1 obsolete file)

1585512-remove-parseHeadersWithArray-2.patch
(deleted), patch
benc
: review+
Details | Diff | Splinter Review

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
Attached patch 1585512-remove-parseHeadersWithArray-1.patch (obsolete) (deleted) — — Splinter Review

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

Attachment

General

Creator:
Created:
Updated:
Size: