Closed Bug 625085 Opened 13 years ago Closed 13 years ago

mailWindowOverlay.js: fixup LoadMsgWithRemoteContent() and allowRemoteContentForSender()

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b2

People

(Reporter: philip.chee, Assigned: philip.chee)

Details

Attachments

(2 files)

Port:
Bug 308875 'space bar' message reading scrolling broken for formatted messages (with remote images).
Bug 465577 exception clicking on "Always allow images from ..." since Mac OS X address book is readonly.
Straight port.
Attachment #503185 - Flags: review?(mnyromyr)
Attachment #503185 - Flags: superreview?(neil)
Attachment #503185 - Flags: superreview?(neil) → superreview+
<Mnyromyr>	RattyAway: using try to catch programming errors is ugly
<RattyAway>	Mnyromyr: any suggestions? besides the try/catch was already there...
<Mnyromyr>	RattyAway: if just the existence of the function to call is doubtful, use "if ('function name' in object)"
	RattyAway: if the function might itself throws exceptions, it's a different matter, of course
<RattyAway>	Mnyromyr: The IDL says: "@exception NS_ERROR_NOT_IMPLEMENTED If the collection cannot do this."
	http://mxr.mozilla.org/comm-central/source/mailnews/addrbook/public/nsIAbCollection.idl#85
Comment on attachment 503185 [details] [diff] [review]
Patch v1.0 fixups.

> function msgHdrForCurrentMessage()
> {
>@@ -2766,18 +2767,23 @@ function allowRemoteContentForSender()
>                              .getService(Components.interfaces.nsIAbManager)
>                              .directories;
>   var cardForEmailAddress;
>   var addrbook;

It'd be nice if you could init those two to null, while you're in the vicinity. :)

>+    // Try/catch because some cardForEmailAddress functions may not be
>+    // implemented.

And a less confusing comment would be helpful. How about "cardForEmailAddress will throw if not implemented"?

>     try {

And move the { on its own line, while you're here.

r=me with that.
Attachment #503185 - Flags: review?(mnyromyr) → review+
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/d3f37c90dfd9

> Karsten Düsterloh      2011-01-13 15:37:58 PST
> 
> Comment on attachment 503185 [details] [diff] [review]
> Patch v1.0 fixups.
> 
>> function msgHdrForCurrentMessage()
>> {
>>@@ -2766,18 +2767,23 @@ function allowRemoteContentForSender()
>>                              .getService(Components.interfaces.nsIAbManager)
>>                              .directories;
>>   var cardForEmailAddress;
>>   var addrbook;
> 
> It'd be nice if you could init those two to null, while you're in the vicinity.
> :)
Fixed.

>>+    // Try/catch because some cardForEmailAddress functions may not be
>>+    // implemented.
> 
> And a less confusing comment would be helpful. How about "cardForEmailAddress
> will throw if not implemented"?
Fixed.

>>     try {
> 
> And move the { on its own line, while you're here.
Fixed.

> r=me with that.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: