Port |Bug 912183 - context menu shows all possible options when only feed accounts are set up|

RESOLVED FIXED in seamonkey2.26

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.26

SeaMonkey Tracking Flags

(seamonkey2.23 affected, seamonkey2.24 fixed, seamonkey2.25 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Bug 640147 introduced a new method to folderDisplay.js, canArchiveSelectedMessages(). Unfortunatly this breaks the context menu if called on a feed (RSS) message:

Error: TypeError: GetIdentityForHeader(...) is null
Source File: chrome://messenger/content/folderDisplay.js
Line: 89

The solution is to port TB bug 912183:

     return this.selectedMessages.every(function(msg) {
-      return getIdentityForHeader(msg).archiveEnabled;
+      let identity = getIdentityForHeader(msg);
+      return Boolean(identity && identity.archiveEnabled);
     });

Note: Our function is called GetIdentityForHeader, not getIdentityForHeader.

Will provide a patch later today.
(Assignee)

Updated

5 years ago
Blocks: TB2SM
(In reply to Jens Hatlak)
>      return this.selectedMessages.every(function(msg) {
> -      return getIdentityForHeader(msg).archiveEnabled;
> +      let identity = getIdentityForHeader(msg);
> +      return Boolean(identity && identity.archiveEnabled);
>      });
[Why bother with the Boolean cast in a boolean evaluation context?]

Comment 2

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #1)
> (In reply to Jens Hatlak)
>> +      return Boolean(identity && identity.archiveEnabled);

> [Why bother with the Boolean cast in a boolean evaluation context?]
I notice that Magnus likes to use this idiom (The Thunerbird patch was written by Magus).
(Assignee)

Comment 3

5 years ago
Posted patch patchSplinter Review
[Approval Request Comment]
Regression caused by (bug #): bug 640147
User impact if declined: MailNews message context menu sometimes broken
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): low
String changes made by this patch: none
Attachment #8347289 - Flags: review?(neil)
Attachment #8347289 - Flags: approval-comm-beta?
Attachment #8347289 - Flags: approval-comm-aurora?
Comment on attachment 8347289 [details] [diff] [review]
patch

Note: not actually tested, but looks sane.

[As a side note, this function must be incredibly inefficient if you have a large number of messages selected, no?]
Attachment #8347289 - Flags: review?(neil) → review+
(Assignee)

Comment 5

5 years ago
Comment on attachment 8347289 [details] [diff] [review]
patch

https://hg.mozilla.org/comm-central/rev/2d3862e59d33


(In reply to neil@parkwaycc.co.uk from comment #4)
> Note: not actually tested, but looks sane.

Don't you worry, I tested.

> [As a side note, this function must be incredibly inefficient if you have a
> large number of messages selected, no?]

Probably. I guess it could/should be optimized quite a bit for when only a certain account is affected or the like, but then there's saved searches, which can affect multiple accounts of different types, and we must not regress those. So better have an own bug for such complicated matters.
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.26

Updated

5 years ago
No longer blocks: TB2SM

Comment 6

5 years ago
Comment on attachment 8347289 [details] [diff] [review]
patch

a=me for aurora and beta.
Attachment #8347289 - Flags: approval-comm-beta?
Attachment #8347289 - Flags: approval-comm-beta+
Attachment #8347289 - Flags: approval-comm-aurora?
Attachment #8347289 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.