Closed Bug 896910 Opened 6 years ago Closed 6 years ago

TEST-UNEXPECTED-FAIL | ../../../../mailnews/resources/logHelper.js | Error console says [stackFrame aFolder is not defined]


(MailNews Core :: Testing Infrastructure, defect)

Not set


(Not tracked)



(Reporter: aceman, Assigned: aceman)




(1 file)

Can be seen in

TEST-INFO | (xpcshell/head.js) | test _async_driver pending (2)
System JS : ERROR resource://gre/modules/dbViewWrapper.js:269
                     ReferenceError: aFolder is not defined

The relevant code is:

247   _folderMoveHelper: function(aOldFolder, aNewFolder) {
248     let oldURI = aOldFolder.URI;
249     let newURI = aNewFolder.URI;
250     // fix up our listener tables.
251     if (oldURI in this._pendingFolderUriToViewWrapperLists) {
252       this._pendingFolderUriToViewWrapperLists[newURI] =
253         this._pendingFolderUriToViewWrapperLists[oldURI];
254       delete this._pendingFolderUriToViewWrapperLists[oldURI];
255     }
256     if (oldURI in this._interestedWrappers) {
257       this._interestedWrappers[newURI] =
258         this._interestedWrappers[oldURI];
259       delete this._interestedWrappers[oldURI];
260     }
262     let wrappers = this._interestedWrappers[newURI];
263     if (wrappers) {
264       // clone the list to avoid confusing mutation by listeners
265       for each (let [, wrapper] in Iterator(wrappers.concat())) {
266         wrapper._folderMoved(aOldFolder, aNewFolder);
267       }
268       // if the folder is deleted, it's not going to get deleted again.
269       delete this._interestedWrappers[aFolder.URI];
270     }

Looks like line 269 should use one of oldURI or newURI, but oldURI is already deleted at like 259... Maybe this is just a leftover line that should be removed altogether.
Blocks: 896738
Asuth, you seem to be the author of that function, can you please look at what the correct logic should be?
Flags: needinfo?(bugmail)
I think the situation is that the 'aFolder' logic is derived from the folderDeleted code elsewhere in the file.  I think aFolder in that case wanted to be aOldFolder, but as you point out, we already cleared that entry out of _interestedWrappers.  (I probably was migrating the logic up to the above case and then got distracted and forgot to remove the lower code.)  Yes, I think the line should be removed, as per your analysis.

If you're a valid reviewer for the code and want to delegate review to me for that change, r=asuth.  Otherwise I think you need to find a reviewer :)
Flags: needinfo?(bugmail)
Attached patch patchSplinter Review
Thanks. I am not a reviewer but I think standard8 will accept take your opinion :)
Assignee: nobody → acelists
Attachment #781963 - Flags: review?(mbanner)
Attachment #781963 - Flags: review?(mbanner) → review+
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.