358247 - Port Bug 358121 and Bug 246414 to SeaMonkey (Phishing detector fix; Background and text colors lost when using a template)

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Frank Wein [:mcsmurf]]

Bug 358121 needs to be ported to SeaMonkey, the JS errors also occour in SeaMonkey:
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgMailNewsUrl.folder]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/mailWindowOverlay.js :: OnMsgLoaded :: line 2324"  data: no]
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 2324
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgMailNewsUrl.folder]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/phishingDetector.js :: isMsgEmailScam :: line 60"  data: no]
Source File: chrome://messenger/content/phishingDetector.js
Line: 60

Comment 1

[Frank Wein [:mcsmurf]]

Attachment: Patch

Comment 2

[[e-mail]]

Comment on attachment 243694 [details] [diff] [review]
Patch

>   // Ignore nntp and RSS messages
>-  var folder = aUrl.folder;
>-  if (folder.server.type == 'nntp' || folder.server.type == 'rss')
>+  // nsIMsgMailNewsUrl.folder can throw an error, especially if we are opening
>+  // a .eml message.
>+  var folder;
>+  try {
>+    folder = aUrl.folder;
>+  } catch (ex) {}
>+
>+  if (folder && (folder.server.type == 'nntp' || folder.server.type == 'rss'))
>     return isEmailScam;
You should test the variable inside the try block, since the folder isn't used anywhere else. Additionally consider using caching folder.server.type directly, or switch(!)ing to a switch statement.

>+    // nsIMsgMailNewsUrl.folder throws an error when opening .eml files.
>+    var folder;
>+    try {
>+      folder = aUrl.folder;
>+    } catch (ex) {}
Worth returning inside the catch block?

Comment 3

[Frank Wein [:mcsmurf]]

Attachment: Patch 2

Comment 4

[Frank Wein [:mcsmurf]]

This patch now fixes Bug 234354, too.

Comment 5

[[e-mail]]

Comment on attachment 244317 [details] [diff] [review]
Patch 2

>+    if (!bodyElement.getAttribute("text"))
>+    if (!bodyElement.getAttribute("bgcolor"))
Nit: should be hasAttribute

>+    } catch (ex) {return;}
Nit: return; on its own line please

r+sr=me with these fixed.

Comment 6

[Frank Wein [:mcsmurf]]

Attachment: Patch to check in

Comment 7

[Frank Wein [:mcsmurf]]

Patch checked in by Bernd.

Comment 8

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 244317 [details] [diff] [review]
Patch 2

first-a=me for 1.1

Comment 9

[Robert Kaiser]

Comment on attachment 244317 [details] [diff] [review]
Patch 2

second-a=me for 1.1

Comment 10

[Andrew Schultz]

Comment on attachment 244318 [details] [diff] [review]
Patch to check in

landed on the branch