1231887 - Activity summary for incoming POP messages filtered by "before junk" rules doesn't work

Status: RESOLVED FIXED

(Thunderbird :: Filters, defect)

Description

[Jorg K (CEST = GMT+2)]

Activity summary for incoming pop messages filtered by "before junk" rules doesn't work.

In bug 1211261 I implemented the "UnincorporatedMessageMoved" event, so that POP messages that got moved by a "before junk" rule would get shown in the activity manager.

This worked nicely. Today I noticed that on Daily of 2015-12-10 the summarising doesn't work any more. I believe it worked a couple of days ago, I will get a regression window.

The summarising is done here:
https://hg.mozilla.org/comm-central/annotate/63a4dcada36a2181c1ba6457f58fa0f0476f4f06/mail/components/activity/modules/moveCopy.js#l316

Did something change with the message ID?

I tried "after junk" rules, which are summarised here:
https://hg.mozilla.org/comm-central/annotate/63a4dcada36a2181c1ba6457f58fa0f0476f4f06/mail/components/activity/modules/moveCopy.js#l126
and they still get summaries properly.

How did that break?

Comment 1

[Jorg K (CEST = GMT+2)]

Any idea why that broke?

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: Patch showing the debug I added

Something has gone bust with activities.

Debug statements are:
dump("+++++++++++++++ |" + activities[activities.length-1].id + "|\n");
dump("+++++++++++++++ |" + this.lastMessage.id + "|\n");

The debug shows:
Incorporate message begin:
uidl string: UID139-1444039152
GetDiskSpaceAvailable returned: 18012839936 bytes
+++++++++++++++ |0|
+++++++++++++++ |undefined|
Incorporate message complete.
Incorporate message begin:
uidl string: UID140-1444039152
GetDiskSpaceAvailable returned: 18012831744 bytes
+++++++++++++++ |0|
+++++++++++++++ |4|
Incorporate message complete.

So two messages were received and incorporated.

Sadly, the last retrieved activity always has id=0 so it never matches.

I also noticed that clicking "Clear List" in the Activity Manager doesn't work and returns this in the console:
JavaScript error: chrome://messenger/content/activity.js, line 263: TypeError: item.detachFromActivity is not a function

The list isn't cleared. On a non-debug Daily the list is cleared. So I don't understand this either.

Comment 3

[Jorg K (CEST = GMT+2)]

OK, regression range:

Worked on 2015-12-04
Busted on 2015-12-05.
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2015-12-04&enddate=2015-12-05+12%3A00

Most likely bug 824104.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Fix broken mail activity after for-each replacement.

Sorry for poor quality patch :P
I think this will fix something.

Jorg, can you test this?  or is there any way to test it locally?

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8697587 [details] [diff] [review]
Fix broken mail activity after for-each replacement.

Thanks, that fixes both problems:
- Activities combine/summarise again as expected.
- Clear List works.

Please review/land this quickly. I'm using Daily and I need this function.

Comment 6

[Kent James (:rkent)]

Comment on attachment 8697587 [details] [diff] [review]
Fix broken mail activity after for-each replacement.

Looks good to me.

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/comm-central/rev/b65465b757bbc3c5632c145893f640d1e0718de9
Bug 1231887 - Fix broken mail activity after for-each replacement. r=rkent

Comment 8

[Wayne Mery (:wsmwk)]

(fixing dependency)

Comment 9

[:aceman]

Wouldn't for..of work here?

Comment 10

[Tooru Fujisawa [:arai]]

for-of requires the object to be iterable.
but _activities property is just an object.
we could replace _activities and related implementation with Map or something to use for-of there.