The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 45.0

Status

Thunderbird
Filters
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Jorg K (GMT+1), Assigned: arai)

Tracking

({regression})

Trunk
Thunderbird 45.0
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
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?
(Reporter)

Comment 1

a year ago
Any idea why that broke?
Flags: needinfo?(rkent)
Keywords: regression
(Reporter)

Comment 2

a year ago
Created attachment 8697559 [details] [diff] [review]
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.
(Reporter)

Comment 3

a year ago
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.
Flags: needinfo?(arai.unmht)
Created attachment 8697587 [details] [diff] [review]
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?
Assignee: nobody → arai.unmht
Flags: needinfo?(arai.unmht) → needinfo?(mozilla)
Attachment #8697587 - Flags: review?(rkent)
(Reporter)

Comment 5

a year ago
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.
Flags: needinfo?(rkent)
Flags: needinfo?(mozilla)
Attachment #8697587 - Flags: feedback+
(Reporter)

Updated

a year ago
Status: NEW → ASSIGNED
Comment on attachment 8697587 [details] [diff] [review]
Fix broken mail activity after for-each replacement.

Looks good to me.
Attachment #8697587 - Flags: review?(rkent) → review+
https://hg.mozilla.org/comm-central/rev/b65465b757bbc3c5632c145893f640d1e0718de9
Bug 1231887 - Fix broken mail activity after for-each replacement. r=rkent
(Reporter)

Updated

a year ago
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
(fixing dependency)
Blocks: 824104

Comment 9

a year ago
Wouldn't for..of work here?
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.
You need to log in before you can comment on or make changes to this bug.