This is the equivalent of TB bug 279191 and includes the rewrite of "JunkBar".

This patch: * Rewrites "Junk Bar" * Adds phishing support to mailnews * Deals with precedence of msg bars better than TB - remembers if mail was possible phish or had remote content even when other msg bars take precedence

I think in contentAreaClick.js the line: if (!gPrefBranch.getBoolPref("mail.phishing.detection.enabled")) needs to be changed to: if (!pref.getBoolPref("mail.phishing.detection.enabled")) so that it works in both navigator and mailnews.

Changes since v0.1a: * Changed contentAreaClick.js so that it finds the correct bundle depending on if it is used from browser or mailnews * Changed contentAreaClick.js so that it can find pref value.

Changes since v0.1b: * Add missing mailnews.js file changes * This bug is just to do with mailnews so let's not phish detect for browsers (that can be another patch if it is needed) I need someone to test on xlinks to make sure my changes to contentAreaClick.js have not broken anything.

To get Remote Image message bar working there are three options I can see: 1) Drop extensions/permissions/nsMailnewsContentBlocker.cpp and use mailnews/base/src/nsMsgContentPolicy.cpp with the nsMsgCookiePolicy bits ifdef'd out so only TB uses them. 2) Drop extensions/permissions/nsMailnewsContentBlocker.cpp and use mailnews/base/src/nsMsgContentPolicy.cpp with the nsMsgCookiePolicy bits moved out into a separate file for TB to use. 3) Keep using extensions/permissions/nsMailnewsContentBlocker.cpp and copy all the extra bits in from mailnews/base/src/nsMsgContentPolicy.cpp (i.e. all of it except the nsMsgCookiePolicy bits). I'd say my least favourite option is the 3rd one, but it is possible TB folks will not let the first two options through. Any thoughts?

cc'ing mvl as may have a view because wrote the original nsMailnewsContentBlocker.cpp

Comment on attachment 185460 [details] [diff] [review] Revised patch v0.1c >Index: mailnews/base/resources/content/mailWindow.js >=================================================================== >-function loadStartPage() { [...] >- // first, clear out the charset setting. >- messenger.setDisplayCharset(""); What happened to this? >+ var startpage = pref.getComplexValue("mailnews.start_page.url", >+ Components.interfaces.nsIPrefLocalizedString).data; >+ if (startpage != "") If startpage is null (for whatever broken background reason), this will be true, so better use |if (startpage)|. >Index: mailnews/base/resources/content/mailWindowOverlay.js >=================================================================== > function HandleJunkStatusChanged(folder) [...] >+ if (GetNumSelectedMessages() == 1) >+ var msgHdr = messenger.messageServiceFromURI(loadedMessage).messageURIToMsgHdr(loadedMessage); >+ if (msgHdr) >+ gMessageNotificationBar.setJunkMsg(msgHdr); > else >- SetUpJunkBar(null); >+ gMessageNotificationBar.setJunkMsg(null); This means, that if msgHdr is invalid, we don't even call setJunkMsg... Better move the actual call to setJunkMsg out of the if. >+ setJunkMsg: function (aMsgHdr) Inferior nit: remove the space before (. >+ return (isJunk && isCurrentlyNotJunk) || (!isJunk && !isCurrentlyNotJunk); You mean |return isJunk == isCurrentlyNotJunk;| I suppose? ;-) But why return this at all? It isn't used anywhere and all the other gMessageNotificationBar.set*Msg functions don't return their state either, so just kick it... >+ setRemoteContentMsg: function (aMsgHdr) Inferior nit: remove the space before (. >+ // aUrl is the nsIURI for the message currently loaded in the message pane >+ setPhishingMsg: function(aUrl) >+ { >+ var msgURI = GetLoadedMessage(); >+ if (msgURI && !(/type=x-message-display/.test(msgURI))) >+ { >+ var msgHdr = messenger.messageServiceFromURI(msgURI).messageURIToMsgHdr(msgURI); >+ // if we've explicitly marked this message as not being an email scam, then don't >+ // bother checking it with the phishing detector. >+ if (msgHdr && msgHdr.getUint32Property("notAPhishMessage")) >+ return; >+ } >+ >+ this.updateMsgNotificationBar(kMsgNotificationPhishingBar, isMsgEmailScam(aUrl)); >+ }, Another too-early-for-early-return case: if the return is triggered, updateMsgNotificationBar isn't called at all... >+ barStatus: 0, Make that mBarStatus. >+ // private method used to set our message notification deck to the correct value... >+ updateMsgNotificationBar: function(aIndex, aSet) I'm quite uncomfortable with all those "1 << (something - 1)" stuff getting computed all over again with every call to updateMsgNotificationBar. How about adding another member to gMessageNotificationBar: // flag bit values for mBarStatus, indexed by kMsgNotificationXXX mBarFlagValues: [ 0, // kMsgNotificationNoStatus, 1, // 1 << (kMsgNotificationJunkBar - 1) 2, // 1 << (kMsgNotificationRemoteImages - 1) 4 // 1 << (kMsgNotificationPhishingBar - 1) ], Don't put this stuff into the global scope, as noone outside of gMessageNotificationBar should be seeing it. >+ var status = 0; You should set status to kMsgNotificationNoStatus to make it more visible what's going on. >+ if (aIndex != kMsgNotificationNoStatus) { Keep the overall file style and move the { onto its own line. >+ var chunk = 1 << (aIndex - 1); With mBarFlagValues above, this could just be var chunk = this.mBarFlagValues[aIndex]; >+ if (status & 1 << (kMsgNotificationJunkBar - 1)) and if (status & this.mBarFlagValues[kMsgNotificationJunkBar]) etc. >+ if (status == 0) if (!status) >+function LoadMsgWithRemoteContent() >+{ >+ // we want to get the msg hdr for the currently selected message >+ // change the "remoteContentBar" property on it >+ // then reload the message >+ >+ var msgURI = GetLoadedMessage(); >+ var msgHdr = null; >+ >+ if (msgURI && !(/type=x-message-display/.test(msgURI))) >+ { >+ msgHdr = messenger.messageServiceFromURI(msgURI).messageURIToMsgHdr(msgURI); Move the |var msgHdr| here (even though the JS parser doesn't care, it helps reading the code later). >+function MsgIsNotAScam() >+{ >+ // we want to get the msg hdr for the currently selected message >+ // change the "isPhishingMsg" property on it >+ // then reload the message >+ >+ var msgURI = GetLoadedMessage(); >+ var msgHdr = null; Same here. >Index: mailnews/base/resources/content/mailWindowOverlay.xul >=================================================================== >+ <button label="&junkInfoButton.label;" oncommand="MsgJunkMailInfo(false)"/> >+ <button label="&notJunkButton.label;" oncommand="JunkSelectedMessages(false);"/> I suppose that keyboard users know that they can hit J/Shift-J (etc.) to do this ... >+ <button label="&loadRemoteContentButton.label;" oncommand="LoadMsgWithRemoteContent();"/> ... but how do they access this functionality? >+ <button label="&removePhishingBarButton.label;" oncommand="MsgIsNotAScam();"/> And this one? Hide and seek via Tab key can't be the solution. >Index: themes/classic/messenger/primaryToolbar.css >Index: themes/modern/messenger/primaryToolbar.css >=================================================================== >+ list-style-image: url("chrome://messenger/skin/icons/junkBar.gif"); [...] >+ list-style-image: url("chrome://messenger/skin/icons/remote-blocked.png"); Then we need these icons, too... >Index: xpfe/communicator/resources/content/contentAreaClick.js >=================================================================== >+ function linkNodeForClickEvent(event) [...] >@@ -144,56 +145,49 @@ > default: > linkNode = findParentNode(event.originalTarget, "a"); > // <a> cannot be nested. So if we find an anchor without an > // href, there is no useful <a> around the target > if (linkNode && !linkNode.hasAttribute("href")) > linkNode = null; > break; > } >- var href; >- if (linkNode) { >- href = new XPCNativeWrapper(linkNode, "href").href; >- } else { >+ >+ if (!linkNode) { > // Try simple XLink > linkNode = target; >- while (linkNode) { >- if (linkNode.nodeType == Node.ELEMENT_NODE) { >- var wrapper = new XPCNativeWrapper(linkNode, "getAttributeNS()"); >- href = wrapper.getAttributeNS("http://www.w3.org/1999/xlink", "href"); >- break; >- } >+ while (linkNode.nodeType != Node.ELEMENT_NODE) { > linkNode = linkNode.parentNode; > } >- if (href && href != "") { >- var baseURI = new XPCNativeWrapper(linkNode, "baseURI").baseURI; >- href = makeURLAbsolute(baseURI, href); >- } While XPCWrappers should be on by default (as per attachment 184151 [details] [diff] [review] of bug 281988), I don't see why makeURLAbsolute should be removed. Can you explain this? (The pre-phishing-detector Thunderbird version of this function is quite old!) >- function makeURLAbsolute( base, url ) See above. AFAICT, the following stuff is taken from phishingDetector.js. I don't think it belongs into contentAreaClick.js, especially since it's focused on mail and contentAreaClick.js is used by navigator.xul, too. It'd be best to have the our own phishingDetector.js (and this would help us keeping that stuff synced). >+ if (!aSilentMode && isPhishingURL) // allow the user to over ride the decision Inferior nit: override >+function hostNameIsIPAddress(aHostName, aUnobscuredHostName) [...] >+ // convert the dword into its component IP parts. >+ ipComponents = new Array; >+ ipComponents[0] = (aHostName >> 24) & 255; >+ ipComponents[1] = (aHostName >> 16) & 255; >+ ipComponents[2] = (aHostName >> 8) & 255; >+ ipComponents[3] = (aHostName & 255); ipComponents = [ (aHostName >> 24) & 255, (aHostName >> 16) & 255, (aHostName >> 8) & 255, aHostName & 255 ]; should be slightly more performant. >+ //where one component is hex, another is octal, etc. Inferior nit: add a space behind // >+ var hostName = ipComponents[0] + '.' + ipComponents[1] + '.' + ipComponents[2] + '.' + ipComponents[3]; var hostName = ipComponents.join("."); >+function isIPv4HostName(aHostName) >+{ >+ var ipv4HostRegExp = new RegExp(/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/); // IPv4 >+ // treat 0.0.0.0 as an invalid IP address >+ if (ipv4HostRegExp.test(aHostName) && aHostName != '0.0.0.0') >+ return true; >+ else >+ return false; return ipv4HostRegExp.test(aHostName) && aHostName != '0.0.0.0'; >+function confirmSuspiciousURL(aPhishingType, aSuspiciousHostName) >+{ >+ var brandShortName = gBrandBundle.getString("brandShortName"); >+ var titleMsg = gMessengerBundle.getString("confirmPhishingTitle"); >+ var dialogMsg; var dialogMsg = null; is somewhat 'nicer' than calling ConfirmEx with an 'undefined' value.

In general, i don't like forking. I also don't mind if nsMailnewsContentBlocker.cpp gets dropped, if there is a better replacement. I wanted it to split it out from the browser content policy stuff, because the mailnews rules are very simple: just block everything.

Changes since v0.1c * Re-wrote patch to use phishingDetector.js * Added keys for Not Scam/Show Remote Content and also menuitems for them * Fixed issue with emails with v-cards showing up as being scams * Fixed issue with emails with v-cards causing an exception as some of their linkNodes can have blank hrefs. There may well be a better way of testing if we're using contentAreaClick.js from mailnews than is currently in there...

(In reply to comment #5) I'd say that this is the most "unhackish" solution: > 2) Drop extensions/permissions/nsMailnewsContentBlocker.cpp and use > mailnews/base/src/nsMsgContentPolicy.cpp with the nsMsgCookiePolicy bits > moved out into a separate file for TB to use. [...] > I'd say my least favourite option is the 3rd one, but it is possible TB folks > will not let the first two options through. Actually, I don't expect major resistance there: David, what's your say?

Scott's really the expert on the content policy stuff - adding him to the cc list.

Comment on attachment 186276 [details] [diff] [review] patch with phishingDetector.js v0.1d >Index: mailnews/base/resources/content/mail3PaneWindowCommands.js >=================================================================== >+ case "cmd_markAsShowRemote": >+ case "cmd_markAsNotPhish": > return (GetNumSelectedMessages() > 0 ); These items should only be enabled if the respective state was set by the phishing detector. >Index: mailnews/base/resources/content/mailWindow.js >=================================================================== > function messagePaneOnClick(event) > { > // if this is stand alone mail (no browser) > // or this isn't a simple left click, do nothing, and let the normal code execute > if (event.button != 0 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) > { >- contentAreaClick(event); >- return; >+ return contentAreaClick(event); > } > > // try to determine the href for what you are clicking on. > // for example, it might be "" if you aren't left clicking on a link >- var href = hrefForClickEvent(event); >- if (!href) >- return; >+ var linkNode = linkNodeForClickEvent(event); >+ if (!linkNode || !linkNode.href) >+ return true; >+ var href = linkNode.href; If we change from hrefForClickEvent to linkNodeForClickEvent, the makeURLAbsolute call for href is lost - that's not good. We have at least two occasions where this should still be done (here and in contentAreaClick), so I'd say it would be useful to accompany linkNodeForClickEvent with this (from hrefForClickEvent): function getHrefOfLinkNode(aLinkNode) { var href; if (aLinkNode) href = aLinkNode.href; else { // Try simple XLink while (aLinkNode) { if (aLinkNode.nodeType == Node.ELEMENT_NODE) { href = aLinkNode.getAttributeNS("http://www.w3.org/1999/xlink", "href"); break; } aLinkNode = aLinkNode.parentNode; } if (href) href = makeURLAbsolute(aLinkNode.baseURI, href); } return href; } So this >+ var href = linkNode.href; becomes var href = getHrefOfLinkNode(linkNode); >Index: mailnews/base/resources/content/mailWindowOverlay.js >=================================================================== >+ // aUrl is the nsIURI for the message currently loaded in the message pane >+ setPhishingMsg: function(aUrl) >+ { >+ var phishingMsg = true; >+ var msgURI = GetLoadedMessage(); >+ if (msgURI && !(/type=x-message-display/.test(msgURI))) >+ { >+ var msgHdr = messenger.messageServiceFromURI(msgURI).messageURIToMsgHdr(msgURI); >+ // if we've explicitly marked this message as not being an email scam, then don't >+ // bother checking it with the phishing detector. >+ if (msgHdr && msgHdr.getUint32Property("notAPhishMessage")) >+ phishingMsg = false; phishingMsg = !msgHdr || !msgHdr.getUint32Property("notAPhishMessage"); (And move the var msgHdr below the comment.) >+ } >+ >+ phishingMsg = phishingMsg ? isMsgEmailScam(aUrl) : false; Setting boolean variables to boolean constants in dependance of boolean expressions is just ugly. ;-) if (phishingMsg) phishingMsg = isMsgEmailScam(aUrl); >Index: mailnews/base/resources/content/mailWindowOverlay.xul >=================================================================== >+ <menuitem label="&markAsShowRemoteCmd.label;" >+ accesskey="&markAsShowRemoteCmd.accesskey;" >+ command="cmd_markAsShowRemote"/> >+ <menuitem label="&markAsNotPhishCmd.label;" >+ accesskey="&markAsNotPhishCmd.accesskey;" >+ command="cmd_markAsNotPhish"/> > <menuitem label="&markAsJunkCmd.label;" > accesskey="&markAsJunkCmd.accesskey;" > command="cmd_markAsJunk"/> > <menuitem label="&markAsNotJunkCmd.label;" > accesskey="&markAsNotJunkCmd.accesskey;" > command="cmd_markAsNotJunk"/> > <menuitem label="&recalculateJunkScoreCmd.label;" > accesskey="&recalculateJunkScoreCmd.accesskey;" Maybe move these items after the junk stuff, as that is more likely to be used? Feel free to argue. ;-) (If moving here, then so at the next two occurences, too.) >Index: mailnews/base/resources/content/messageWindow.js >=================================================================== >+ case "cmd_markAsShowRemote": >+ case "cmd_markAsNotPhish": > return (gCurrentMessageUri != null); See mail3PaneWindowCommands.js above. >Index: mailnews/base/resources/content/phishingDetector.js >=================================================================== >+ >+ var hostName = ipComponents.join("."); >+ >+ // only set aUnobscuredHostName if we are looking at an IPv4 host name >+ if (isIPv4HostName(hostName)) Move the var line after the comment. >Index: mailnews/base/resources/locale/en-US/messenger.dtd >=================================================================== > <!-- junk bar --> >+<!-- Remote Content Bar --> >+<!-- Phshing bar Bar --> I'd sell a bar and buy an i. ;-) And use the same capitalization on all three lines. >Index: xpfe/communicator/resources/content/contentAreaClick.js >=================================================================== >- function hrefForClickEvent(event) getHrefOfLinkNode should reside here. >+ function linkNodeForClickEvent(event) > function contentAreaClick(event) > { > if (!event.isTrusted) { > return true; > } > > var isKeyPress = (event.type == "keypress"); >- var href = hrefForClickEvent(event); >- if (href) { >+ var linkNode = linkNodeForClickEvent(event); Add var href = getHrefOfLinkNode(linkNode); >+ if (linkNode && linkNode.href) { if (linkNode && href) { > if (isKeyPress) { >- openNewTabWith(href, true, event.shiftKey); >+ openNewTabWith(linkNode.href, true, event.shiftKey); No change needed here then. > event.preventBubble(); > } > else { >- handleLinkClick(event, href, null); >+ handleLinkClick(event, linkNode.href, null); No change needed here then. The remote content backend issue should go into another diff. :)

Ah, two late changes: > function getHrefOfLinkNode(aLinkNode) > { > var href; var href = ""; > >+ function linkNodeForClickEvent(event) > > > function contentAreaClick(event) > > { > > if (!event.isTrusted) { > > return true; > > } > > > > var isKeyPress = (event.type == "keypress"); > >- var href = hrefForClickEvent(event); > >- if (href) { > >+ var linkNode = linkNodeForClickEvent(event); > > Add > var href = getHrefOfLinkNode(linkNode); > > >+ if (linkNode && linkNode.href) { > > if (linkNode && href) { if (href) does suffice here then.

Changes since v0.1e * Moved phishing/remote content menu items to have junk mail menu items * Phishing/remote content menu items checks msghdr properties for enablement * Changed function in contentAreaClick.js to return both href and linkNode as they are both needed * Corrects some spelling mistakes * Other review comments addressed

Comment on attachment 187247 [details] [diff] [review] patch with href and linknode patch v0.1e >Index: mailnews/base/resources/content/mail3PaneWindowCommands.js >=================================================================== >+ case "cmd_markAsShowRemote": >+ return (GetNumSelectedMessages() > 0 && checkMsgHdrProperty("remoteContentPolicy", 2)); >+ case "cmd_markAsNotPhish": >+ return (GetNumSelectedMessages() > 0 && checkMsgHdrProperty("notAPhishMessage", 1)); Please use constants for these values (1 and 2), the more shared the better. >Index: mailnews/base/resources/content/mailWindowOverlay.js >=================================================================== >+ setRemoteContentMsg: function(aMsgHdr) >+ { >+ var blockRemote = aMsgHdr && >+ aMsgHdr.getUint32Property("remoteContentPolicy") == kBlockRemoteContent; >+ this.updateMsgNotificationBar(kMsgNotificationRemoteImages, blockRemote); >+ }, Shouldn't this, too, call checkMsgHdrProperty? >+ setPhishingMsg: function(aUrl) >+ { >+ // if we've explicitly marked this message as not being an email scam, then don't >+ // bother checking it with the phishing detector. >+ var phishingMsg = false; >+ if (!checkMsgHdrProperty("notAPhishMessage", 0)) Again, a "talking" constant would be better. >+function LoadMsgWithRemoteContent() >+{ >+ // we want to get the msg hdr for the currently selected message >+ // change the "remoteContentBar" property on it >+ // then reload the message >+ >+ var msgURI = GetLoadedMessage(); >+ >+ if (msgURI && !(/type=x-message-display/.test(msgURI))) >+ { >+ var msgHdr = messenger.messageServiceFromURI(msgURI).messageURIToMsgHdr(msgURI); >+ if (msgHdr) >+ { >+ msgHdr.setUint32Property("remoteContentPolicy", kAllowRemoteContent); Now you've factored out the getUint32Property patterns into checkMsgHdrProperty, it'd make sense to make that a set of getMsgHdrProperty/setMsgHdrProperty and factor out the above pattern also? >Index: mailnews/base/resources/content/messageWindow.js >=================================================================== >+ case "cmd_markAsShowRemote": >+ return (gCurrentMessageUri != null && checkMsgHdrProperty("remoteContentPolicy", 2)); >+ case "cmd_markAsNotPhish": >+ return (gCurrentMessageUri != null && checkMsgHdrProperty("notAPhishMessage", 1)); See mail3PaneWindowCommands.js. >Index: xpfe/communicator/resources/content/contentAreaClick.js >=================================================================== >+ if (href && href != "") |if (href)| does suffice, because Boolean("") == false. So, nearly there, I'd say. :) No structural differences left, only the above style and refactoring nits...

Changes since v0.1e: * Moved to using constants instead of values in mailWindowOverlay.js, mail3PaneWindowCommands.js and messageWindow.js * Factored out setting of property to create setMsgHdrPropertyAndReload function * Made requested change to contentAreaClick.js setRemoteContentMsg already does equivalent check to that in checkMsgHdrPropertyIsNot and is passed aMsgHdr so does not need to work it out -> left function alone

Comment on attachment 187982 [details] [diff] [review] setMsgHdrPropertyAndReload patch v0.1f >Index: mailnews/base/resources/content/mailWindowOverlay.js >=================================================================== >+const kIsAPhishMessage = 0; >+const kNotAPhishMessage = 1; Move these down right before the kMsgNotification* constants to emphasize their scope. r=me with that. :)

Changes since v0.1f: * Moved the Phish constants in mailWindowOverlay.js to just before the kMsgNotification* ones Carrying forward r= and requesting sr=

Comment on attachment 188065 [details] [diff] [review] moved constants patch v0.1g Rqeuesting sr from David as he sr'd the TB equivalent of this bug

Changes since v0.1g: * Some unbitrotting Carrying forward r= and requesting sr=

Comment on attachment 189414 [details] [diff] [review] Unbitrotted patch v0.1h Requesting approval for reasonably low risk, suite only patch

This is the patch that was actually checked in. Only difference was some of changes in contentAreaClick.js had been checked in as part of Bug 295582

Comment on attachment 191987 [details] [diff] [review] Re-unbitrotted patch v0.1i (Checked in) Checking in mailnews/jar.mn; new revision: 1.99; previous revision: 1.98 mailnews/mailnews.js; new revision: 3.248; previous revision: 3.247 mailnews/base/resources/content/mail3PaneWindowCommands.js; new revision: 1.145; previous revision: 1.144 mailnews/base/resources/content/mail3PaneWindowVertLayout.xul; new revision: 1.106; previous revision: 1.105 mailnews/base/resources/content/mailWindow.js; new revision: 1.100; previous revision: 1.99 mailnews/base/resources/content/mailWindowOverlay.js; new revision: 1.221; previous revision: 1.220 mailnews/base/resources/content/mailWindowOverlay.xul; new revision: 1.298; previous revision: 1.297 mailnews/base/resources/content/messageWindow.js; new revision: 1.105; previous revision: 1.104 mailnews/base/resources/content/messageWindow.xul; new revision: 1.81; previous revision: 1.80 mailnews/base/resources/content/messenger.xul; new revision: 1.258; previous revision: 1.257 mailnews/base/resources/content/msgHdrViewOverlay.js; new revision: 1.141; previous revision: 1.140 mailnews/base/resources/content/msgMail3PaneWindow.js; new revision: 1.282; previous revision: 1.281 mailnews/base/resources/content/phishingDetector.js; initial revision: 1.1 mailnews/base/resources/locale/en-US/messenger.dtd; new revision: 1.202; previous revision: 1.201 mailnews/base/resources/locale/en-US/messenger.properties; new revision: 1.122; previous revision: 1.121 themes/classic/jar.mn; new revision: 1.136; previous revision: 1.135 themes/classic/messenger/primaryToolbar.css; new revision: 1.12; previous revision: 1.11 themes/classic/messenger/icons/remote-blocked.png; initial revision: 1.1 themes/modern/jar.mn; new revision: 1.150; previous revision: 1.149 themes/modern/messenger/primaryToolbar.css; new revision: 1.17; previous revision: 1.16 themes/modern/messenger/icons/remote-blocked.png; initial revision: 1.1 xpfe/communicator/resources/content/contentAreaClick.js; new revision: 1.50; previous revision: 1.49 done

[Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050809 SeaMonkey/1.0a] (nightly) (WXP) (I have 2 Pop3 accounts in my profile, junk control activated) Seeing this exception: (not sure yet at which step) {{ Error: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIIOService.newURI]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "JS frame :: chrome://messenger/content/phishingDetector.js :: isPhishingURL :: line 107" data: no] Source File: chrome://messenger/content/phishingDetector.js Line: 107 }}

Comment on attachment 191987 [details] [diff] [review] Re-unbitrotted patch v0.1i (Checked in) >Index: themes/modern/messenger/primaryToolbar.css ... >+/* ::::: message notification bar style rules ::::: */ > >-#junkBar { >+.msgNotificationBar { > border-bottom: 1px solid; > -moz-border-bottom-colors: #000000; >+ -moz-appearance: toolbox; > background-color: #C7BC8F; >+ color: black; > } Modern should never use -moz-appearance: ... also Modern style is to use #000000 instead of colour names.