It's really quite unexpected that a head.js would call waitForFinish() - that isn't even supposed to be called before the test's test() method is. It's also quite confusing to have variables declared in head.js, since it's not obvious when reading tests where they come from. Also, rather than defining some globals and having all tests follow essentially the same |addTab(URI); browser.addEventListener();| pattern, I think it would be better to add a common onTabLoad() helper that accepts a callback to invoke on load of a given URI.

Assigning to ddahl as I believe he created head.js when splitting up the tests. Obviously, we're trying to finish up some of the feature still, but we should ensure that the tests follow best practices as much as is possible.

(In reply to comment #0) > It's really quite unexpected that a head.js would call waitForFinish() - that > isn't even supposed to be called before the test's test() method is. It's also > quite confusing to have variables declared in head.js, since it's not obvious > when reading tests where they come from. > I thought that was the whole point of head.js: http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/test/browser/head.js http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/test/head.js http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpinstall/head.js http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/tabview/head.js > Also, rather than defining some globals and having all tests follow essentially > the same |addTab(URI); browser.addEventListener();| pattern, I think it would > be better to add a common onTabLoad() helper that accepts a callback to invoke > on load of a given URI. That sounds cleaner, yes. This sounds like a good post-fx-4 clean up bug

(In reply to comment #2) > (In reply to comment #0) > > It's really quite unexpected that a head.js would call waitForFinish() - \\ I imagine this is true, the previous comment was about defining functions in head.js

Functions are fine, it's the variables that aren't. Currently tests assign to tab/browser/hudId/hud/hudBox/filterBox/outputNode/cs without declaring them, and it looks like they're using undeclared variables until you remember that they're declared in head.js. There isn't really any benefit to avoiding the declaration in the test files, and there are cleaner/more obvious ways to share state.

mass change: filter on PRIORITYSETTING

As Gavin mentioned in bug 660619, we should also be removing the imports from head.js: "Test code shouldn't be re-importing all of these things - if they're really needed they're already imported by the chrome window the test is running in."

triage. Filter on PINKISBEAUTIFUL

Inactive for 5 years. Closing now.