Closed Bug 1697906 Opened 4 years ago Closed 3 years ago

remove attachments from compose.beginNew(msg.id)

Categories

(Thunderbird :: Add-Ons: Extensions API, defect)

defect

Tracking

(thunderbird_esr78- affected, thunderbird88- wontfix)

RESOLVED WONTFIX
Tracking Status
thunderbird_esr78 - affected
thunderbird88 - wontfix

People

(Reporter: buecher, Unassigned)

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.72 Safari/537.36

Steps to reproduce:

if msg.id is supplied, the content of the original mail is copied over.
Using composeDetails, this can be modified. body="", for example, removes the body of the original email from the new mail.

Actual results:

I did not find a method to remove the attachments. That should be possible as well.

There is a workaound: compoase.listMessages followed by removeMessages. Drawback: can only be done after compose window is rendered. So it first comes up with attachments visible, which then (visibly) disappear. That looks unprofessional.

Expected results:

if attachments == null in composedetails: remove the original attachments (or something similiar)

Status: UNCONFIRMED → NEW
Ever confirmed: true

The issue here is, that the elements in attachment are supposed to be added. What is the expected result for an email with 2 attachements, which is opened with BeginNew() and one entry given in details.attachments?

A) Remove the 2 original attachments and add the new one
B) Just add the new one

Currently, it is B), that is why an empty attachment list is not doing anything. Having a mix of both feels strange (an empty attachments array will clear all original attachments, a single entry will add it but not touch the other ones).

Hm.

Ideas:

  1. have an extra flag whether to keep/clear old attachments
  2. somehow hold rendering until other actions are done (listattachment, removeattachment works fine. if the end user would not see that, all were fine)

How badly is this needed for 91? Is it a blocker?

(In reply to Wayne Mery (:wsmwk) from comment #3)

How badly is this needed for 91? Is it a blocker?

Not a blocker, it works, but it could be improved.

Flags: needinfo?(john)
Flags: needinfo?(john)

For ForwardInline, Redirect and EditAsNew, the API has to use MailServices.compose.OpenComposeWindow, which cannot manipulate its attachments before the window is fully loaded.

For all the Reply cases (which never include attachments) and for the ForwardAsAttachment case, the API is using MailServices.compose.OpenComposeWindowWithParams. That does accept a nsIMsgComposeParams parameter which has a nsIMsgCompFields property. But even that is not able to remove existing attachments. It does have a removeAttachments() member function [1], but that can only remove attachments which have been added previously.

That is why the attachments property of the ComposeDetails type can only be used to add attachments.

There is a small chance, that for the ForwardAsAttachment case we could change core code to also strip attachments (like the Reply cases do it already and always), but since ForwardInline, Redirect and EditAsNew have to use the other "simple" codepath which does not support attachment manipulation, the behaviour of the API would be inconsistent. Or the API would have to remove the attachments in these cases after the window has been created. But that would be the exact same thing as using the WebExt API to remove the attachments after the window has been created. So there would be almost no gain.

I therefore close this as a wontfix.

[1] https://searchfox.org/comm-central/rev/ce30946f9f46d13d98655dddcdd5d55ac48fffac/mailnews/compose/public/nsIMsgCompFields.idl#68-70

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(john)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.