532035 - /mailnews/test/resources/ cause xpcshell-tests fail on JS strict mode

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Jae-Seong Lee-Russo]

Attachment: /mailnews/test/resources/, 0

>>>>>>>
  ../../mailnews/resources/alertTestUtils.js:62: TypeError: anonymous function does not always return a value:
../../mailnews/resources/alertTestUtils.js:62:   },

../../mailnews/resources/alertTestUtils.js:62: ...................................................................^

  <<<<<<<

Comment 1

[Mark Banner (:standard8)]

Comment on attachment 415340 [details] [diff] [review]
/mailnews/test/resources/, 0

> var alertUtilsPrompts = {
>   alert: function(aDialogTitle, aText) {
>     if (typeof alert == "function") {
>-      alert(aDialogTitle, aText);
>-      return;
>+      return alert(aDialogTitle, aText);
>     }
> 
>     do_throw("alert unexpectedly called: " + aText + "\n");
>+    return 0;
>   },
>   
>   alertCheck: function(aDialogTitle, aText, aCheckMsg, aCheckState) {
...

These two functions shouldn't return a value at all (see http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/public/nsIPrompt.idl).

>   confirm: function(aDialogTitle, aText) {
>     if (typeof confirm == "function") {
>       return confirm(aDialogTitle, aText);
>     }
> 
>     do_throw("confirm unexpectedly called: " + aText + "\n");
>+    return 0;
>   },

For this function and the following:

confirmCheck
prompt
promptUsernameAndPassword
promptPassword
select

I'd prefer them to return false and not 0 as they are meant to return booleans (yes, the do_throw means it'll never get called, but it will be more consistent with the idl).


> var alertUtilsPromptService = {

With this one again, I'd like to follow the idl return types (which are actually the same as the previous http://mxr.mozilla.org/comm-central/source/mozilla/embedding/components/windowwatcher/public/nsIPromptService.idl).

Comment 2

[Jae-Seong Lee-Russo]

Attachment: /mailnews/test/resources/, 1, comment #1

Comment 3

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/a49a504fa002

Thanks for fixing this.