951520 - Error: this.folderDisplay.treeSelection is null File: chrome://messenger/content/messageWindow.js Line: 260

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[ISHIKAWA, Chiaki]

I got the error in the title,

Error: this.folderDisplay.treeSelection is null

in Error console of Thunderbird while I am using the current release (24.0.2) under Debian GNU/Linux 32 bits.

I got this error when I opened an attached email messages (I mean an e-mail message is attached to a mail message and was sent.) I opened the attached message 
in a stand alone window. The error(s) seemed to be related to the opening and closing of the message thus opened, but there is no clear reproducible manner.
I noticed the problem last evening and this morning, and co-related what I was doing to the timestamp of errors, and figured that the error seemed easily triggered by opening such attached messages (actually I got three attached messages in one e-mail) and closing them. But the error seemed to be generated 
almost asynchronously AFTER the messages are closed.

Anyway, looking at the code where the error occurs,
chrome://messenger/content/messageWindow.js (inside omni.ja)


  onMessagesRemoved:
      function StandaloneMessageDisplayWidget_onMessagesRemoved() {
==>   if (this.folderDisplay.treeSelection.count == 0 &&
        Services.prefs.getBoolPref("mail.close_message_window.on_delete")) {
      window.close();
      return true;
    }
    return false;
  },

and see that a JS file explicitly checks for the treeSelection validity when 
a message is removed as in
http://mxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#1156


1124   /**
1125    * Messages (that may have been displayed) have been removed; this may impact
1126    * our message selection. We might know it's coming; if we do then
1127    * this._nextViewIndexAfterDelete should know what view index to select next.
1128    * For the imap mark-as-deleted we won't know beforehand.
1129    */
1130   onMessagesRemoved: function FolderDisplayWidget_onMessagesRemoved() {
1131     FolderDisplayListenerManager._fireListeners("onMessagesRemoved",
1132                                                 [this]);
1133 
1134     if (this.messageDisplay.onMessagesRemoved())
1135       return;
1136 
1137     // - we saw this coming
1138     let rowCount = this.view.dbView.rowCount;
1139     if (!this._massMoveActive && (this._nextViewIndexAfterDelete != null)) {
1140       // adjust the index if it is after the last row...
1141       // (this can happen if the "mail.delete_matches_sort_order" pref is not
1142       //  set and the message is the last message in the view.)
1143       if (this._nextViewIndexAfterDelete >= rowCount)
1144         this._nextViewIndexAfterDelete = rowCount - 1;
1145       // just select the index and get on with our lives
1146       this.selectViewIndex(this._nextViewIndexAfterDelete);
1147       this._nextViewIndexAfterDelete = null;
1148       return;
1149     }
1150 
1151     // - we didn't see it coming
1152 
1153     // A deletion happened to our folder.
1154     let treeSelection = this.treeSelection;
1155     // we can't fix the selection if we have no selection
1156     if (!treeSelection)
1157       return;
1158 

we probably check for !treeSelection as in line 1156 above and return doing nothing here also.

I can create a patch if this is acceptable.

TIA

Comment 1

[ISHIKAWA, Chiaki]

I am more convinced that my idea above seems to be acceptable now that
I searched for 
and found two instances:
this.folderDisplay.treeSelection.count == 0

Found 2 matching lines in 2 files
/mail/base/content/messageWindow.js (View Hg log or Hg annotations)
    line 263 -- if (this.folderDisplay.treeSelection.count == 0 &&
/mail/base/content/messageDisplay.js (View Hg log or Hg annotations)
    line 539 -- if (this.folderDisplay.treeSelection.count == 0 &&

The one in messageDisplay.js seems to be the code in question and the other
is
http://mxr.mozilla.org/comm-central/source/mail/base/content/messageWindow.js#263
and note the check on line 261.

259   onMessagesRemoved:
260       function StandaloneMessageDisplayWidget_onMessagesRemoved() {
261     if (!this.folderDisplay.treeSelection)
262       return true;
263     if (this.folderDisplay.treeSelection.count == 0 &&
264         Services.prefs.getBoolPref("mail.close_message_window.on_delete")) {
265       window.close();
266       return true;
267     }
268     return false;
269   },

So somehow the checking !this.folderDisplay.treeSelection was missed here.

TIA

Comment 2

[ISHIKAWA, Chiaki]

Attachment: check-treeSelection.patch

A proposed patch.

TIA

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 8349218 [details] [diff] [review]
check-treeSelection.patch

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

I'll steal this review

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 8349218 [details] [diff] [review]
check-treeSelection.patch

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

Can't reproduce, but it looks reasonable. r=mkmelin

Comment 5

[ISHIKAWA, Chiaki]

(In reply to Magnus Melin from comment #4)
> Comment on attachment 8349218 [details] [diff] [review]
> check-treeSelection.patch
> 
> Review of attachment 8349218 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can't reproduce, but it looks reasonable. r=mkmelin

Thank you.

Reproducing the issue locally was difficult even. The error was logged AFTER I 
displayed the attached e-mail messages, deleted the standalone window, and went to a meeting. When I came back, THEN I noticed the error message(s) that cropped 20 minutes after I left the desk (!)
So it *IS* reproducible, but not easy.
I suspect some kind of left-over callback triggered by regularly recurring event or something that invoked this processing associated with the deletion of a message, but not
exactly sure what it is. (Or can it be a spam filter that deletes spams in the 
incoming messages downloaded regularly. Sound like it. There is no selection of message then.)

Maybe if we can dump the JS stack in detail when the problem is noticed, it would have been easy to spot the cause. Maybe the US interpreter should be told to dump the JS stack in
meaningful manner by a user selectable flag or something. I know there is a gdb method, but that sounds an awful overhead for a casual tester like mine :-(
This could be a Request-for-Enhancement.

I put the checkin-needed keyword.

Comment 6

[ISHIKAWA, Chiaki]

Correction.
[OLD]
Maybe the US interpreter should be told to dump the JS stack in
meaningful manner by a user selectable flag or something. I know there is a gdb method, but that sounds an awful overhead for a casual tester like mine :-(

[NEW]
Maybe the *JS* interpreter should be told to dump the JS stack in
meaningful manner by a user selectable flag or something. I know there is a gdb method, but that sounds an awful overhead for a casual tester like *me* :-(

The default font sizes for the web browser could  be made larger for people with 
bad eyesight :-)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/e304453cd7c7