1716051 - onNewMailReceived event fires not only for really new messages, but for moved/copied old messages in IMAP folders, too

Status: RESOLVED FIXED

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

Description

[Alex Ihrig [:Thunderbird_Mail_DE]]

In my experimental MailExtension the event onNewMailReceived fires, when I copy or move old messages from folder A to IMAP folder B.

Copying to a local folder and to a POP account folder seems to be okay and doesn't fire the event.

Comment 1

[Alex Ihrig [:Thunderbird_Mail_DE]]

There was a comment with the idea, that my IMAP server marks the moved/copied messages as new. But this is not the case. The copied messages are not marked as completely new with the star icon (and not as unread) in destination folder in Thunderbird at all.

Comment 2

[Alex Ihrig [:Thunderbird_Mail_DE]]

After running a lot of test cycles for my addon development, I noticed, that Thunderbirds own function detachAllAttachments() triggers the event because of the saved stripped message copy in the IMAP folder. I report this, to show, that there is no other copy/move process necessary. I understand, that in this case the new created stripped message is a new message for the IMAP server, but this shouldn't fire the event.

Doing the same in local or POP folders doesn't trigger the event.

STR:

• Create a simple MX Addon and register the eventListener

browser.messages.onNewMailReceived.addListener(() => {
  console.debug("onNewMailReceived is fired");
});

• Open a message with attachments
• Go to the attachment bar at the bottom, click on Thunderbirds own "Detach" option and confirm to detach/delete the attachments.

Comment 3

[John Bieling (:TbSync)]

Could you verify, that this is fixed by bug 1627604? I added this:
https://hg.mozilla.org/comm-central/rev/58d779e4cc24#l1.209

Comment 4

[Alex Ihrig [:Thunderbird_Mail_DE]]

Seems not to be completely fixed.

1.) It is fixed, when detaching attachments. The event isn't fired any more.

2.) It is not fixed, when moving/copying a message from one imap folder to another imap folder (in the same account). The event is then still fired, although Thunderbird itself does (correctly) not mark the message as new.

Comment 5

[John Bieling (:TbSync)]

Attachment: messageEvents.zip

I am not able to reproduce this behavior. I can move messages around as much as I want, but I only get the onMoved event, not the onNewMailReceived event.

What I see (and did not know before), I get a onNewMailReceived event for the message copy being placed in the send folder, when a message is send. Will fix that.

It would help me a great deal if you could retry with the attached example add-on and look at the console output while moving messages around. I used Daily 91.0a1 (2021-07-06) (64-bit) for my tests.

Comment 6

[Alex Ihrig [:Thunderbird_Mail_DE]]

This is wired...

Today I am not able to reproduce the fired event by moving / copying messages. This seems to be correct.

But there is a related new bug due to your mechanism to suppress the false onNewMailReceived event:

• The event isn't fired now, when a new mail is tagged by a filter. In this case I get only the onModified event, which seems to suppres the onNewMailReceived event.
• When I combine 2 filters to first(!) tag the message and than move the message to an other folder, it works and fires the onNewMailReceived event as expected.
• When I combine 2 filter to first tag and than mark the message with a star, it doesn't fire the onNewMailReceived event. So the "Move" makes the difference in the szenario above.
• When I combine the same 2 filters in the other order (first move and than tag the message) the filter mechanism itself fails. The message is moved, but not tagged anymore. This seems to be a bug independently from your work on MX API events.

Comment 7

[John Bieling (:TbSync)]

Filters make this interesting. Regardless of the reporting being broken at the moment, would you expect the newMailNotification to be send for the initial inbox or for the final location after filters have been applied?

What about all the modifications (moves, updates, deletes?) done by the filters? Should all this be actually hidden and the message should "appear" for the WebExtension only after all filters have been applied? Currently, I can see those filter actions.

Comment 8

[John Bieling (:TbSync)]

After trying different things, I believe in the end we have to clone
https://searchfox.org/comm-central/source/mailnews/base/src/MailNotificationManager.jsm

for the API.

Comment 9

[John Bieling (:TbSync)]

Attachment: Bug 1716051 - Fix onNewMailReceived event. r=mkmelin

Comment 10

[John Bieling (:TbSync)]

@Alex: I do not know how long it takes till this lands in Daily, but to speed up the uplift process, could you give this try build a spin?
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/fhOHuSHJTISeRwt-pQYxbw/runs/0/artifacts/public/build/target.zip

I assume you use Windows? If not I can provide other builds as well.

This should now always give a notification when Thunderbird itself is giving one (I think the TB code actually has a bug as described in bug 1720947, which I fixed for the WebExt API)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/96980ab75808
Fix onNewMailReceived event. r=mkmelin

Comment 12

[Alex Ihrig [:Thunderbird_Mail_DE]]

Regression caused by latest checkin/patch:
"folder" object is always "null". See in German forums:
https://www.thunderbird-mail.de/forum/thread/87346-attachmentextractor-continued-version-4-0-f%C3%BCr-thunderbird-91/?postID=481102#post481102

Comment 13

[John Bieling (:TbSync)]

Attachment: Bug 1716051 - Fix incomplete test and return correct folder. r=darktrojan

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d4cd2f0797b8
Fix incomplete test and return correct folder. r=darktrojan

Comment 15

[Alex Ihrig [:Thunderbird_Mail_DE]]

(In reply to John Bieling (:TbSync) from comment #7)

Filters make this interesting. Regardless of the reporting being broken at the moment, would you expect the newMailNotification to be send for the initial inbox or for the final location after filters have been applied?

The question is, what will happen if the message is moved by a filter after firing the event. Will this be a problem for my and other addons because of the "disappaering" message in the initial folder, when it should be processed by the addon? So this would be an argument for the event firing after all filter actions are finally done.

What about all the modifications (moves, updates, deletes?) done by the filters? Should all this be actually hidden and the message should "appear" for the WebExtension only after all filters have been applied? Currently, I can see those filter actions.

These other events should be visible, when listening for them.

Comment 16

[John Bieling (:TbSync)]

Comment on attachment 9231653 [details]
Bug 1716051 - Fix onNewMailReceived event. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
onNewMailReceived event stays broken.

Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Low.

Comment 17

[John Bieling (:TbSync)]

Comment on attachment 9232467 [details]
Bug 1716051 - Fix incomplete test and return correct folder. r=darktrojan

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
onNewMailReceived event stays broken.

Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Low.

Comment 18

[Wayne Mery (:wsmwk)]

Comment on attachment 9232467 [details]
Bug 1716051 - Fix incomplete test and return correct folder. r=darktrojan

[Triage Comment]
Approved for beta

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 9231653 [details]
Bug 1716051 - Fix onNewMailReceived event. r=mkmelin

[Triage Comment]
Approved for beta

Comment 20

[Rob Lemley [:rjl]]

Thunderbird 91.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/bb63540145e8
https://hg.mozilla.org/releases/comm-beta/rev/bdd81ef5b134