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

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Philip Chee]

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.

Comment 1

[Philip Chee]

Attachment: Patch v1.0 fixups.

Straight port.

Comment 2

[Philip Chee]

<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 3

[Karsten Düsterloh]

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.

Comment 4

[Philip Chee]

Attachment: Patch as checked in.

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.