1719996 - Remove nsISeekableStream assumption from message writing code.

Status: RESOLVED FIXED

(MailNews Core :: Backend, task)

Description

[Ben Campbell]

As a holdover from the mbox-only days, a lot of the code which writes out messages assumes that it is always writing to a seekable stream.
Usually this is used to force the file position to the end of the file for appending.
But there are a few places which use it to determine the message offset in the file.
Both of these should be handled by other means.

Comment 1

[Ben Campbell]

Attachment: Bug 1719996 - Some minor clarifications for nsParseMailMessageState. r?mkmelin

No functional changes, just adds a few comments and shuffles around some of
the member declarations to better show which memory buffers the values point
into.

Comment 2

[Ben Campbell]

Attachment: Bug 1719996 - Remove nsPop3Sink assumption that message output stream is seekable. r?mkmelin

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/189a9eedb45f
Some minor clarifications for nsParseMailMessageState. r=mkmelin
https://hg.mozilla.org/comm-central/rev/bddb0660ea90
Remove nsPop3Sink assumption that message output stream is seekable. r=mkmelin

Comment 4

[Ben Campbell]

Attachment: Bug 1719996 - Remove nsImapMailFolder assumption that message output stream is seekable. r?mkmelin

This removes an nsIMsgHdr.setMessageOffset() call. That call was redundant - it's handled
already inside the mbox mailstore.

Comment 5

[Ben Campbell]

Attachment: Bug 1719996 - Remove nsImapService assumption that message output stream is seekable. r?mkmelin

Depends on D119940

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8c94e187005d
Remove nsImapMailFolder assumption that message output stream is seekable. r=mkmelin
https://hg.mozilla.org/comm-central/rev/f2e31554f987
Remove nsImapService assumption that message output stream is seekable. r=mkmelin

Comment 7

[Worcester12345]

Was this "fixed"?

Comment 8

[Ben Campbell]

(In reply to Worcester12345 from comment #7)

Was this "fixed"?

Not yet. There are still a few places that assume we're always dealing with mbox files.

Comment 9

[Ben Campbell]

Attachment: Bug 1719996 - Remove some output stream seeks in nsMsgLocalMailFolder. r=mkmelin

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/32e946603b43
Remove some output stream seeks in nsMsgLocalMailFolder. r=mkmelin

Comment 11

[Ben Campbell]

Reopening - looks like the patch in Comment 9 caused an issue (Bug 1729778), so I'm backing it out for now.

My take on it is that the old code was relying on the nsISeekable QI to fail, in order to detect if the copy destination was a non-offline IMAP folder.
I'll have another go at it.

Comment 12

[Ben Campbell]

Attachment: Bug 1719996 - Remove unused txnMgr var in nsMsgCopyService::CopyFileMessage(). r=mkmelin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4f110d7df0f8
Remove unused txnMgr var in nsMsgCopyService::CopyFileMessage(). r=mkmelin

Comment 14

[Ben Campbell]

Oops - https://phabricator.services.mozilla.com/D124428 slipped through the net.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ffc062c99371
Remove some output stream seeks in nsMsgLocalMailFolder. r=mkmelin