javascript strict warnings in mail-offline.js

VERIFIED FIXED

Status

VERIFIED FIXED
17 years ago
13 years ago

People

(Reporter: bugzilla, Assigned: sspitzer)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

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

Updated

17 years ago
QA Contact: esther → stephend
yes, I got it.  cleaning up the code, fix in hand.
Status: NEW → ASSIGNED
whoops, new patch coming.  checkfunc needs to return a value, to prevent us 
from going off line if the user doesn't want to.
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

17 years ago
*** Bug 83525 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 6

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

obsolete. is there a new patch underway?
Attachment #52271 - Attachment is obsolete: true

Comment 7

17 years ago
Created attachment 80393 [details] [diff] [review]
proposed patch

this patch fixes it in my tree...
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

17 years ago
Created attachment 80394 [details] [diff] [review]
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.

Updated

17 years ago
Attachment #80393 - Attachment is obsolete: true

Comment 10

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

17 years ago
thinko; s/bug/patch/ sorry for the spam
your patch looks wrong, you should really |return true| as I'll do in the patch
I'll attach in a second

Comment 14

17 years ago
Comment on attachment 81661 [details] [diff] [review]
patch v3

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

r=morten@nilsen.com
Attachment #81661 - Flags: review+

Comment 15

17 years ago
Comment on attachment 81661 [details] [diff] [review]
patch v3

sr=alecf
Attachment #81661 - Flags: superreview+
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
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.
Verified FIXED with the latest trunk self-build on Windows 2000 with JS strict
warnings enabled.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.