Closed
Bug 896910
Opened 8 years ago
Closed 8 years ago
TEST-UNEXPECTED-FAIL | ../../../../mailnews/resources/logHelper.js | Error console says [stackFrame aFolder is not defined]
Categories
(MailNews Core :: Testing Infrastructure, defect)
MailNews Core
Testing Infrastructure
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: aceman, Assigned: aceman)
References
Details
Attachments
(1 file)
1.12 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Can be seen in https://tbpl.mozilla.org/php/getParsedLog.php?id=25582134 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 } 261 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.
Asuth, you seem to be the author of that function, can you please look at what the correct logic should be?
Flags: needinfo?(bugmail)
Comment 2•8 years ago
|
||
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)
Thanks. I am not a reviewer but I think standard8 will accept take your opinion :)
Updated•8 years ago
|
Attachment #781963 -
Flags: review?(mbanner) → review+
Comment 4•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/bc77857518f2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•