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

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

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.

Comment 1

[[e-mail]]

(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

[Philip Chee]

(In reply to [e-mail] 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).

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

[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

Comment 4

[[e-mail]]

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?]

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 8347289 [details] [diff] [review]
patch

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


(In reply to [e-mail] 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.

Comment 6

[Philip Chee]

Comment on attachment 8347289 [details] [diff] [review]
patch

a=me for aurora and beta.

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 8347289 [details] [diff] [review]
patch

https://hg.mozilla.org/releases/comm-aurora/rev/9753a7ca30b4
https://hg.mozilla.org/releases/comm-beta/rev/8adf90b209c1