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

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
MailNews: Message Display
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.1b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
Created attachment 503185 [details] [diff] [review]
Patch v1.0 fixups.

Straight port.
Attachment #503185 - Flags: review?(mnyromyr)
(Assignee)

Updated

7 years ago
Attachment #503185 - Flags: superreview?(neil)

Updated

7 years ago
Attachment #503185 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 2

6 years ago
<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

6 years ago
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+
(Assignee)

Comment 4

6 years ago
Created attachment 503788 [details] [diff] [review]
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.
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Target Milestone: --- → seamonkey2.1b2
You need to log in before you can comment on or make changes to this bug.