Replace JS1.7's nonstandard destructuring for…in loops in mochitest/server.js with regular for…in loops. JS1.8 changed the behavior of key/value destructuring of arrays and we would like to remove SpiderMonkey's special case for JS1.7 (bug 1083498). https://developer.mozilla.org/en-US/docs/Web/JavaScript/New_in_JavaScript/1.8#Changes_in_destructuring_for..in

Replace JS1.7's nonstandard destructuring for…in loops in layout/analysis/simple-match.js with regular for…in loops. Also remove unused variable `matches`, replace nonstandard expression closure `identity` with a real function (bug 1083459), and fix some comment typos. JS1.8 changed the behavior of key/value destructuring of arrays and we would like to remove SpiderMonkey's special case for JS1.7 (bug 1083498). https://developer.mozilla.org/en-US/docs/Web/JavaScript/New_in_JavaScript/1.8#Changes_in_destructuring_for..in I suspect that no one has used this script (simple-match.js and pixel-conversion.js) since it landed in 2009 (bug 495002) because it would have been broken by the destructuring for…in loop change in JS1.8. Should we just remove these scripts in https://hg.mozilla.org/mozilla-central/file/840cfd5bc971/layout/analysis?

Comment on attachment 8582209 [details] [diff] [review] replace-layout-analysis-loop.patch This seems fine. I always forget the details of these JS looping techniques, but to sanity-check, I tested a simplified version of the loop with & without the change -- they both behave the same (and have the variables set correctly) when inside of a script tag like: <script type="application/javascript;version=1.7"> BUT, the old version is indeed broken when inside of a script tag with a 1.8 version-number: <script type="application/javascript;version=1.8"> (This is the first I've heard of these scripts, and I'm also wondering whether they serve any purpose now. I'd check with the author or reviewer before removing them; but I'm happy to sign off on this minor cleanup.)

Comment on attachment 8582209 [details] [diff] [review] replace-layout-analysis-loop.patch Daniel, you wrote the layout/analysis/pixel-conversion.js and simple-match.js scripts for some CSSPixelsToDevPixels and DevPixelsToCSSPixels source transformations. Is there any need to keep these scripts in mozilla-central? They are broken in JS1.8 and JS1.8.5, js shell's default JS dialect, because they depend on some JS1.7-specific quirks of for…in loops.

Thanks, Joel! replace-mochitest-server-loops.patch https://hg.mozilla.org/integration/mozilla-inbound/rev/ab32e77962e2

Remove unused layout/analysis/pixel-conversion.js and simple-match.js scripts. Daniel, how about we just remove these scripts that are broken in JS1.8 and not referenced by any scripts in mozilla-central?

Comment on attachment 8583571 [details] [diff] [review] remove-unused-layout-scripts.patch See end of comment 3 -- I'd like input from someone who was involved w/ these scripts being added before yanking them out. (It's not too surprising that they're not used in the tree; IIUC, they exist to help patch-authors do tree-wide refactoring. Though I'm not sure they've ever been for that used since they were added.)

Comment on attachment 8583571 [details] [diff] [review] remove-unused-layout-scripts.patch Remove unused layout/analysis/pixel-conversion.js and simple-match.js scripts. These scripts were added in bug 495002 to find code that could use the CSSPixelsToDevPixels() and DevPixelsToCSSPixels() conversion functions. Is there any need to keep these scripts in mozilla-central? They are broken in js shell's default JS dialect (JS1.8.5) because they depend on JS1.7-specific behavior of for…in loops (and even that JS1.7-specific behavior has been removed in Nightly 40 by bug 1083498).

Comment on attachment 8583571 [details] [diff] [review] remove-unused-layout-scripts.patch Review of attachment 8583571 [details] [diff] [review]: ----------------------------------------------------------------- Oops, misfire

Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/7ba5b502e7b7