Closed Bug 826732 Opened 12 years ago Closed 9 years ago

JavaScript strict warning seen during "make mozmill" run of TB (DEBUG BUILD)

Categories

(Thunderbird :: Testing Infrastructure, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ishikawa, Unassigned)

References

(Depends on 4 open bugs, Blocks 1 open bug)

Details

(Whiteboard: [leave open])

Attachments

(3 files, 5 obsolete files)

While testing thunderbird (debug build of comm-central source) by
running "make mozmill" locally, I noticed many JavaScript strict
warning warning messages.

Background:

My point of raising the issue here is to produce less clutter in the
log of "make mozmill" run of TB (DEBUG BUILD) so that we can focus on
real problems by eliminating JavaScript strict warning messages as
many as we can.
So, even if the warning might be a benign kind to the veteran's eyes,
we should edit the script files IMHO.
(Some of the warnings are possibly due to variables not bound to a
proper value due to error handling path, but then, such 
cases should be properly handled without triggering strict warning.)

I understand there is a discussion about whether capturing and
analyzing these warnings may be a good idea or not (in the sense of
return on investment).

Bug 811078 - Capture JavaScript strict warnings during xpcshell harness execution
(especially see comment, https://bugzilla.mozilla.org/show_bug.cgi?id=811078#c2 , and its follow-up. )

But, for improving the quality of TB and making it easy for the
long-term maintenance, I would argue for removing the constructs that
lead to these warning messages. (As much as we can so that signal to
noise ratio is good enough.)

I will file a separate bug entry for the files listed below, and
make this bug entry as the META BUG entry for them.
(Probably in the future, I will create a further meta bug to
reduce the clutter in the session log of "make mozmill" run of TB
[DEBUG BUILD] 
The current session-log is so voluminous and signal-to-noise ratio
is low. And there *ARE* NS_ERROR_* 
error values that are thrown and not caught all the way up to the
top-level and these, I think, are real bugs, but it seems that
not many people are looking at the output of make mozmill run of TB [DEBUG BUILD])

List of JavaScript strict warnings:

Here is the output of
grep ^JavaScript Session-Log-File | sort | uniq -c | sort -n -r | egrep -v "(jquery.js|jquery-ui.js)"

(The scripts, jquery.js and jquery-ui.js, produce so many warnings and
they are from an upstream site (http://jquery.com), I removed them
from the listing below.)

A few script files below seem to be relevant only during testing
(marked by * at the beginning of the line), but other files do seem to
have problems and used in TB in production.

     52 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 352: reference to undefined property aTabType.panelId
     52 JavaScript strict warning: chrome://global/content/bindings/browser.xml, line 223: reference to undefined property this.boxObject.QueryInterface(...).docShell
     46 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 86: reference to undefined property event.detail
*    25 JavaScript error: resource://mozmill/modules/frame.js -> file:///TB-NEW/TB-3HG/new-src/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///TB-NEW/TB-3HG/new-src/mailnews/test/resources/logHelper.js, line 397: TypeError: null has no properties
*    18 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///TB-NEW/TB-3HG/new-src/mail/test/mozmill/shared-modules/test-cloudfile-helpers.js, line 134: setting a property that has only a getter
     13 JavaScript strict warning: chrome://messenger/content/mailWindowOverlay.js, line 3386: reference to undefined property aMsgHdr.flags
     12 JavaScript strict warning: chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 245: anonymous function does not always return a value
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 233: anonymous function does not always return a value
      7 JavaScript strict warning: chrome://global/content/bindings/radio.xml, line 133: reference to undefined property val.value

      (The above is already reported in 
      https://bugzilla.mozilla.org/show_bug.cgi?id=824324
      and it seems that there may be a deeper cause and it is not 
      a matter of simple rewriting of the script.)

      6 JavaScript strict warning: chrome://messenger/content/messageWindow.js, line 264: function StandaloneMessageDisplayWidget_onMessagesRemoved does not always return a value
      5 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 53: reference to undefined property this.treeBoxObject.view
      3 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 101: reference to undefined property this.onEditableArea
      3 JavaScript strict warning: chrome://messenger/content/addressbook/abTrees.js, line 54: in strict mode code, functions may be declared only at top level or immediately within another function
      3 JavaScript strict warning: chrome://messenger/content/accountcreation/readFromXML.js, line 39: reference to undefined property xml.displayName
      2 JavaScript strict warning: chrome://editor/content/EdDialogCommon.js, line 1006: in strict mode code, functions may be declared only at top level or immediately within another function
*     1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///TB-NEW/TB-3HG/new-src/mail/test/mozmill/shared-modules/test-window-helpers.js, line 505: reference to undefined property this.waitingList[windowType]
*     1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///TB-NEW/TB-3HG/new-src/mail/test/mozmill/cloudfile/test-cloudfile-notifications.js, line 425: reference to undefined property Ci.nsIMsgCloudFileProvider.NS_OK
*     1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///TB-NEW/TB-3HG/new-src/mail/test/mozmill/cloudfile/test-cloudfile-notifications.js, line 386: reference to undefined property Ci.nsIMsgCloudFileProvider.NS_OK
*     1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///TB-NEW/TB-3HG/new-src/mail/test/mozmill/cloudfile/test-cloudfile-notifications.js, line 347: reference to undefined property Ci.nsIMsgCloudFileProvider.NS_OK
      1 JavaScript strict warning: resource:///modules/mailInstrumentation.js, line 62: reference to undefined property this._accountsChanged
      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1054: reference to undefined property this._underlyingFolders[0]
      1 JavaScript strict warning: chrome://messenger/content/folderPane.js, line 1436: reference to undefined property (intermediate value)[2]


BTW, I searched for "JavaScript strict warning" and came up quite a
few bugzilla entries. But funny thing is that I didn't find mentions
of above warnings except for Bug 824324. [Maybe I missed a few.]

I wonder if this is because (a) DEBUG BUILD of TB is not officially
tested and the log of its test run is not publicly available, 
(b) "make mozmill" test of TB is not comprehensive enough to cover all
the JavaScript files yet, or (c) all of the above.

TIA
Depends on: 824324
Component: Untriaged → General
Depends on: 771084
The current state of affairs from running "make mozmill" on DEBUG BUILD of thunderbird (comm-central)

We now have these warnings:

 ========================================
 JavaScript strict warning
 jquery.js and jquery-ui.js are ignored. 
 ========================================

     53 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 352: reference to undefined property aTabType.panelId
     53 JavaScript strict warning: chrome://global/content/bindings/browser.xml, line 223: reference to undefined property this.boxObject.QueryInterface(...).docShell
     46 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 86: reference to undefined property event.detail
     18 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///COMM-CENTRAL/comm-central/mail/test/mozmill/shared-modules/test-cloudfile-helpers.js, line 134: setting a property that has only a getter
     18 JavaScript error: resource://mozmill/modules/frame.js -> file:///COMM-CENTRAL/comm-central/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///COMM-CENTRAL/comm-central/mailnews/test/resources/logHelper.js, line 390: TypeError: undefined has no properties
     13 JavaScript strict warning: chrome://messenger/content/mailWindowOverlay.js, line 3398: reference to undefined property aMsgHdr.flags
     12 JavaScript strict warning: chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 236: anonymous function does not always return a value
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 208: anonymous function does not always return a value
      7 JavaScript strict warning: chrome://messenger/content/messageWindow.js, line 259: function StandaloneMessageDisplayWidget_onMessagesRemoved does not always return a value
      6 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 53: reference to undefined property this.treeBoxObject.view
      4 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 101: reference to undefined property this.onEditableArea
      3 JavaScript strict warning: chrome://messenger/content/addressbook/abTrees.js, line 54: in strict mode code, functions may be declared only at top level or immediately within another function
      2 JavaScript strict warning: chrome://editor/content/EdDialogCommon.js, line 1006: in strict mode code, functions may be declared only at top level or immediately within another function
      1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///COMM-CENTRAL/comm-central/mail/test/mozmill/shared-modules/test-window-helpers.js, line 505: reference to undefined property this.waitingList[windowType]
      1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///COMM-CENTRAL/comm-central/mail/test/mozmill/cloudfile/test-cloudfile-notifications.js, line 425: reference to undefined property Ci.nsIMsgCloudFileProvider.NS_OK
      1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///COMM-CENTRAL/comm-central/mail/test/mozmill/cloudfile/test-cloudfile-notifications.js, line 386: reference to undefined property Ci.nsIMsgCloudFileProvider.NS_OK
      1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///COMM-CENTRAL/comm-central/mail/test/mozmill/cloudfile/test-cloudfile-notifications.js, line 347: reference to undefined property Ci.nsIMsgCloudFileProvider.NS_OK
      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1054: reference to undefined property this._underlyingFolders[0]
      1 JavaScript strict warning: chrome://messenger/content/threadPaneColumnPicker.xml, line 173: in strict mode code, functions may be declared only at top level or immediately within another function
      1 JavaScript strict warning: chrome://messenger/content/msgMail3PaneWindow.js, line 1043: reference to undefined property treeBoxObj.view
I can fix some of those, patch being prepared.

And I think mconley can fix these 2 as I do not know what are the proper return values for these functions:
JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 236: anonymous function does not always return a value
JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 208: anonymous function does not always return a value
(In reply to :aceman from comment #2)
> I can fix some of those, patch being prepared.
> 
> And I think mconley can fix these 2 as I do not know what are the proper
> return values for these functions:
> JavaScript strict warning:
> chrome://messenger/content/messengercompose/bigFileObserver.js, line 236:
> anonymous function does not always return a value
> JavaScript strict warning:
> chrome://messenger/content/messengercompose/bigFileObserver.js, line 208:
> anonymous function does not always return a value

That will be great. These superficial problems might hide really tricky issues.
Attached patch patch 1 (obsolete) — Splinter Review
Aryx, could you please make a try run with this patch, all tests all platforms? Thanks.
Thanks. Looks like I did break some tests. I just couldn't determine that on my machine alone as there are many temporary failures on linux (popup problems).
I am sure the "reference to undefined property" messages are real bugs and the tests just do not fail by coincidence. Why do we reference something that does not exist? We are in some unexpected state.

There are tons of these warnings. I just get them inconsistently. It looks like when I run a test with "mozmill-one" argument, the messages are more verbose and show more errors than just running "make mozmill". Mconley would know more.
Attached patch patch - part 1 v2 (obsolete) — Splinter Review
This one should be better.
Assignee: nobody → acelists
Attachment #729827 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #730405 - Flags: review?(mconley)
(In reply to :aceman from comment #8)
> I am sure the "reference to undefined property" messages are real bugs and
> the tests just do not fail by coincidence. Why do we reference something
> that does not exist? We are in some unexpected state.
> 
> There are tons of these warnings. I just get them inconsistently. It looks
> like when I run a test with "mozmill-one" argument, the messages are more
> verbose and show more errors than just running "make mozmill". Mconley would
> know more.

I am pleased to learn that there are people with similar sentiment when they
see these tons of warnings :-)

I think we have eliminated easier part of C++ bugs (there are still others
which trigger ASSERTIONS, but I don't know the details of the code yet).
Occasionally, an undefined value creeps up in C++ code, but I have eliminated a few of them already.

So I think it is time to eliminate these undefined warnings in javascript.
(I am not bothering with jquery.js, BTW.)

I am afraid that there are too many warning lines from the DEBUG BUILD each vying for attention and thus the observer get insensitive. TB, at least, needs 
more attention to eliminate these problems. (I suspect, FF, may not fare better, but at least it receives more attention, I think.)

BTW, How do I run "mozmill-one" argument. Does it take the form of "make mozmill argument"???

TIA
(In reply to Mark Banner (:standard8) from comment #11)
> Details of mozmill-one can be found here:
> 
> https://developer.mozilla.org/en-US/docs/Thunderbird/
> Thunderbird_MozMill_Testing#Running_a_specific_test.3A

Will check. Obtaining more messages just because we run tests in a single file does not sound right to me. (Maybe some timing issues? Or are there some persistent data passed to a next run from the previous run that changes the state of running TB image?) Anyway, I will find out the verbosity myself.
(In reply to ISHIKAWA, chiaki from comment #12)
> Anyway, I will find out the verbosity myself.

That would be best. As it is just my feeling because I sometimes could not see the warnings you pasted, but when I looked at the tests more closely I could see them. It can also be I was not properly redirecting all message levels (standard output and error) from the console to the log file.
(In reply to :aceman from comment #13)
> (In reply to ISHIKAWA, chiaki from comment #12)
> > Anyway, I will find out the verbosity myself.
> 
> That would be best. As it is just my feeling because I sometimes could not
> see the warnings you pasted, but when I looked at the tests more closely I
> could see them. It can also be I was not properly redirecting all message
> levels (standard output and error) from the console to the log file.

In my quest for making TB a rock-solid mail client ;-)
I have updated the entry

Bug 824259 - [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIMsgDBView.getMsgHdrsForSelection]" nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: chrome://messenger/content/folderDisplay.js :: FolderDi 

in it, I mention why you may possibly feel that the warning lines, etc.
are higher in a single test case in contrast to a large log of many tests.

But now I see you said,
sometimes "could not see the warnings you pasted". Hmm...
I wonder if these are my own particular warnings I insert in the code or not.
If so, I am sorry to cause your confusion. Maybe I should show exactly what changes are used in the source to print some debug output.


(In reply to Mark Banner (:standard8) from comment #11)
> Details of mozmill-one can be found here:
> 
> https://developer.mozilla.org/en-US/docs/Thunderbird/
> Thunderbird_MozMill_Testing#Running_a_specific_test.3A

Thank you. I figured it out, I could make it work with my scripts
to run TB under valgrind/memcheck/helgrind if necessary. Before it took me a whole day to finish the whole run under valgrind, but maybe I can run such a long run once in a while, and focus on a particular test that causes strange bugs using this SOLO_TEST to cut down elapsed time.

TIA
(In reply to ISHIKAWA, chiaki from comment #14)
> But now I see you said,
> sometimes "could not see the warnings you pasted". Hmm...
> I wonder if these are my own particular warnings I insert in the code or not.
> If so, I am sorry to cause your confusion. Maybe I should show exactly what
> changes are used in the source to print some debug output.

Don't worry, all the messages you pasted are valid. I just couldn't always see them reliably, but eventually I have found all of them.
Attachment #730405 - Attachment description: This one should be better. → patch - part 1 v2
Attached patch patch - part 2 (obsolete) — Splinter Review
This fixes some more warnings.

The "aMsgHdr.flags" one was really hard until I found out we send some dummy header in case we have .eml file in a window.
Attachment #731282 - Flags: review?(mconley)
Comment on attachment 730405 [details] [diff] [review]
patch - part 1 v2

Review of attachment 730405 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few nits. With these fixed, r=me.

::: mail/base/content/threadPaneColumnPicker.xml
@@ +169,5 @@
>          if (useChildren) {
>            // Generate an observer notification when we have finished configuring
>            // all folders.  This is currently done for the benefit of our mozmill
>            // tests.
> +          let observerCallback = function observerCallback() {

Don't need to name the function now, so should just be:

let observerCallback = function() {
  // ...
}

::: mail/components/compose/content/bigFileObserver.js
@@ +81,5 @@
>  
>      if (event.type in bucketCallbacks)
>        bucketCallbacks[event.type].call(this, event.detail);
>  
> +

Remove the new newline

@@ +86,3 @@
>      if (event.type in itemCallbacks)
> +      itemCallbacks[event.type].call(this, event.target,
> +                                     ("detail" in event)?event.detail:null);

I think I'd prefer this spacing:

("detail" in event) ? event.detail : null

::: mail/test/mozmill/shared-modules/test-window-helpers.js
@@ +412,5 @@
>          this.monitoringList.splice(iWin, 1);
>      }
>  
> +    return this.waitingList.has(this.waitingForOpen) &&
> +          (this.waitingList.get(this.waitingForOpen) != null);

nit - indent one space.
Attachment #730405 - Flags: review?(mconley) → review+
Comment on attachment 731282 [details] [diff] [review]
patch - part 2

Review of attachment 731282 [details] [diff] [review]:
-----------------------------------------------------------------

The code looks OK, but I'm not sure about the flags thing, and what impact that could have. You might want Standard8 to take a look at that before checkin-needing this patch.

::: mail/base/content/mailWindowOverlay.js
@@ +3393,5 @@
>     *
>     * @return true if message is a feed, false if not.
>     */
>    isFeedMessage: function (aMsgHdr) {
> +    return Boolean((aMsgHdr instanceof Components.interfaces.nsIMsgDBHdr) &&

We really don't need to wrap this in a Boolean - let's get rid of that.

::: mail/base/content/msgHdrViewOverlay.js
@@ +2747,5 @@
>    messageId : null,
>    date : 0,
>    accountKey : "",
> +  // Shouldn't we have a special message flag to indicate a dummy header?
> +  flags : 0,

Hm... might want to double check with Standard8 on this...

::: mailnews/test/resources/logHelper.js
@@ +403,5 @@
>        type: "domNode",
>        name: name,
>        value: aObj.nodeValue,
>        namespace: aObj.namespaceURI,
>        boundingClientRect: {left: bounds.left, top: bounds.top,

Seems like we could just return bounds here.
Attachment #731282 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) from comment #18)
> ::: mail/base/content/mailWindowOverlay.js
> @@ +3393,5 @@
> >     *
> >     * @return true if message is a feed, false if not.
> >     */
> >    isFeedMessage: function (aMsgHdr) {
> > +    return Boolean((aMsgHdr instanceof Components.interfaces.nsIMsgDBHdr) &&
> 
> We really don't need to wrap this in a Boolean - let's get rid of that.

Maybe it is to convert any failure like null or undefined to normal bool (true/false).
I don't see failures like that happening in here. I think it can be cut.
OK, maybe not now. But before when it was just return "Boolean(aMsgHdr && ... " couldn't the result be null ?
(In reply to :aceman from comment #21)
> OK, maybe not now. But before when it was just return "Boolean(aMsgHdr &&
> ... " couldn't the result be null ?

Yes, I believe that was possible.
Thanks.
Attachment #730405 - Attachment is obsolete: true
Attachment #735325 - Flags: review+
Attached patch patch - part 2 v2 (obsolete) — Splinter Review
Attachment #731282 - Attachment is obsolete: true
Attachment #735326 - Flags: review?(mbanner)
Comment on attachment 735326 [details] [diff] [review]
patch - part 2 v2

Review of attachment 735326 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/msgHdrViewOverlay.js
@@ +2746,5 @@
>    listPost : null,
>    messageId : null,
>    date : 0,
>    accountKey : "",
> +  // Shouldn't we have a special message flag to indicate a dummy header?

I don't think this matters generally, as the places it really matter, we are already handling the header as a "dummyHeader" or "dummyMsgHeader".
Attachment #735326 - Flags: review?(mbanner) → review+
Comment on attachment 735326 [details] [diff] [review]
patch - part 2 v2

Review of attachment 735326 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/test/mozmill/shared-modules/test-window-helpers.js
@@ +1353,5 @@
>    // DOM element.
>    if ("ownerDocument" in elem) {
>      arr.push(normalize_for_json(elem));
> +    if (elem.ownerDocument)
> +      win = elem.ownerDocument.defaultView;

If this is truly a DOM element, ownerDocument won't be null. If it can be a document, maybe you should call it a node rather than an element.
(In reply to Mark Banner (:standard8) from comment #25)
> > +  // Shouldn't we have a special message flag to indicate a dummy header?
> 
> I don't think this matters generally, as the places it really matter, we are
> already handling the header as a "dummyHeader" or "dummyMsgHeader".

But as we can see, we were sending the header to a general function that wasn't aware of any dummy/real difference.
(In reply to :Ms2ger from comment #26)
> ::: mail/test/mozmill/shared-modules/test-window-helpers.js
> @@ +1353,5 @@
> >    // DOM element.
> >    if ("ownerDocument" in elem) {
> >      arr.push(normalize_for_json(elem));
> > +    if (elem.ownerDocument)
> > +      win = elem.ownerDocument.defaultView;
> 
> If this is truly a DOM element, ownerDocument won't be null. If it can be a
> document, maybe you should call it a node rather than an element.

In the tests run, we got cases where ownerDocument was null, therefore I had to add a check for it. Not sure if that is not a bug in some test. However, this function seems universal and is applied on any node.
So I 'd be OK with the renaming. Mconley?
Flags: needinfo?(mconley)
(In reply to :aceman from comment #27)
> (In reply to Mark Banner (:standard8) from comment #25)
> > > +  // Shouldn't we have a special message flag to indicate a dummy header?
> > 
> > I don't think this matters generally, as the places it really matter, we are
> > already handling the header as a "dummyHeader" or "dummyMsgHeader".
> 
> But as we can see, we were sending the header to a general function that
> wasn't aware of any dummy/real difference.

Right, but they generally don't need to know its a dummy - they seemed to be cases that aren't going near message folders, so I think we're fine here.
As it didn't know it is a dummy, it accessed the flags attribute directly and it wasn't existing. We may hit some new missing attributes in the future if we don't have a way to check if a hdr is a dummy.
(In reply to :aceman from comment #30)
> and it wasn't existing. We may hit some new missing attributes in the future
> if we don't have a way to check if a hdr is a dummy.

I think you're thinking of it the wrong way - if it is a dummy header, it should implement (reasonably) all attributes of the interface that may be accessed (just like if it was implemented in c++).
Yes, so can you explain why 'flags' was missing and how this does not happen in the future?
I've added the same flags:0 definition to the suite version of msgHdrViewOverlay.js and changes the name of the elem variable as ms2ger notes.
Attachment #735326 - Attachment is obsolete: true
Attachment #737642 - Flags: review?(mbanner)
Comment on attachment 737642 [details] [diff] [review]
patch - part 2 v3 [checkin: comment 35]

Unfortunately xpcom/js doesn't protect us against missing attributes, so we'll just have to change this as necessary and try and remember to check where to add things when changing interfaces.
Attachment #737642 - Flags: review?(mbanner) → review+
Component: General → Testing Infrastructure
Flags: needinfo?(mconley)
Keywords: checkin-needed
OS: Linux → All
Whiteboard: [check in both patches and leave open]
Attachment #735325 - Attachment description: patch - part 1 v3 → patch - part 1 v3 [checkin: comment 35]
Attachment #737642 - Attachment description: patch - part 2 v3 → patch - part 2 v3 [checkin: comment 35]
Hi,

Thank you for the patches, etc. In another week's time, I will summarize my
finding of the warnings from TB's make mozmill run so that we know 
nothing new sneaks in the JavaScript warning category.

TIA
Thanks. The patches do not fix all warnings but should reduce them much. Please post if it got better.
Chiaki, any news here from the findings you wanted to do ?
Flags: needinfo?(ishikawa)
(In reply to :aceman from comment #38)
> Chiaki, any news here from the findings you wanted to do ?

Thank you for the latest fixes that went into the source tree.

I realized that many issues would have been solved and so wanted to take a snapshot of the status of the warnings, etc.

But the bug https://bugzilla.mozilla.org/show_bug.cgi?id=842114
suddenly got in the way.
Basically, I got so many warnings from an NS_ASSERTION statement that I can't continue running "make mozmill". 
The NS_ASSERTION() seems to be tweaked early this year, but I have no idea
what triggered this so conspicuously. 
Until bug 842114 gets resolved, I can't collect reasonable snapshot of the
JavaScript warnings, etc.

Thank you again.

Stay tuned...
Flags: needinfo?(ishikawa)
Blocks: 303545
Bug 842114 is still being worked on, but I managed to cut down the warnings about half, and luckily after my patch posted there, only about 180- such stack dumps occur from the particular NS_ASSERTION as I found out over the weekend during the entire "make mozmill" test run of Thunderbird DEBUG BUILD.

So here is the snapshot of Java strict warning from running "make mozmill" test of Thunderbird DEBUG BUILD:

The version tested was the source code refreshed on June 4th (JST) (with some additional local patches to address some issues).

The following list is created by a series of command where $1 is the session log file
recorded by script command under linux when I ran "make mozmill" test.
Command:
grep ^JavaScript $1 | sort | uniq -c | sort -r -n | egrep -v "(jquery.js|jquery-ui.js)"

 ========================================
 JavaScript strict warning
 jquery.js and jquery-ui.js are ignored. 
 ========================================

     52 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 352: reference to undefined property aTabType.panelId
     52 JavaScript strict warning: chrome://global/content/bindings/browser.xml, line 223: reference to undefined property this.boxObject.QueryInterface(...).docShell
     12 JavaScript strict warning: chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]
     10 JavaScript strict warning: chrome://messenger/content/mailWindowOverlay.js, line 1122: reference to undefined property msgHdr.author
     10 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 53: reference to undefined property this.treeBoxObject.view
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 237: anonymous function does not always return a value
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 209: anonymous function does not always return a value
      4 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 101: reference to undefined property this.onEditableArea
      2 JavaScript strict warning: chrome://messenger/content/addressbook/abTrees.js, line 54: in strict mode code, functions may be declared only at top level or immediately within another function
      2 JavaScript strict warning: chrome://editor/content/EdDialogCommon.js, line 947: in strict mode code, functions may be declared only at top level or immediately within another function
      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1049: reference to undefined property this._underlyingFolders[0]
      1 JavaScript strict warning: chrome://messenger/content/msgMail3PaneWindow.js, line 1040: reference to undefined property treeBoxObj.view
      1 JavaScript strict warning: chrome://global/content/bindings/textbox.xml, line 100: reference to undefined property this.inputField.QueryInterface(...).editor
      1 JavaScript error: chrome://messenger/content/addressbook/abContactsPanel.js, line 39: cards is null

Simply counting the occurrences, we now have about the half the number of different
warnings now (!) as compared before. Great.
 
We may need to analyze the warnings again to determine the seriousness and the nature.

TIA

CAVEAT: Due to some known test function error(s), some tests may not complete successfully, and so there *may* be a warning (or a few) not caught in this particular run, but I believe the major warnings are caught since they tend to be printed in multiple different tests.

PS: I just noticed there is much earlier generic entry for javascript warnings of thunderbird in Bug 303545.
But bug 303545 seems to be about warnings seen inside the app.

The bug here is fine to keep about warnings from tests.
Chiaki, I can't see the warnings
"JavaScript error: chrome://messenger/content/addressbook/abContactsPanel.js, line 39: cards is null"
and
"JavaScript strict warning: chrome://messenger/content/msgMail3PaneWindow.js, line 1040: reference to undefined property treeBoxObj.view"

In which tests do you see them? Also, do you use a 64bit linux?
I quote the test session log for the warnings below.

I am still using 32-bit linux for now.  (I am about to switch to a new
PC with 16GB memory, then will switch to 64-bit linux.  But switching
motherboard is such a bother. It will take another week or so
before I can be comfortable to migrate to the new box after checking
the driver works for existing peripheral boards in new box, etc. in my spare time.)

For the second warning, after searching through the log and got
confused with "treeBoxObject.view" vs "treeBoxObj.view", I have a
feeling that "treeBoxObj.view" may be a misspelling of
"TreeBoxObject.view" (?)
But then there are 10 warnings of the form: "JavaScript strict
warning: chrome://global/content/bindings/tree.xml, line 53: reference
to undefined property this.treeBoxObject.view"
Either way, we are far from "rock solid" code status.

(1) cards is null warning : in test_send_enabled_address_contacts_sidebar

This is followed immediately with "Test Failure: a != b: 'true' !=
'false'."  From what I understand, such error may be intermittant and
that is why we may see the warning or not depending on this particular
error happens or not.


TEST-START | /COMM-CENTRAL/comm-central/mail/test/mozmill/composition/test-send-button.js | test_send_enabled_address_contacts_sidebar
Step Pass: {"function": "Controller.keypress()"}
++DOCSHELL 0x1338dc48 == 19 [id = 29]
++DOMWINDOW == 41 (0x149bc5dc) [serial = 70] [outer = (nil)]
++DOCSHELL 0x150b1ef8 == 20 [id = 30]
++DOMWINDOW == 42 (0x1584fe04) [serial = 71] [outer = (nil)]
++DOCSHELL 0xd4bd068 == 21 [id = 31]
++DOMWINDOW == 43 (0x16832fdc) [serial = 72] [outer = (nil)]
search "ldap" not found - skipping.
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file /COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp, line 8362
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file /COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp, line 8362
WARNING: Subdocument container has no frame: file /COMM-CENTRAL/comm-central/mozilla/layout/base/nsDocumentViewer.cpp, line 2380
++DOMWINDOW == 44 (0x1545f974) [serial = 73] [outer = 0x149bc5dc]
++DOMWINDOW == 45 (0xddc5c64) [serial = 74] [outer = 0x1584fe04]
WARNING: Subdocument container has no frame: file /COMM-CENTRAL/comm-central/mozilla/layout/base/nsDocumentViewer.cpp, line 2380
++DOMWINDOW == 46 (0x15109ddc) [serial = 75] [outer = 0x16832fdc]
--DOMWINDOW == 45 (0x19594924) [serial = 66] [outer = 0x16b3db44] [url = about:blank]
--DOMWINDOW == 44 (0x16b3db44) [serial = 63] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 43 (0x14d7d674) [serial = 64] [outer = 0x1392a0c4] [url = about:blank]
--DOMWINDOW == 42 (0x1392a0c4) [serial = 61] [outer = (nil)] [url = about:blank]
--DOCSHELL 0x14d18230 == 20 [id = 24]
++DOMWINDOW == 43 (0xcb64b24) [serial = 76] [outer = 0x1584fe04]
WARNING: NS_ENSURE_TRUE(selection->GetRangeCount()) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/base/nsEditor.cpp, line 3833
WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 0x80004005: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp, line 316
WARNING: NS_ENSURE_TRUE(sheet) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2953
search "ldap" not found - skipping.
WARNING: NS_ENSURE_TRUE(aNode) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/base/nsEditor.cpp, line 3528
WARNING: Unable to test style tree integrity -- no content node: file /COMM-CENTRAL/comm-central/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 8253
--DOMWINDOW == 42 (0xdf4ee9c) [serial = 67] [outer = 0x138b960c] [url = about:blank]
--DOMWINDOW == 41 (0x138b960c) [serial = 62] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 40 (0xddc5c64) [serial = 74] [outer = (nil)] [url = about:blank]
++DOMWINDOW == 41 (0x12ba7ee4) [serial = 77] [outer = 0x149bc5dc]
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "Controller.waitForElement()"}
JavaScript error: chrome://messenger/content/addressbook/abContactsPanel.js, line 39: cards is null
Test Failure: a != b: 'true' != 'false'.
--DOCSHELL 0x194e16b0 == 19 [id = 21]
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(editor) failed: file /COMM-CENTRAL/comm-central/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 1604
WARNING: NS_ENSURE_TRUE(editor) failed: file /COMM-CENTRAL/comm-central/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 630
--DOCSHELL 0x195c7c38 == 18 [id = 23]
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(ps) failed: file /COMM-CENTRAL/comm-central/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 2957
WARNING: NS_ENSURE_TRUE(editor) failed: file /COMM-CENTRAL/comm-central/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 1604
WARNING: NS_ENSURE_TRUE(editor) failed: file /COMM-CENTRAL/comm-central/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 630
--DOCSHELL 0xd4bd068 == 17 [id = 31]
++DOCSHELL 0x144775b8 == 18 [id = 32]
++DOMWINDOW == 42 (0x158f5ecc) [serial = 78] [outer = (nil)]
++DOMWINDOW == 43 (0x1563db54) [serial = 79] [outer = 0x158f5ecc]
TEST-UNEXPECTED-FAIL | /COMM-CENTRAL/comm-central/mail/test/mozmill/composition/test-send-button.js | test-send-button.js::test_send_enabled_address_contacts_sidebar

(2) treeBoxObj.view warning in  test_close_multiple_windows_tabs_on_delete_from_3pane_window

    It seems to me that it happens very close to the closing of TB (or
    something) and I suppose that is why I see the message from
    valgrind/memcheck below.

TEST-START | /COMM-CENTRAL/comm-central/mail/test/mozmill/folder-display/test-close-window-on-delete.js | test_close_multiple_windows_tabs_on_delete_from_3pane_window
Step Pass: {"function": "Controller.keypress()"}
GetNewMsgOutputStream
offset = 10302
key sent to EnsureKeyExists = 10302
row for the key created.
new key = 10302
key sent into hdr = 10302
key sent to EnsureKeyExists = 10302
GetSearchResultsTable:StringToToken()=0x00000000, searchFolderUri=mailbox://nobody@smart%20mailboxes/Trash
GetSearchResultsTable: GetTableKind()=0x00000000, numTables=33, *table=0x13a8f284
--DOCSHELL 0x16fe85d0 == 20 [id = 107]
--DOCSHELL 0x134c92e0 == 19 [id = 110]
GetNewMsgOutputStream
offset = 0
key sent to EnsureKeyExists = 0
row for the key created.
new key = 0
key sent into hdr = 0
key sent to EnsureKeyExists = 0
MessageState::FinalizeHeaders
offset = 0
key found by offset!
new key = 0
GetNewMsgOutputStream
offset = 0
key sent to EnsureKeyExists = 0
row for the key created.
new key = 0
key sent into hdr = 0
key sent to EnsureKeyExists = 0
MessageState::FinalizeHeaders
offset = 0
key found by offset!
new key = 0
GetNewMsgOutputStream
offset = 0
key sent to EnsureKeyExists = 0
row for the key created.
new key = 0
key sent into hdr = 0
key sent to EnsureKeyExists = 0
MessageState::FinalizeHeaders
offset = 0
key found by offset!
new key = 0
++DOMWINDOW == 49 (0x16f52c34) [serial = 606] [outer = 0x15fd8e04]
++DOMWINDOW == 50 (0xd3b9334) [serial = 607] [outer = 0x15fd8e04]
--DOMWINDOW == 49 (0x1547020c) [serial = 604] [outer = 0xe3067d4] [url = mailbox:///TEST-MAIL-DIR/objdir-tb3/mozilla/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/CloseWindowOnDeleteA?number=2162]
--DOMWINDOW == 48 (0x15d93244) [serial = 603] [outer = 0x1552556c] [url = about:blank]
--DOMWINDOW == 47 (0x1552556c) [serial = 601] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 46 (0x1333986c) [serial = 596] [outer = 0xcf7ae54] [url = mailbox:///TEST-MAIL-DIR/objdir-tb3/mozilla/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/CloseWindowOnDeleteA?number=1842]
--DOMWINDOW == 45 (0xcf7ae54) [serial = 592] [outer = (nil)] [url = mailbox:///TEST-MAIL-DIR/objdir-tb3/mozilla/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/CloseWindowOnDeleteA?number=1842]
--DOMWINDOW == 44 (0x149bc83c) [serial = 595] [outer = 0x127f84ec] [url = about:blank]
--DOMWINDOW == 43 (0x127f84ec) [serial = 593] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 42 (0x15fa7744) [serial = 565] [outer = (nil)] [url = about:blank]
--DOCSHELL 0x188c37a0 == 18 [id = 100]
--DOCSHELL 0x44608c8 == 17 [id = 108]
--DOCSHELL 0x1394b848 == 16 [id = 105]
JavaScript strict warning: chrome://messenger/content/msgMail3PaneWindow.js, line 1040: reference to undefined property treeBoxObj.view
==21789== Warning: invalid file descriptor 1024 in syscall close()
==21789== Memcheck, a memory error detector
==21789== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==21789== Using Valgrind-3.9.0.SVN and LibVEX; rerun with -h for copyright info
==21789== Command: /usr/bin/pulseaudio --start --log-target=syslog
==21789==
==21789== Memcheck, a memory error detector
==21789== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==21789== Using Valgrind-3.9.0.SVN and LibVEX; rerun with -h for copyright info
==21789== Command: /usr/bin/pulseaudio --start --log-target=syslog
==21789==
==21789== Warning: invalid file descriptor 1044 in syscall close()
==21792==
==21792== HEAP SUMMARY:
==21792==     in use at exit: 30,915 bytes in 233 blocks
==21792==   total heap usage: 2,728 allocs, 2,495 frees, 720,829 bytes allocated
==21792==
==21792== LEAK SUMMARY:
==21792==    definitely lost: 0 bytes in 0 blocks
==21792==    indirectly lost: 0 bytes in 0 blocks
==21792==      possibly lost: 0 bytes in 0 blocks
==21792==    still reachable: 30,915 bytes in 233 blocks
==21792==	  suppressed: 0 bytes in 0 blocks
==21792== Reachable blocks (those to which a pointer was found) are not shown.
==21792== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==21792==
==21792== For counts of detected and suppressed errors, rerun with: -v
==21792== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 261 from 14)
==21791==
==21791== HEAP SUMMARY:
==21791==     in use at exit: 15 bytes in 2 blocks
==21791==   total heap usage: 116 allocs, 114 frees, 99,574 bytes allocated
==21791==
==21791== LEAK SUMMARY:
==21791==    definitely lost: 0 bytes in 0 blocks
==21791==    indirectly lost: 0 bytes in 0 blocks
==21791==      possibly lost: 0 bytes in 0 blocks
==21791==    still reachable: 15 bytes in 2 blocks
==21791==	  suppressed: 0 bytes in 0 blocks
==21791== Reachable blocks (those to which a pointer was found) are not shown.
==21791== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==21791==
==21791== For counts of detected and suppressed errors, rerun with: -v
==21791== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 167 from 8)
==21789==
==21789== HEAP SUMMARY:
==21789==     in use at exit: 56 bytes in 4 blocks
==21789==   total heap usage: 142 allocs, 138 frees, 146,760 bytes allocated
==21789==
==21789== LEAK SUMMARY:
==21789==    definitely lost: 0 bytes in 0 blocks
==21789==    indirectly lost: 0 bytes in 0 blocks
==21789==      possibly lost: 0 bytes in 0 blocks
==21789==    still reachable: 56 bytes in 4 blocks
==21789==	  suppressed: 0 bytes in 0 blocks
==21789== Reachable blocks (those to which a pointer was found) are not shown.
==21789== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==21789==
==21789== For counts of detected and suppressed errors, rerun with: -v
==21789== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 167 from 8)
TEST-PASS | /COMM-CENTRAL/comm-central/mail/test/mozmill/folder-display/test-close-window-on-delete.js | test-close-window-on-delete.js::test_close_multiple_windows_tabs_on_delete_from_3pane_window


TIA
Oh, so you run valgrind. That may slow things down and cause a different order of some events. But we should fix that too.

How do you get such verbose output from the test?
I only get this on the second test:

TEST-START | mail/test/mozmill/folder-display/test-close-window-on-delete.js | test_close_multiple_message_windows_on_delete_from_3pane_window
Step Pass: {"function": "Controller.keypress()"}
++DOMWINDOW == 73 (0xa497e0a8) [serial = 91] [outer = 0xb7290348]
WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///var/SSD/TB-hg/mail/themes/linux/mail/icon
WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///var/SSD/TB-hg/mail/themes/linux/mail/icon
++DOMWINDOW == 74 (0xa497fea8) [serial = 92] [outer = 0xb7290348]
++DOMWINDOW == 75 (0xa497edc8) [serial = 93] [outer = 0xb7290348]
GetDiskSpaceAvailable returned: 5563056128 bytes
TEST-PASS | mail/test/mozmill/folder-display/test-close-window-on-delete.js | test-close-window-on-delete.js::test_close_multiple_message_windows_on_delete_from_3pane_window
Attached patch patch - part 3 (obsolete) — Splinter Review
This should fix:
10 JavaScript strict warning: chrome://messenger/content/mailWindowOverlay.js, line 1122: reference to undefined property msgHdr.author

2 JavaScript strict warning: chrome://messenger/content/addressbook/abTrees.js, line 54: in strict mode code, functions may be declared only at top level or immediately within another function

2 JavaScript strict warning: chrome://editor/content/EdDialogCommon.js, line 947: in strict mode code, functions may be declared only at top level or immediately within another function

It attempts also this one, please check it:
 1 JavaScript error: chrome://messenger/content/addressbook/abContactsPanel.js, line 39: cards is null
Attachment #761088 - Flags: feedback?(ishikawa)
Thanks. Will check and report back tomorrow.

Regarding the verbose output of the second test,
> GetSearchResultsTable:StringToToken()=0x00000000, 
> searchFolderUri=mailbox://nobody@smart%20mailboxes/Trash
> GetSearchResultsTable: GetTableKind()=0x00000000, numTables=33, *table=0x13a8f284

these are output to figure out what is causing the following NS_ERROR_INVALID_POINTER caused in the following sequence
during the test of test_setup_virtual_folder_and_compact

TEST-START | /COMM-CENTRAL/comm-central/mail/test/mozmill/folder-display/test-savedsearch-reload-after-compact.js | test_setup_virtual_folder_and_compact

  [...]


GetHeadersFromSelection: GetMsgHdrForViewIndex failed, rv=0x80004003, viewIndex=0, index=0, numIndices=1
GetHeadersFromSelection failed. rv=0x80004003, numIndices=1
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004003: file /COMM-CENTRAL/comm-central/mailnews/base/src/nsMsgDBView.cpp, line 2425
[Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIMsgDBView.getMsgHdrsForSelection]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"	 location: "JS frame :: chrome://messenger/content/folderDisplay.js :: FolderDisplayWidget.prototype.selectedMessages :: line 1996"  data: no]

It seems that when the GetHeaderFromSelection somehow fails, the javascript code
blows up. 
Funny, though, the test as a whole seems to succeed. Hmm...

>offset = 10302
>key sent to EnsureKeyExists = 10302
>row for the key created.
>new key = 10302

these and friends are the addition for 4GB boundary checking about message key, etc. I probably added in one of my local patches to record these values just in
case something went wrong during my test of 4GB+ folder creation (by sending
many e-mails of small-to-medium size to a local account.

> ==21789== Warning: invalid file descriptor 1024 in syscall close()
> ==21789== Memcheck, a memory error detector
> ==21789== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
> ==21789== Using Valgrind-3.9.0.SVN and LibVEX; rerun with -h for copyright info
> ==21789== Command: /usr/bin/pulseaudio --start --log-target=syslo

These are of course from valgrind/memcheck. I have been meaning to ask someone what this "Warning: invalid file descriptor 1024 in syscall close()" is, but
failed to so up till now.

TIA
Comment on attachment 761088 [details] [diff] [review]
patch - part 3

:aceman,

Here is what I obtained after running "make mozmill" test of
comm-central thunderbird
WITHOUT invoking valgrind/memcheck.
(Sorry, it takes almost a full day PLUS more to run "make mozmill" with
valgrind/memcheck, so I took a short cut this time around.)



 ========================================
 JavaScript strict warning
 jquery.js and jquery-ui.js are ignored. 
 ========================================

     52 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 352: reference to undefined property aTabType.panelId
     52 JavaScript strict warning: chrome://global/content/bindings/browser.xml, line 223: reference to undefined property this.boxObject.QueryInterface(...)
.docShell
     12 JavaScript strict warning: chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 237: anonymous function does not always return a valu
e
      7 JavaScript strict warning: chrome://messenger/content/messengercompose/bigFileObserver.js, line 209: anonymous function does not always return a valu
e
      6 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 53: reference to undefined property this.treeBoxObject.view
      4 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 101: reference to undefined property this.onEditableArea
      2 JavaScript strict warning: chrome://messenger/content/msgMail3PaneWindow.js, line 1040: reference to undefined property treeBoxObj.view
      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1049: reference to undefined property this._underlyingFolders[0]
      1 JavaScript strict warning: chrome://global/content/bindings/textbox.xml, line 100: reference to undefined property this.inputField.QueryInterface(...
).editor

 ========================================

It certainly looks that it fixed some bugs.

TIA
Attachment #761088 - Flags: feedback?(ishikawa) → feedback+
Blocks: 867077
Attachment #761088 - Flags: review?(mconley)
Attachment #761088 - Flags: review?(mbanner)
Comment on attachment 761088 [details] [diff] [review]
patch - part 3

Review of attachment 761088 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/ui/dialogs/content/EdDialogCommon.js
@@ +943,5 @@
>      }
>      if (anchorList.length)
>      {
>        // case insensitive sort
> +      let compare = function compare(a, b)

Why not fix this one in the same way as abTrees.js?
It is longer so inlining it would be ugly :) But tell me what you wish.
Flags: needinfo?(mbanner)
this._children.sort(function nameSort(a, b) {
anchorList.sort(function compare(a, b) {

That's shorter isn't it?
Flags: needinfo?(mbanner)
I mean the compare function body is longer.
Do you want this:

anchorList.sort(function compare(a, b) {
  if (a.sortkey < b.sortkey) return -1;
  if (a.sortkey > b.sortkey) return 1;
    return 0;
});

Hm, doesn't look too bad...
Flags: needinfo?(mbanner)
I think that's fine, its no worse than what we have currently.
Flags: needinfo?(mbanner)
OK, done.
Attachment #761088 - Attachment is obsolete: true
Attachment #761088 - Flags: review?(mconley)
Attachment #761088 - Flags: review?(mbanner)
Attachment #767889 - Flags: review?(mbanner)
Attachment #767889 - Flags: review?(mbanner) → review+
Keywords: checkin-needed
Comment on attachment 767889 [details] [diff] [review]
patch - part 3 v2 [checkin: comment 55]

https://hg.mozilla.org/comm-central/rev/85ab07b6ae9a
Attachment #767889 - Attachment description: patch - part 3 v2 → patch - part 3 v2 [checkin: comment 55]
What do we focus on next? Can you rerun the tests and make new statistics?
Depends on: 903636
Flags: needinfo?(ishikawa)
(In reply to :aceman from comment #56)
> What do we focus on next? Can you rerun the tests and make new statistics?

Thank you for the reminder.
The warnings decrease after a long while, but then a flurry of new warning appear.
It is as if the number of warnings never comes down to zero !

I will place more comments later, but here is the list of
strict warnings recorded during the debug run of |make mozmill| of TB (released
early May.) The tree has a host of local patches, but
the strict warnings are independent of the patches from what I observed before and after the patch application.

 ========================================
 JavaScript strict warning
 jquery.js and jquery-ui.js are ignored.
 ========================================

     55 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 353: reference to undefined property aTabType.panelId
     55 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns
     12 JavaScript strict warning: chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]
      9 JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 214: this._accountType is null
      3 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 102: reference to undefined property this.onEditableArea
      3 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 62: reference to undefined property this.treeBoxObject.view
      2 JavaScript strict warning: file:///REF-OBJ-DIR/objdir-tb3/mozilla/dist/bin/components/storage-mozStorage.js, line 248: assignment to undeclared variable encType
      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1047: reference to undefined property this._underlyingFolders[0]
      1 JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 226: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMLocation.href]

 ========================================

The list certainly shrunk from the original list!
I would like to thank everybody who chimed in to fix the problems in this bugzilla entry
and other entries.

But the difficult ones seem to still remain. (To be honest,
because a few remaining ones are related to happen only when the ISP configuration 
information fails to be provided to TB under test, and I am not that concerned myself there since I am more worried about problems that an on-going user may experience. 
I know I should pay attention to the ISP setup issues, but there is only 
24 hours a day :-)

Note: There seem to be some test programs that fail due to
some mysterious timeout errors, and when these happen, I don't see some
strict errors :-(
So the list above is a typical list I get (this morning actually).

TIA
Flags: needinfo?(ishikawa)
Depends on: 1025340
Depends on: 1025547
Depends on: 1025550
Depends on: 1025551
(In reply to ISHIKAWA, Chiaki from comment #57)
>      55 JavaScript strict warning: chrome://messenger/content/tabmail.xml,
> line 353: reference to undefined property aTabType.panelId
This one is bug 903636.

>      55 JavaScript strict warning:
> chrome://global/content/bindings/tree.xml, line 50: reference to undefined
> property this.treeBoxObject.columns
>       3 JavaScript strict warning:
> chrome://global/content/bindings/tree.xml, line 62: reference to undefined
> property this.treeBoxObject.view
These are seen just by starting TB but I still don't know why they happen. They come from Toolkit. Not sure yet if TB uses the widgets wrongly, or there is a bug in the widgets in Toolkit. Hopefully somebody fixes this.

>      12 JavaScript strict warning:
> chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123:
> reference to undefined property def[1]
Done in bug 1025547.

>       9 JavaScript error:
> chrome://messenger/content/cloudfile/addAccountDialog.js, line 214:
> this._accountType is null
Bug 1025551.

>       3 JavaScript strict warning:
> chrome://messenger/content/nsContextMenu.js, line 102: reference to
> undefined property this.onEditableArea
No idea yet.

>       2 JavaScript strict warning:
> file:///REF-OBJ-DIR/objdir-tb3/mozilla/dist/bin/components/storage-
> mozStorage.js, line 248: assignment to undeclared variable encType
Done, bug 1025340.

>       1 JavaScript strict warning: resource:///modules/dbViewWrapper.js,
> line 1047: reference to undefined property this._underlyingFolders[0]
Bug 1025550.

>       1 JavaScript error:
> chrome://messenger/content/cloudfile/addAccountDialog.js, line 226:
> NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff
> (NS_ERROR_UNEXPECTED) [nsIDOMLocation.href]
Not sure yet. I do not see this when I run all mozmill. Maybe it got fixed since early May?
Dear aceman,

Thank you for taking care of the remaining bugs.
I have comments on a couple of errors:


 
> >      55 JavaScript strict warning:
> > chrome://global/content/bindings/tree.xml, line 50: reference to undefined
> > property this.treeBoxObject.columns
> >       3 JavaScript strict warning:
> > chrome://global/content/bindings/tree.xml, line 62: reference to undefined
> > property this.treeBoxObject.view
> These are seen just by starting TB but I still don't know why they happen.
> They come from Toolkit. Not sure yet if TB uses the widgets wrongly, or
> there is a bug in the widgets in Toolkit. Hopefully somebody fixes this.

I will keep my eyes on this.

> >       3 JavaScript strict warning:
> > chrome://messenger/content/nsContextMenu.js, line 102: reference to
> > undefined property this.onEditableArea
> No idea yet.


This seems to be triggered when a message is edited during a test.
(Not sure why this happens in the mozmill test. Maybe some timing issues?)
 
 
> >       1 JavaScript error:
> > chrome://messenger/content/cloudfile/addAccountDialog.js, line 226:
> > NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff
> > (NS_ERROR_UNEXPECTED) [nsIDOMLocation.href]
> Not sure yet. I do not see this when I run all mozmill. Maybe it got fixed
> since early May?

I think this one is encountered in an error path when the
account creation using the ISP database fails somehow.
(On the particular PC which I used to gather the errors, 
this ISP database never seems to succeed, and it times out. And so,
presumably, some variables and data are not properly initialized and then
referenced causing the error(s). Pure guess, but I am afraid that error return path
is not tested quite well.

TIA

PS: I could compile C-C TB using gcc 4.9 after refreshing the source, and so will run
the test again to see if some bugs are indeed gone, and if new ones have crept in under our radar :-)
Here is the latest log output.

 ========================================
 JavaScript strict warning
 jquery.js and jquery-ui.js are ignored. 
 ========================================

     51 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 353: reference to undefined property aTabType.panelId

     51 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns

     12 JavaScript strict warning: chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]

     11 JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 214: this._accountType is null

      3 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 102: reference to undefined property this.onEditableArea

      3 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 62: reference to undefined property this.treeBoxObject.view

      2 JavaScript error: chrome://messenger/content/folderDisplay.js, line 2460: this.view.dbView is null

      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1047: reference to undefined property this._underlyingFolders[0]


      CI comment:
      I have never seen the following before. So obviously it was introduced within last two months.

      1 JavaScript strict warning: chrome://messenger/content/specialTabs.js, line 1286: reference to undefined property notificationBox.PRIORITY_MEDIUM_HIGH


      This also seems to be a relative new comer. I will investigate more.
      1 JavaScript strict warning: chrome://messenger/content/addressbook/abTrees.js, line 263: reference to undefined property aParent.URI


      CI comment: The following error occurs when the following test is executed.
TEST-START | /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/cloudfile/test-cloudfile-add-account-dialog.js | test_lone_provider_auto_selected


      1 JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 226: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMLocation.href]



And I also noticed some *NEW* JS errors, too:
The three uncaught exceptions are disconcerting.
I have no idea how to debug them.
Where should I set a breakpoint for example???
Although, as JavaScript interpreter thinks it captures an error
thrown to it and print the error top-level, 
we need to figure out where the error, which possibly occurs
on C++ side even (via XPCOM magic), is triggered. 

Quote from the log:
========================================
System JS: errors and warnings.
========================================

mutating the prototype warning

     35 System JS : WARNING file:///REF-OBJ-DIR/objdir-tb3/mozilla/dist/bin/components/steelApplication.js:786 - mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create



Other System JS errors:

First the summary, and then the individual lines.: FF mochitest-plain produced so many of them.
We should check each one of them to see if these are fatal or low-priority types.

      CI comment: the first one from nsIMsgMailNewsUrl.server seems to be rather new (they appeared around the
      beginning or middle of April, I think.)

      4 System JS : ERROR resource:///modules/activity/alertHook.js:48 - NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgMailNewsUrl.server]


      1 System JS : WARNING chrome://messenger/content/accountcreation/fetchConfig.js:112 - function fetchConfigFromISP does not always return a value


      CI comment: The following timeout was recorded when a big file is sent by using
      a file storage service: but it seems to have timed out on my PC.

      1 System JS : ERROR resource://mozmill/modules/frame.js -> file:///REF-COMM-CENTRAL/comm-central/mail/test/mozmill/shared-modules/test-window-helpers.js:363 - Error: Timeout while waiting for modal dialog.


      1 System JS : ERROR file:///REF-OBJ-DIR/objdir-tb3/mozilla/dist/bin/components/nsHightail.js:704 - SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data


      CI comment: the following error seems to have occurred
      in the test:
      TEST-START | /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/folder-display/test-message-window.js | test_next_unread

      1 System JS : ERROR chrome://messenger/content/messageWindow.js:253 - TypeError: this.folderDisplay.view.dbView is null

      CI comment: the following error occurs during testing of

TEST-START | /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/newmailaccount/test-newmailaccount.js | test_restored_ap_tab_works


and is is preceded by:

[16378] WARNING: There is no observer for "invalidformsubmit". One should be implemented!: file /REF-COMM-CENTRAL/comm-central/mozilla/content/html/content/src/HTMLFormElement.cpp, line 1904

Step Pass: {"function": "controller.click()"}

++DOMWINDOW == 39 (0x5f21300) [pid = 16378] [serial = 48] [outer = 0x3865a20]

System JS : ERROR (null):0 - uncaught exception: This login already exists.


      1 System JS : ERROR (null):0 - uncaught exception: This login already exists.




      CI comment: The following error occurs during
TEST-START | /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/attachment/test-attachment-menus.js | test_multiple_attachments_all_deleted



      1 System JS : ERROR (null):0 - uncaught exception: 2153578497




      CI comment: The following seems to occur when |make mozmill| test suite
      begins to run tests under |notification| directory
      >   INFO | (runtestlist.py) | Running directory: notification

      after previous tests under |newmailaccount| fininshed.
      >INFO | (runtestlist.py) | newmailaccount: 29 passed, 0 failed

      No individual tests seem to have been invoked when this error occurs!

      1 System JS : ERROR (null):0 - uncaught exception: 2147500037


Individual lines

	   [...]

TIA
Depends on: 1026629
(In reply to ISHIKAWA, Chiaki from comment #59)
> PS: I could compile C-C TB using gcc 4.9 after refreshing the source, and so
> will run
> the test again to see if some bugs are indeed gone, and if new ones have
> crept in under our radar :-)
Version of gcc should have no effect on these errors, as they are in Javascript.

(In reply to ISHIKAWA, Chiaki from comment #60)
> Here is the latest log output.
>       2 JavaScript error: chrome://messenger/content/folderDisplay.js, line
> 2460: this.view.dbView is null
> 
>       1 JavaScript strict warning: resource:///modules/dbViewWrapper.js,
> line 1047: reference to undefined property this._underlyingFolders[0]
I think these 2 may be related.

>       CI comment:
>       I have never seen the following before. So obviously it was introduced
> within last two months.
> 
>       1 JavaScript strict warning:
> chrome://messenger/content/specialTabs.js, line 1286: reference to undefined
> property notificationBox.PRIORITY_MEDIUM_HIGH
Real, bug 1026629.

> And I also noticed some *NEW* JS errors, too:
> The three uncaught exceptions are disconcerting.
> I have no idea how to debug them.
> Where should I set a breakpoint for example???
> Although, as JavaScript interpreter thinks it captures an error
> thrown to it and print the error top-level, 
> we need to figure out where the error, which possibly occurs
> on C++ side even (via XPCOM magic), is triggered. 
We should really make there JS errors cause the tests to fail. Then the authors of the changes would spot them immediately in the test runs. But that probably depends on the authors of mozmill.

> mutating the prototype warning
> 
>      35 System JS : WARNING
> file:///REF-OBJ-DIR/objdir-tb3/mozilla/dist/bin/components/steelApplication.
> js:786 - mutating the [[Prototype]] of an object will cause your code to run
> very slowly; instead create the object with the correct initial
> [[Prototype]] value using Object.create
I'm on this, bug 1025316.

> First the summary, and then the individual lines.: FF mochitest-plain
> produced so many of them.
> We should check each one of them to see if these are fatal or low-priority
> types.
> 
>       CI comment: the first one from nsIMsgMailNewsUrl.server seems to be
> rather new (they appeared around the
>       beginning or middle of April, I think.)
> 
>       4 System JS : ERROR resource:///modules/activity/alertHook.js:48 -
> NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff
> (NS_ERROR_UNEXPECTED) [nsIMsgMailNewsUrl.server]
Activity manager shows errors even while using TB. I'd consider this low priority.

>       1 System JS : WARNING
> chrome://messenger/content/accountcreation/fetchConfig.js:112 - function
> fetchConfigFromISP does not always return a value
I do not see this in my log. Please file it as separate bug and CC BenB there. Maybe we could make the function (and it's siblings) return null instead of plain 'return'.

>       CI comment: The following timeout was recorded when a big file is sent
> by using
>       a file storage service: but it seems to have timed out on my PC.
> 
>       1 System JS : ERROR resource://mozmill/modules/frame.js ->
> file:///REF-COMM-CENTRAL/comm-central/mail/test/mozmill/shared-modules/test-
> window-helpers.js:363 - Error: Timeout while waiting for modal dialog.
> 
>       1 System JS : ERROR
> file:///REF-OBJ-DIR/objdir-tb3/mozilla/dist/bin/components/nsHightail.js:704
> - SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the
> JSON data
Yes, I do see these. Please file them.

>       CI comment: the following error occurs during testing of
> TEST-START |
> /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/newmailaccount/test-
> newmailaccount.js | test_restored_ap_tab_works
> 
> 
> and is is preceded by:
> 
> [16378] WARNING: There is no observer for "invalidformsubmit". One should be
> implemented!: file
> /REF-COMM-CENTRAL/comm-central/mozilla/content/html/content/src/
> HTMLFormElement.cpp, line 1904
> 
> Step Pass: {"function": "controller.click()"}
> ++DOMWINDOW == 39 (0x5f21300) [pid = 16378] [serial = 48] [outer = 0x3865a20]
> System JS : ERROR (null):0 - uncaught exception: This login already exists.
>       1 System JS : ERROR (null):0 - uncaught exception: This login already
> exists.
These seem to be related in the same test. Please file it as a new bug an CC mconley.

>       CI comment: The following error occurs during
> TEST-START |
> /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/attachment/test-attachment-
> menus.js | test_multiple_attachments_all_deleted
>       1 System JS : ERROR (null):0 - uncaught exception: 2153578497
Yes, I do see it. No idea about it.
(In reply to :aceman from comment #61)
> (In reply to ISHIKAWA, Chiaki from comment #59)
> > PS: I could compile C-C TB using gcc 4.9 after refreshing the source, and so
> > will run
> > the test again to see if some bugs are indeed gone, and if new ones have
> > crept in under our radar :-)
> Version of gcc should have no effect on these errors, as they are in
> Javascript.
> 

Dear aceman,

Thank you for your comment. You are absolutely right regarding the JavaScript errors
should  be immune from compiler version changes (as far as the compiler generates correct code).

I will submit new bugzilla entries as you suggest, and thank you again for your
comement and analysis of my bugzilla reports.

TIA
Depends on: 1029793
Depends on: 1029806
Depends on: 1029820
Here is the summary latest javascript strict errors/warnings 
from the "make mozmill" run of DEBUG version of TB.

It looks that a whole new set of bugs/warnings have crept in the last 3 months or so.
I refreshed my source yesterday (Sept 27th).
(I have taken care of easy deprecated literal octal string in 4 files before
I ran this test.)

--- begin quote

It is either that the errors crept in, or
JavaScript interpreter now has stricter checking, or
BOTH.

 ========================================
 JavaScript strict warning
 jquery.js and jquery-ui.js are ignored.
 ========================================

   3005 JavaScript strict warning: , line 0: TypeError: setting a
   property that has only a getter

The above is new and is discussed in 
Bug 1067942 - "TypeError: setting a property that has only a getter"
without mentioning file and property name ]

    121 JavaScript error: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/nsPhishingProtectionApplication.js, line 156: TypeError: listManager.setUpdateUrl is not a function

The above is new and is discussed in
Bug 1032767 - Removed setUpdateUrl function used [Error: listManager.setUpdateUrl is not a function]

     55 JavaScript strict warning: chrome://messenger/content/tabmail.xml, line 353: ReferenceError: reference to undefined property aTabType.panelId
(already known)

     55 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: ReferenceError: reference to undefined property this.treeBoxObject.columns
(already known)

     36 JavaScript warning: resource://gre/modules/Preferences.jsm, line 381: mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create
(already known?)

     36 JavaScript strict warning: resource://jsbridge/modules/server.js, line 93: ReferenceError: assignment to undeclared variable globalRegistry

(this and the following three warnings are new.)

     36 JavaScript strict warning: resource://jsbridge/modules/server.js, line 329: ReferenceError: assignment to undeclared variable session

     36 JavaScript strict warning: resource://jsbridge/modules/server.js, line 216: ReferenceError: reference to undefined property Ci.nsIConverterOutputStream.DEFAULT_REPLACEMENT_CHARACTER

     36 JavaScript strict warning: resource://jsbridge/modules/server.js, line 204: ReferenceError: assignment to undeclared variable backstage

     36 JavaScript strict warning: resource:///modules/distribution.js, line 5: ReferenceError: assignment to undeclared variable EXPORTED_SYMBOLS

     35 JavaScript warning: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/steelApplication.js, line 786: mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create
(already known?)

Most of the rest seems to be new to me (or stricter checking?)

     35 JavaScript strict warning: resource://mozmill/stdlib/securable-module.js, line 355: ReferenceError: assignment to undeclared variable name
     35 JavaScript strict warning: resource://mozmill/stdlib/os.js, line 82: ReferenceError: assignment to undeclared variable mPlatform
     35 JavaScript strict warning: resource://mozmill/stdlib/os.js, line 65: ReferenceError: assignment to undeclared variable p
     35 JavaScript strict warning: resource://mozmill/stdlib/httpd.js, line 2795: SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function
     35 JavaScript strict warning: resource://mozmill/stdlib/arrays.js, line 41: ReferenceError: assignment to undeclared variable i
     35 JavaScript strict warning: resource://mozmill/modules/utils.js, line 131: TypeError: variable appention redeclares argument
     35 JavaScript strict warning: resource://mozmill/modules/mozmill.js, line 92: ReferenceError: assignment to undeclared variable applicationDictionary
     35 JavaScript strict warning: resource://mozmill/modules/mozmill.js, line 190: ReferenceError: assignment to undeclared variable MozMillAsyncTest
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 76: ReferenceError: assignment to undeclared variable mozmill
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 76: ReferenceError: assignment to undeclared variable modules
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 76: ReferenceError: assignment to undeclared variable elementslib
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 70: ReferenceError: assignment to undeclared variable arrayRemove
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 564: ReferenceError: assignment to undeclared variable thread
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 436: ReferenceError: reference to undefined property test_module[i]._mozmillasynctest
     35 JavaScript strict warning: resource://mozmill/modules/frame.js, line 177: ReferenceError: assignment to undeclared variable timers
     35 JavaScript strict warning: resource://mozmill/modules/elementslib.js, line 444: TypeError: variable exp redeclares argument
     35 JavaScript strict warning: resource://mozmill/modules/elementslib.js, line 261: SyntaxError: test for equality (==) mistyped as assignment (=)?
     35 JavaScript strict warning: resource://mozmill/modules/controller.js, line 1341: ReferenceError: assignment to undeclared variable MozMillAsyncTest
     35 JavaScript strict warning: resource://mozmill/modules/controller.js, line 1336: ReferenceError: assignment to undeclared variable controllerAdditions
     35 JavaScript strict warning: resource://mozmill/modules/controller.js, line 100: ReferenceError: assignment to undeclared variable waitForEvents
     35 JavaScript strict warning: resource://jsbridge/modules/server.js, line 134: ReferenceError: assignment to undeclared variable i
     35 JavaScript strict warning: resource://jsbridge/modules/events.js, line 6: ReferenceError: assignment to undeclared variable backchannel
     35 JavaScript strict warning: resource://gre/modules/DownloadUtils.jsm, line 79: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: resource:///modules/msgDBCacheManager.js, line 131: SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function
     35 JavaScript strict warning: resource:///modules/FeedUtils.jsm, line 455: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: resource:///modules/FeedUtils.jsm, line 299: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: resource:///modules/FeedUtils.jsm, line 298: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: resource:///modules/FeedUtils.jsm, line 273: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: resource:///modules/FeedUtils.jsm, line 272: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: resource:///modules/FeedUtils.jsm, line 1402: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     35 JavaScript strict warning: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/imContacts.js, line 101: SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function
     35 JavaScript strict warning: chrome://messenger-newsblog/content/Feed.js, line 399: SyntaxError: applying the 'delete' operator to an unqualified name is deprecated
     25 JavaScript strict warning: resource://mozmill/modules/utils.js, line 92: ReferenceError: assignment to undeclared variable w
     11 JavaScript error: chrome://messenger/content/folderDisplay.js, line 2460: TypeError: this.view.dbView is null
      9 JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 214: TypeError: this._accountType is null
      7 JavaScript strict warning: resource://gre/modules/FormHistory.jsm, line 639: ReferenceError: reference to undefined property change.guid
      6 JavaScript strict warning: resource:///modules/attachmentChecker.js, line 94: ReferenceError: assignment to undeclared variable onmessage
      6 JavaScript strict warning: resource:///modules/activity/moveCopy.js, line 129: ReferenceError: reference to undefined property this.lastMessage.sourceFolder
      5 JavaScript strict warning: resource://mozmill/modules/controller.js, line 312: ReferenceError: reference to undefined property node.tagName
      4 JavaScript strict warning: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/nsAbAutoCompleteSearch.js, line 364: SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function
      4 JavaScript error: resource:///modules/activity/alertHook.js, line 48: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgMailNewsUrl.server]
      4 JavaScript error: chrome://messenger/content/nsContextMenu.js, line 413: TypeError: aNode is null
      4 JavaScript error: chrome://messenger/content/mailContextMenus.js, line 50: TypeError: gContextMenu is null
      4 JavaScript error: chrome://messenger/content/folderPane.js, line 2076: TypeError: null has no properties
      3 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 62: ReferenceError: reference to undefined property this.treeBoxObject.view
      2 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 103: ReferenceError: reference to undefined property this.onEditableArea
      2 JavaScript error: resource://mozmill/modules/frame.js -> file:///REF-COMM-CENTRAL/comm-central/mail/test/mozmill/shared-modules/test-window-helpers.js, line 363: Error: Timeout while waiting for modal dialog.
      1 JavaScript strict warning: resource://mozmill/modules/frame.js -> file:///REF-COMM-CENTRAL/comm-central/mail/test/mozmill/tabmail/test-tabmail-dragndrop.js, line 328: TypeError: "recentlyClosedTabs" is read-only
      1 JavaScript strict warning: resource:///modules/jsmime.jsm -> resource:///modules/jsmime/jsmime.js, line 769: ReferenceError: reference to undefined property token.toString(...)[0]
      1 JavaScript strict warning: resource:///modules/dbViewWrapper.js, line 1047: ReferenceError: reference to undefined property this._underlyingFolders[0]
      1 JavaScript strict warning: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/nsSearchService.js, line 3094: ReferenceError: reference to undefined property cache.directories
      1 JavaScript strict warning: chrome://messenger/content/jsTreeView.js, line 44: ReferenceError: reference to undefined property aCol.id
      1 JavaScript strict warning: chrome://messenger/content/addressbook/abTrees.js, line 263: ReferenceError: reference to undefined property aParent.URI
      1 JavaScript error: resource:///modules/dbViewWrapper.js, line 1047: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIMsgDBView.open]
      1 JavaScript error: chrome://messenger/content/messageWindow.js, line 253: TypeError: this.folderDisplay.view.dbView is null
      1 JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 226: NS_ERROR_UNEXPECTED:
      1 JavaScript error: chrome://messenger/content/chat/imStatusSelector.js, line 36: TypeError: document.getElementById(...) is null
      1 JavaScript error: , line 0: uncaught exception: 2153578497
      1 JavaScript error: , line 0: uncaught exception: 2147500037

The last two are disturbing, but I have no clue where (in what .js
file)  they came from.

      1 JavaScript error: , line 0: uncaught exception: 2153578497

The above appeared during the run of the following test.

TEST-START | /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/attachment/test-attachment-menus.js | test_multiple_attachments_all_deleted



      1 JavaScript error: , line 0: uncaught exception: 2147500037

The above appeared during the initialization phase of the tests under
the following directory:

INFO | (runtestlist.py) | Running directory: notification



--- end quote
Depends on: 1073912
literal octal string issues are these:


    105 JavaScript strict warning: resource://mozmill/stdlib/httpd.js, line 2768: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: resource://mozmill/stdlib/httpd.js, line 2674: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: resource://mozmill/stdlib/httpd.js, line 2139: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: resource://mozmill/modules/utils.js, line 258: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: resource://mozmill/modules/utils.js, line 221: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: resource://mozmill/modules/utils.js, line 137: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: resource://mozmill/modules/utils.js, line 135: SyntaxError: octal literals and octal escape sequences are deprecated

    105 JavaScript strict warning: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/XULStore.js, line 147: SyntaxError: octal literals and octal escape sequences are deprecated

      3 JavaScript strict warning: resource://gre/modules/DownloadPaths.jsm, line 61: SyntaxError: octal literals and octal escape sequences are deprecated

Filed bug 1073956, bug 1073955, bug 1073591, and bug 1073912.
Depends on: 1073951, 1073955, 1073956
> Filed bug 1073956, bug 1073955, bug 1073591, and bug 1073912.
                                ****************
The third one ought to read bug 1073951.
>36 JavaScript strict warning: resource://jsbridge/modules/server.js, line 216: ReferenceError: reference to undefined property Ci.nsIConverterOutputStream.DEFAULT_REPLACEMENT_CHARACTER

Searching through MXR, I think now 
DEFAULT_REPLACEMENT_CHARACTER is only available in
Ci.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER
and not for OutputStream. Must be related to the
removal and fine tuning of encoding routines in M-C and corresponding C-C changes lately.
Yes, we are badly broken by some changes in the Gecko core, seem like increased strictness of JS engine.

I think we should focus on files that we actually can fix (e.g. not mozmill components). E.g. filing the Feed*s.jsm problems would be useful.

Also, I have already fixed some of the problems, they just haven't been checked in...
Depends on: 1070614
OK, the Feeds* stuff is already filed in bug 1070525 :)
Depends on: 1070525
(In reply to ISHIKAWA, Chiaki from comment #63)


>       1 JavaScript error: , line 0: uncaught exception: 2153578497
>       1 JavaScript error: , line 0: uncaught exception: 2147500037
> 
> The last two are disturbing, but I have no clue where (in what .js
> file)  they came from.
> 
>       1 JavaScript error: , line 0: uncaught exception: 2153578497
> 
> The above appeared during the run of the following test.
> 
> TEST-START |
> /REF-COMM-CENTRAL/comm-central/mail/test/mozmill/attachment/test-attachment-
> menus.js | test_multiple_attachments_all_deleted
> 
> 
> 
>       1 JavaScript error: , line 0: uncaught exception: 2147500037
> 
> The above appeared during the initialization phase of the tests under
> the following directory:


These strange uncaught exception seems to be caused by recent change
in m-c according to the comment in loghelper.js

http://mxr.mozilla.org/comm-central/source/mailnews/test/resources/logHelper.js#69

69           // Unfortunately changes to mozilla-central are throwing lots
70           // of console errors during testing, so disable (we hope temporarily)
71           // failing on XPCOM console errors (see bug 1014350).
72           // An XPCOM error aMessage looks like this:
73           //   [JavaScript Error: "uncaught exception: 2147500037"]
74           // Capture the number, and allow known XPCOM results.

The second exception number in my quote matchtes the number in the comment (line 73) !

So hopefully this can be ignored for now.
Assignee: acelists → nobody
Status: ASSIGNED → NEW
Depends on: 1032767, 1025316
Depends on: 1074814
Depends on: 1076677
Depends on: 1076683
      2 JavaScript strict warning: chrome://messenger/content/nsContextMenu.js, line 103: ReferenceError: reference to undefined property this.onEditableArea

This one should be fixed via bug 912116.

You should also decide where to draw the line and when to close this bug. We should not drag it undefinitely.
Depends on: 912116
(In reply to :aceman from comment #70)
>       2 JavaScript strict warning:
> chrome://messenger/content/nsContextMenu.js, line 103: ReferenceError:
> reference to undefined property this.onEditableArea
> 
> This one should be fixed via bug 912116.
> 
> You should also decide where to draw the line and when to close this bug. We
> should not drag it undefinitely.

Maybe I should start "(2015 version)  JavaScript strict warning seen during "make mozmill" run of TB (DEBUG BUILD)" to start afresh each year (but making sure to
carry on all the dependent entries intact)?

Actually, I am submitting a buzilla for the issues with mozmill code. Yes, mozmill code
began spewing many warning lines since JS interpreter implemented stricter check last summer.

TIa
What's the status of this now?
I can find any "strict warning" in the latest mozmill logs for debug and opt builds.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Sorry I could not respond earlier. I have had a serious hardware issue starting in August (UPS died, a hard disk died, an external hard disk enclosure died (backplane to connect hard disks seemed to have fried?), the backup computer's mother board died, etc.
(Maybe I should check the quality of power to my house....)

Anyway, I have salvaged the hardware and could get the minimum of disk files to continue compiling TB and updating the TB source tree to the latest. 
I will check on my own and report. Thank you for the patience.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: