489325 - Sanitize Send feedback and progress handling

Status: NEW

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

Attachment: WIP

The current implementation of our status feedback and message progress handling is plain complicated.

We have nsIMsgStatusFeedback and nsIMsgProgress. nsIMsgProgress inherits from nsIWebProgressListener.

nsMsgStatusFeedback wants to know about status updates and progress, so it implements nsIMsgStatusFeedback and nsIMsgProgress.

nsMsgProgress also wants to know about status updates and progress, so it also implements both nsIMsgStatusFeedback and nsIMsgProgress.

So both these classes want similar things, but have to implement extra functions to do so. We then also get a nice mix of interfaces implemented, and when we start passing these classes round the system we get of different interface types being stored and used.

This all just leads to confusion when we could just provide one interface (e.g. nsIMsgFeedback) to link together the status and progress information, and then have nsIMsgStatusFeedback and nsIMsgProgress to inherit from it and do their specific functions.

Although I don't like doing big changes like this at this stage, I'm hoping to get this in for b3 for bug 440794 - I'll then be able to make the progress reporting of send in background more sane and reflect what's actually going on.

Attaching WIP patch, its only the C++ changes so far so don't expect it to work, but it'll give an idea of the interface changes that I'm proposing.

Comment 1

[Mark Banner (:standard8)]

Ok, I'm hijacking my own bug into something slightly different. This is still going to do the work I need for send in background, but will do it in a slightly less wide-ranging way.

Comment 2

[Mark Banner (:standard8)]

Attachment: WIP v2

This is a very rough patch that I'm working on, it:

- Makes sending messages work with just one listener being supplied.
- Enables send in background to pass one listener and get status and progress back via that one listener.
- Probably breaks other progress/status feedback!

The basic idea with this patch is to make nsISendListener implement nsIMsgStatusFeedback so that it can be passed to nsMsgMailNewsUrl. Along the way, nsIMsgStatusFeedback currently picks up some new interface methods.

My next step will be to extract parts of nsIMsgStatusFeedback into a common interface that nsIMsgStatusFeedback and nsISendListener can interface to, and hence, nsMsgMailNewsUrl and the internals of mailnews can deal with that.

What I will then be able to do is to fire send errors back to the UI via nsISendListener (rather than pushing them straight to the UI in prompts), and hence resolve some of my issues of not being able to handle errors in the way I want to.

Comment 3

[Mark Banner (:standard8)]

Due to the amount of work left for completing send in background, we've decided that it isn't going to make Thunderbird 3. Therefore this bug isn't needed for TB 3 to complete that work, so taking off the blocking list.

Comment 4

[Wayne Mery (:wsmwk)]

Is this still a concern?

Comment 5

[Mark Banner (:standard8)]

This would need to be reviewed by an active developer.