bug 591981 didn't turn the tinderboxen orange, but it stops most of the mochitests in the MochiKit directory from running. The reason the tests don't "fail" in a way tinderbox catches is because we fail to load the SimpleTest.js file, which declares 'ok' and 'is', meaning that we can't inform the harness that we've failed, so we end up passing having run 0 tests. We need to fix this somehow; we should probably back out bug 591981 in the meantime.

This probably needs to block b7, no? Sounds like a web compat issue to me.

Is this bug 559839 all over again?

(In reply to comment #1) > Sounds like a web compat issue to me. If this were a Web compat issue, how would the scripts in question work in WebKit or IE?

How about compatibility with the 30% of the web that runs FF without this bug?

> If this were a Web compat issue, how would the scripts in question work in > WebKit or IE? I have no idea, but mochikit works in webkit and IE last I checked.

(In reply to comment #2) > Is this bug 559839 all over again? That's a part of this -- the change here probably should have turned the tree orange. It's probably worth filing a separate bug on that, though.

Fixed by backing out bug 591981 http://hg.mozilla.org/mozilla-central/rev/c0852588831b

(In reply to comment #0) > bug 591981 didn't turn the tinderboxen orange, but it stops most of the > mochitests in the MochiKit directory from running. There are multiple directories named MochiKit or mochikit. Which one failed? (In reply to comment #4) > How about compatibility with the 30% of the web that runs FF without this bug? So far, there's no evidence of any Web site breaking due to bug 591981. It wouldn't be the first time when mochitests depend on Geckoisms that the Web doesn't depend on, because the Web also needs to work with IE and WebKit. Even if bug 591981 did break a site, we have to be able to converge on standard behaviors and not perpetuate browser differences even if sites that UA sniff expect differences to be perpetual. (In reply to comment #7) > Fixed by backing out bug 591981 Thanks. I'm trying to figure out what broke and why.

FWIW, the demos linked from http://mochikit.com/demos.html work with bug 591981 fixed, so the assumption that bug 591981 broke all MochiKit sites looks a bit premature.

So testing/mochitest/tests/MochiKit-1.4.2/tests/SimpleTest/SimpleTest.js loads the other SimpleTest.js using a non-blocking mechanism but expects blocking behavior. The link in the comments is to an article that specifically tries to be non-blocking. And bug 591981 made Gecko really non-blocking.

(In reply to comment #9) > FWIW, the demos linked from http://mochikit.com/demos.html work with bug 591981 > fixed, so the assumption that bug 591981 broke all MochiKit sites looks a bit > premature. I don't think anyone made that claim. Clearly most other tests in our harness are running fine, and obviously they run mochitest. In any event, this discussion isn't about if we wan't to do bug 591981 or not. However all code that lands on mozilla-central needs to pass tests and what landed did not. So we need to just figure out why and fix it, nothing extraordinary to see here. My assumption was that it was the tests in the following directory that failed: /tests/MochiKit-1.4.2/tests/ I.e. the ones at the top if you run mochitest manually.

If the tests are wrong, please do fix the tests (ideally in a separate patch)

(In reply to comment #11) > I don't think anyone made that claim. I guess I misunderstood comment 4. > So we need to just figure out why and fix it, nothing > extraordinary to see here. Indeed. My point was that there isn't a Web compat problem here. I'm sorry I didn't mention that I was unable to back out at the time when I wrote comment 3. (In reply to comment #12) > If the tests are wrong, please do fix the tests (ideally in a separate patch) I think I have a fix. I'm testing the fix still.

Comment on attachment 479017 [details] [diff] [review] Make MochiKit tests use the Mozilla version of SimpleTest.js properly This works fine, and is consistent with the way other mochitests load SimpleTest.js.

Thanks for the r. Landed: http://hg.mozilla.org/mozilla-central/rev/6e9809698df7

A changeset from this bug was associated with a XP Ts, Cold MED Dirty Profile regression on Firefox. boo-urns :( Previous: avg 420.356 stddev 3.679 of 30 runs up to a9d1ad0bc386 New : avg 430.832 stddev 1.905 of 5 runs since a60414d076b5 Change : +10.475 (2.49% / z=2.847) Graph : http://mzl.la/bZFaB3 The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-ts_cold_generated_med] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.

A changeset from this bug was associated with a Win7 Dromaeo (CSS) regression on Firefox. boo-urns :( Previous: avg 2014.419 stddev 40.480 of 30 runs up to a9d1ad0bc386 New : avg 1901.610 stddev 12.432 of 5 runs since a60414d076b5 Change : -112.809 (-5.6% / z=2.787) Graph : http://mzl.la/9gFu4n The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.

A changeset from this bug was associated with a Win7 Dromaeo (jslib) regression on Firefox. boo-urns :( Previous: avg 127.610 stddev 4.222 of 30 runs up to a9d1ad0bc386 New : avg 116.384 stddev 0.751 of 5 runs since a60414d076b5 Change : -11.226 (-8.8% / z=2.659) Graph : http://mzl.la/bZu5UP The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.

A changeset from this bug was associated with a XP Dromaeo (CSS) regression on Firefox. boo-urns :( Previous: avg 2045.275 stddev 49.676 of 30 runs up to a9d1ad0bc386 New : avg 1936.120 stddev 13.937 of 5 runs since a60414d076b5 Change : -109.155 (-5.34% / z=2.197) Graph : http://mzl.la/b0dlou The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.

A changeset from this bug was associated with a XP Dromaeo (jslib) regression on Firefox. boo-urns :( Previous: avg 129.703 stddev 4.099 of 30 runs up to a9d1ad0bc386 New : avg 117.954 stddev 0.660 of 5 runs since a60414d076b5 Change : -11.749 (-9.06% / z=2.866) Graph : http://mzl.la/avZij4 The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.

perf regression(s): must be due to 1 of the 2 other pushes, but not this one. ***** (In reply to comment #10) > So testing/mochitest/tests/MochiKit-1.4.2/tests/SimpleTest/SimpleTest.js loads > the other SimpleTest.js using a non-blocking mechanism but expects blocking > behavior. The link in the comments is to an article that specifically tries to > be non-blocking. And bug 591981 made Gecko really non-blocking. As I understood it when I wrote the stub in bug 427500, the idea was that the actual file load (and execution) itself could be non-blocking, as the actual script node was inserted in HEAD, and HEAD was supposed to be fully processed before BODY would start to be. I'm not familiar with bug 591981 "execute-in-insertion-order" (removal): iiuc, that means scripts are now loaded (in parallel?) without a specific order. But I would have expected that to be true for HEAD and BODY separately. Was it really intended to remove the "all HEAD done before BODY start" rule?

(In reply to comment #22) > perf regression(s): must be due to 1 of the 2 other pushes, but not this one. Yeah. Furthermore, the patch for bug 591981 had already been backed out without Dromaeo effects, so it's safe to attribute the Dromaeo effects to PGO noise. > (In reply to comment #10) > > So testing/mochitest/tests/MochiKit-1.4.2/tests/SimpleTest/SimpleTest.js loads > > the other SimpleTest.js using a non-blocking mechanism but expects blocking > > behavior. The link in the comments is to an article that specifically tries to > > be non-blocking. And bug 591981 made Gecko really non-blocking. > > As I understood it when I wrote the stub in bug 427500, > the idea was that the actual file load (and execution) itself could be > non-blocking, If the test case isn't trying to tests non-blocking script loading, it's best to use a plain <script src='...'></script> tag in the .html file. In general, it's a bad idea to use tricks in mochitests unless the tricks are what's specifically being tested. Some tricks will accidentally be Gecko-specific and will cause grief when Gecko aligns with standards and the standard adopted IE's or WebKit's behavior instead of Gecko's. (There are times when standards adopt Gecko's behaviors, but of course we don't see mochitest failures due to that.) > as the actual script node was inserted in HEAD, > and HEAD was supposed to be fully processed before BODY would start to be. HEAD and BODY really have nothing to do with this before or after bug 591981. > I'm not familiar with bug 591981 "execute-in-insertion-order" (removal): > iiuc, that means scripts are now loaded (in parallel?) without a specific > order. It means that insertion order execution isn't enforced for *script-inserted* scripts--i.e. scripts created with document.createElement() as opposed to being parser from a <script> tag. Script-created inline scripts (i.e. those that don't have an src attribute) execute when they are inserted to a document. Script-created external scripts (i.e. those that do have an src attribute) execute as soon as the script text has been loaded from the network and the event loop gets to spin. > But I would have expected that to be true for HEAD and BODY separately. > > Was it really intended to remove the "all HEAD done before BODY start" rule? There was no such rule in Gecko. The old rule was that if you had <script src="a.js"></script><script src="b.js"></script> and a.js inserted a script node pointing to c.js, b.js would wait until c.js had been loaded from the network and been evaluated. The new rule is that b.js waits for a.js but not for c.js. c.js will be executed as soon as it's ready. In the old case and in the new case, HEAD and BODY have nothing to do with it. That is, advice such as http://www.nczonline.net/blog/2009/07/28/the-best-way-to-load-external-javascript/ has been working as advertised in WebKit and IE but not in Gecko. Now it works as advertised in Gecko, too.