As in summary. Bonus points for (as far as JS is concerned) checking for stupid mistakes like assigning to undeclared variables, using undefined variables, else after return, etc. (but some of those are harder because you need to know what variables are around and what gets created by DOM, etc.)

This would have prevented bug 999080. I have a WIP patch for the CSS case.

Alright, let's see the damage. remote: https://tbpl.mozilla.org/?tree=Try&rev=1278e410d17c

Mike, want to have a look at how you feel about this test? :-)

Awesome, I was just thinking about something like this the other day too :)

(In reply to :Gijs Kruitbosch from comment #2) > Alright, let's see the damage. > > remote: https://tbpl.mozilla.org/?tree=Try&rev=1278e410d17c Well, that was dumb. :-(

So I went and filed & patched bug 1004410 and checked that this time the patch isn't busted (at least, it seems to work locally...), and repushed: remote: https://tbpl.mozilla.org/?tree=Try&rev=92798d0cf31f I fully expect to find more extant CSS problems in the try push, which I intend to add to the filter so we can land this ASAP and then eliminate issues one-by-one as appropriate.

Filed bug 1004418 for the JS issue.

Leaks everywhere, but no more CSS issues. Good, I guess. I pushed another patch that doesn't leak for me locally (when building debug - I'd already fixed the leaks found in an opt build, but debug builds have a different leakfinder and so I had to do some more fiddling...). remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7f7abc547fb9 I also changed the whitelist mechanism to allow being more specific about what to ignore, so we can still detect "new" issues in files even if there are "known" issues in the file. Requesting review in a bit.

(In reply to :Gijs Kruitbosch from comment #8) > Leaks everywhere, but no more CSS issues. Good, I guess. I pushed another > patch that doesn't leak for me locally (when building debug - I'd already > fixed the leaks found in an opt build, but debug builds have a different > leakfinder and so I had to do some more fiddling...). > > remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7f7abc547fb9 Err, I meant: remote: https://tbpl.mozilla.org/?tree=Try&rev=9193c3bb0221

Comment on attachment 8415840 [details] [diff] [review] add test to parse all our CSS and check for obvious issues, Review of attachment 8415840 [details] [diff] [review]: ----------------------------------------------------------------- I think this is both fine and super useful, providing: 1) We add documentation about what things are whitelisted and why 2) We add documentation on how each of these functions operate 3) We ensure that this does not incur a huge cost time-wise in terms of running mochitest-bc Overall though, I think this is a splendid idea. ::: browser/base/content/test/general/browser_parsable_css.js @@ +1,1 @@ > +const kWhitelist = [ Missing license header @@ +1,5 @@ > +const kWhitelist = [ > + {sourceName: /cleopatra.*(tree|ui)\.css/i}, > + {sourceName: /codemirror\.css/i}, > + {sourceName: /web\/viewer\.css/i, errorMessage: /Unknown pseudo-class.*(fullscreen|selection)/i }, > + {sourceName: /downloads\.css/i}, Can we put some explanations in here about why these things are whitelisted? @@ +153,5 @@ > + doc.head.innerHTML = ''; > + doc = null; > + iframe = null; > +}); > + Unnecessary newlines at the end of the file.

Thanks for the super-quick review! Landed with a bunch of docs in the file, and the downloads.css whitelist removed because I fixed the relevant bits in bug 1004431. remote: https://hg.mozilla.org/integration/fx-team/rev/706bc42060aa remote: https://hg.mozilla.org/integration/fx-team/rev/a38146bf6d18 I'll post to m.d.platform and fx-dev about this so people are aware.

(In reply to :Gijs Kruitbosch from comment #12) > I'll post to m.d.platform and fx-dev about this so people are aware. https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.platform/HfjjSyPi5MU https://groups.google.com/forum/?fromgroups=&hl=en#!topic/firefox-dev/HfjjSyPi5MU

https://hg.mozilla.org/mozilla-central/rev/9c0ab8f376e6 https://hg.mozilla.org/mozilla-central/rev/a38146bf6d18

(In reply to Mike Conley (:mconley) from comment #11) > 3) We ensure that this does not incur a huge cost time-wise in terms of > running mochitest-bc Our slowest bc1 runs these days are Win7 and 10.6 (debug), and checking on win7, I'm seeing runtimes of 42-44minutes both before and after landing this, so I don't think it's having much of an impact. :-)