Last Comment Bug 625085 - mailWindowOverlay.js: fixup LoadMsgWithRemoteContent() and allowRemoteContentForSender()
: mailWindowOverlay.js: fixup LoadMsgWithRemoteContent() and allowRemoteContent...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b2
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-12 09:12 PST by Philip Chee
Modified: 2011-01-14 03:06 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 fixups. (1.94 KB, patch)
2011-01-12 09:20 PST, Philip Chee
mnyromyr: review+
neil: superreview+
Details | Diff | Splinter Review
Patch as checked in. (2.24 KB, patch)
2011-01-14 02:27 PST, Philip Chee
no flags Details | Diff | Splinter Review

Description Philip Chee 2011-01-12 09:12:42 PST
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 2011-01-12 09:20:31 PST
Created attachment 503185 [details] [diff] [review]
Patch v1.0 fixups.

Straight port.
Comment 2 Philip Chee 2011-01-13 01:14:20 PST
<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 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. :)

>+    // 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 2011-01-14 02:27:37 PST
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.

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