1876407 - Refactor nsParseMailMessageState et al

Status: ASSIGNED

(MailNews Core :: General, enhancement)

Description

[Ben Campbell]

At core, nsParseMailMessageState (and nsMsgMailboxParser and nsParseNewMailState) are responsible for taking a header block from a message and parsing out the fields required by the msgDB (i.e. everything you'll find in nsIMsgDBHdr).

But over time so much extra responsibility has accumulated here that the core message parsing has so many side effects that it's quite intractable.

In addition to parsing out message headers for the DB, they were handling writing to mbox files, invoking filters on new messages, showing alerts in the GUI, triggering notifications etc etc etc...

This stuff all needs some major refactoring.
Really, there should just be a nice simple routine which takes the header section of a message and parses out the fields wanted by the database. Everything else should be handled in more appropriate places.

see:
https://searchfox.org/comm-central/source/mailnews/local/src/nsParseMailbox.h
https://searchfox.org/comm-central/source/mailnews/local/src/nsParseMailbox.cpp

Bug 1873282 is a good indication of the kind of problem the overcomplication here causes.

Comment 1

[Ben Campbell]

Bug 1888790 throws up a few new thoughts:
New-message filtering is handled differently for pop3 and for imap. POP3 handles it as part of the header parsing (nsParseNewMailState). This is quite a pain.
To refactor the header parsing code we need to excise the filtering from nsParseNewMailState and get POP3 to handle it elsewhere.
Ideally, new-message filtering should be unified and protocol agnostic!
So we need to document how filtering is implemented currently for various protocols, then hopefully unify it in a single place.

Comment 2

[Hartmut Welpmann [:welpy-cw]]

Attachment: Bug 1876407 - Remove unused intermediate class nsMsgMailboxParser. r=benc

This could be a first step to reduce the complexity in nsParseMailbox.cpp.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e82f290f6edd
Remove unused intermediate class nsMsgMailboxParser. r=BenC

Comment 4

[Ben Campbell]

Attachment: Bug 1876407 - Add stand-alone header parser function, ParseMsgHeaders(). r=#thunderbird-back-end-reviewers

ParseMsgHeaders() not complete yet, but eventually will do all the
header-parsing that nsParseMailMessageState does now, but statelessly and
decoupled from the msgStore, filtering, GUI and higher-level policy decisions.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/eb9929397ca0
Add stand-alone header parser function, ParseMsgHeaders(). r=darktrojan

Comment 6

[Ben Campbell]

Attachment: Bug 1876407 - Extend capabilities of ParseMsgHeaders() to extract more data from headers. r=#thunderbird-back-end-reviewers

Adds a bunch more header parsing, divined from nsParseMailMessageState et al.
Not yet a 100% drop-in replacement - there are still a couple of things
missing, but should be a good enough basis for avoiding detached nsIMsgDBHdrs
for Panorama use.

Comment 7

[Ben Campbell]

Attachment: Bug 1876407 - Make most of nsParseMailMessageState members protected. r=#thunderbird-back-end-reviewers

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/823151492072
Extend capabilities of ParseMsgHeaders() to extract more data from headers. r=darktrojan

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/58bb78de4ad3
Make most of nsParseMailMessageState members protected. r=mkmelin

Comment 10

[Ben Campbell]

Attachment: Bug 1876407 - Remove unused nsIMsgParseMailMsgState.headers attr. r=#thunderbird-back-end-reviewers

Comment 11

[Ben Campbell]

Attachment: Bug 1876407 - Hide a couple of nsParseMailMessageState functions that don't need to be public. r=#thunderbird-back-end-reviewers

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/662e93e9a8e0
Remove unused nsIMsgParseMailMsgState.headers attr. r=mkmelin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ec77d6614a3b
Hide a couple of nsParseMailMessageState functions that don't need to be public. r=mkmelin