Currently, when run stand-alone, the event data is expected to be null, whereas when run in combination with the other test files, event data is 0.

sounds like worksforme with new test organization

Comment on attachment 364076 [details] [diff] [review] patch General nit: foo = function() { }; // <-- I like to see a semicolon at the end of function definitions (e.g. where you define this.invoke). >- var wasCaught = invoker.wasCaught[jdx]; >+ var wasCaught = invoker.wasCaught[idx]; > if (!testFailed) > testFailed = wasCaught; > > ok(!wasCaught, > msg + "There is unexpected " + typeStr + " event."); > > } else { >- var wasCaught = invoker.wasCaught[jdx]; >+ var wasCaught = invoker.wasCaught[idx]; nit: wasCaught is redeclared unecessarily (via "var") + // Makes sure tree children accessibles are created otherwise they won't + // be a couse of name changed events. nit: type "couse" should be "cause" ? That's all I noticed. Did you run on all platforms? r=me with these changes, and confirmation of pass on all platforms, and Marco's approval.

Comment on attachment 364076 [details] [diff] [review] patch Nice work, thanks! r=me with David's comments fixed.

(In reply to comment #2) > (From update of attachment 364076 [details] [diff] [review]) > General nit: > > foo = function() { }; // <-- > I like to see a semicolon at the end of function definitions (e.g. where you > define this.invoke). do you mean function constructor() { this.func = function bla() { }; // <= } We didn't use this style in mochitest at all. What is the particular reason to change this? > nit: wasCaught is redeclared unecessarily (via "var") nope, the first wasCaught is declared in "if" statement, the second is in "else" statement. > nit: type "couse" should be "cause" ? right. > That's all I noticed. Did you run on all platforms? I tested it on Windows and Linux. But since this test failed previously I would appreciate if someone of us will retest it as well (sure if Marco didn't perform any tests).

The massive failure why I disabled the file completely a while back seems to have been fixed. We didn't have any problems with re-enabling XUL files over the past couple of weeks.

(In reply to comment #5) > The massive failure why I disabled the file completely a while back seems to > have been fixed. We didn't have any problems with re-enabling XUL files over > the past couple of weeks. Mostly I meant this // XXX see bug 454647 TreeInvalidatedHandlerHelper(aEvent, null, null, 0, 0, // "nsITreeBoxObject::invalidateColumn"); the reason this bug has been filed.

The tests all pass here as well. I think we're good to go with this!

(In reply to comment #4) > (In reply to comment #2) > > (From update of attachment 364076 [details] [diff] [review] [details]) > > General nit: > > > > foo = function() { }; // <-- > > I like to see a semicolon at the end of function definitions (e.g. where you > > define this.invoke). > > do you mean > function constructor() > { > this.func = function bla() > { > }; // <= > } Yes. > > We didn't use this style in mochitest at all. What is the particular reason to > change this? Okay, it just caught my eye this time. No need to change if that's our style. > > > nit: wasCaught is redeclared unecessarily (via "var") > > nope, the first wasCaught is declared in "if" statement, the second is in > "else" statement. The potential for redeclaration is still there, but it is only a nit. It isn't harmful :) r=me ( still :) ) Thanks.

(In reply to comment #8) > > We didn't use this style in mochitest at all. What is the particular reason to > > change this? > > Okay, it just caught my eye this time. No need to change if that's our style. thanks, I got usual to it :) > > nope, the first wasCaught is declared in "if" statement, the second is in > > "else" statement. > > The potential for redeclaration is still there, but it is only a nit. It isn't > harmful :) sorry, still don't see.

with David's nit