102517 - javascript strict warnings in mail-offline.js

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Henrik Gemal]

Just starting mailnews with a clean profile produces:

Warning: function PromptSendMessages does not always return a value
Source File: chrome://messenger/content/mail-offline.js
Line: 140
Source Code:
}

Warning: function PromptDownloadMessages does not always return a value
Source File: chrome://messenger/content/mail-offline.js
Line: 186
Source Code:
}

Warning: function MailCheckBeforeOfflineChange does not always return a value
Source File: chrome://messenger/content/mail-offline.js
Line: 271
Source Code:
}

Build 20011001

Comment 1

[(not reading, please use [e-mail] instead)]

yes, I got it.  cleaning up the code, fix in hand.

Comment 2

[(not reading, please use [e-mail] instead)]

Attachment: clean up.

Comment 3

[(not reading, please use [e-mail] instead)]

whoops, new patch coming.  checkfunc needs to return a value, to prevent us 
from going off line if the user doesn't want to.

Comment 4

[Alex Vincent [:WeirdAl]]

Anything new on this?  I was about to create a patch, but I figured I'd better
check Bugzilla first.  Last comment was two months ago.

Comment 5

[Stephen Walker]

*** Bug 83525 has been marked as a duplicate of this bug. ***

Comment 6

[Henrik Gemal]

Comment on attachment 52271 [details] [diff] [review]
clean up.

obsolete. is there a new patch underway?

Comment 7

[Morten Nilsen]

Attachment: proposed patch

this patch fixes it in my tree...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Um.... that patch looks odd.  That makes the function _always_ return false,
effectively.  That seems wrong, given the comment at the start of the function.

In fact, it looks like the final return should be |return true;|, from a brief
skim of the code.

Comment 9

[Morten Nilsen]

Attachment: patch v2

This is the alternative... I will try both to see if they make any difference

either may work, or both may.
a third alternative is to remove the "false" from the other returns, but that
may not work, if the comment is to be believed :) will investigate further.

Comment 10

[Morten Nilsen]

Comment on attachment 80393 [details] [diff] [review]
proposed patch

the other bug was the way to go.
I found this out by stepping thru the code in my head, and a little help from
bz

Comment 11

[Morten Nilsen]

thinko; s/bug/patch/ sorry for the spam

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

your patch looks wrong, you should really |return true| as I'll do in the patch
I'll attach in a second

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v3

Comment 14

[Morten Nilsen]

Comment on attachment 81661 [details] [diff] [review]
patch v3

I actually made this change locally, but forgot to attach a new diff :P

r=[e-mail]

Comment 15

[Alec Flett]

Comment on attachment 81661 [details] [diff] [review]
patch v3

sr=alecf

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in

Comment 17

[Christopher Aillon (sabbatical, not receiving bugmail)]

I love strict warning fixes.  But I don't like when they break functionality. 
This last checkin on the latest patch caused regression bug 143708.  Please make
sure you test what you're fixing in strict warning patches.

Comment 18

[[e-mail] (gone - use [e-mail] instead)]

Verified FIXED with the latest trunk self-build on Windows 2000 with JS strict
warnings enabled.