Last Comment Bug 358247 - Port Bug 358121 and Bug 246414 to SeaMonkey (Phishing detector fix; Background and text colors lost when using a template)
: Port Bug 358121 and Bug 246414 to SeaMonkey (Phishing detector fix; Backgroun...
Status: RESOLVED FIXED
: fixed-seamonkey1.1
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Frank Wein [:mcsmurf]
:
Mentors:
Depends on:
Blocks: 234354
  Show dependency treegraph
 
Reported: 2006-10-26 12:45 PDT by Frank Wein [:mcsmurf]
Modified: 2006-11-19 20:41 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (1.70 KB, patch)
2006-10-26 12:56 PDT, Frank Wein [:mcsmurf]
neil: review+
neil: superreview-
Details | Diff | Splinter Review
Patch 2 (3.69 KB, patch)
2006-11-01 09:16 PST, Frank Wein [:mcsmurf]
neil: review+
neil: superreview+
kairo: approval‑seamonkey1.1+
Details | Diff | Splinter Review
Patch to check in (3.71 KB, patch)
2006-11-01 09:35 PST, Frank Wein [:mcsmurf]
no flags Details | Diff | Splinter Review

Description Frank Wein [:mcsmurf] 2006-10-26 12:45:50 PDT
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] 2006-10-26 12:56:48 PDT
Created attachment 243694 [details] [diff] [review]
Patch
Comment 2 neil@parkwaycc.co.uk 2006-10-26 13:24:27 PDT
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] 2006-11-01 09:16:07 PST
Created attachment 244317 [details] [diff] [review]
Patch 2
Comment 4 Frank Wein [:mcsmurf] 2006-11-01 09:16:50 PST
This patch now fixes Bug 234354, too.
Comment 5 neil@parkwaycc.co.uk 2006-11-01 09:25:50 PST
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] 2006-11-01 09:35:41 PST
Created attachment 244318 [details] [diff] [review]
Patch to check in
Comment 7 Frank Wein [:mcsmurf] 2006-11-01 11:11:17 PST
Patch checked in by Bernd.
Comment 8 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-11-04 09:57:59 PST
Comment on attachment 244317 [details] [diff] [review]
Patch 2

first-a=me for 1.1
Comment 9 Robert Kaiser 2006-11-13 07:15:02 PST
Comment on attachment 244317 [details] [diff] [review]
Patch 2

second-a=me for 1.1
Comment 10 Andrew Schultz 2006-11-19 20:41:14 PST
Comment on attachment 244318 [details] [diff] [review]
Patch to check in

landed on the branch

Note You need to log in before you can comment on or make changes to this bug.