[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 Brion Vibber changed: What|Removed |Added Depends on|29784 | --- Comment #28 from Brion Vibber 2011-08-04 14:04:38 UTC --- Removing the bug 29784 dependency -- it should still be fixed but isn't as big a problem for smaller site JS files. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 Mark A. Hershberger changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Comment #27 from Mark A. Hershberger 2011-08-03 18:37:37 UTC --- (In reply to comment #25) > This ought to do the job, but I recommend some more thorough testing to make > sure it doesn't break anything & some performance testing -- it does look like > JSParser is a bit sluggish but I haven't really solidly compared its behavior. It has been a month and there aren't reports of problems on TWN. Closing this. Bugs in implmentation or performance should be reported separately. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 Mark A. Hershberger changed: What|Removed |Added Blocks|29068 |29097 -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 Brion Vibber changed: What|Removed |Added Depends on||29784 --- Comment #26 from Brion Vibber 2011-07-11 21:45:00 UTC --- Per notes about memory usage from TranslateWiki (bug 29784) I've disabled validation for file-backed JS modules by default in r91914. Large libraries like jQuery & jQuery.ui when bundled up can hit the memory limits sooner than we like; while the core jQuery survived at my default setting, it would fail at 80M for me. The primary target for validation is on-wiki pages that are editable, while things on disk are provided with MediaWiki core & extensions and should already be debugged, so they don't really need to be validated anyway. This may still though die on largeish input from the wiki, so would be nice to cut it down. PHP/Zend's in-memory representation of zvals is notoriously inefficient, so the parse tree structure is likely super wasteful if it hasn't been optimized specifically for memory. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #25 from Brion Vibber 2011-07-06 21:51:40 UTC --- Ok r91591 adds JSMin+ to libs and uses its parser on the minifier unit tests (now tweaked to be valid JS programs). r91608 adds a $wgResourceLoaderValidateJs option (on by default) and uses JSMin+'s parser to validate JS coming from files or wiki pages into ResourceLoader. On parse error, the bad script chunk is replaced with a JS exception throw which lists the file/page name, line number, and parse error. This ought to do the job, but I recommend some more thorough testing to make sure it doesn't break anything & some performance testing -- it does look like JSParser is a bit sluggish but I haven't really solidly compared its behavior. Advantage of using a real parser here is of course that we can report errors based on the original line numbers rather than forcing people to switch to debug mode just to get a location. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #24 from P.Copp 2011-07-06 21:42:09 UTC --- Sorry, I meant bug 26791 -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #23 from P.Copp 2011-07-06 21:40:52 UTC --- (In reply to comment #21) > It's probably worth comparing jsmin+'s minification qualities to our current > home-grown minifier; it sounds like just swapping to jsmin+ as a minifier > might > be more robust (since it should be able to detect a JavaScript parse failure > and throw an error that lets us skip the bad file) but we could also use just > the parser as a verifier. > AFAIR JsMinPlus had been considered as a minifier for Mediawiki but turned out to be much slower than the alternatives, see bug 27691 for some benchmarks. (In reply to comment #22) > Unfortunately many of the test cases don't appear to be valid in the first > place... > Yeah, they weren't designed to be complete js programs, just little fragments to test edge cases that are likely to be broken by other minifiers. Should be straightforward to make them valid programs, though. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #22 from Brion Vibber 2011-07-06 19:04:07 UTC --- Created attachment 8752 --> https://bugzilla.wikimedia.org/attachment.cgi?id=8752 Quick patch adding jsmin+ and running its JS parser on JavaScriptMinifier test case results This quick patch adds jsminplus.php into libs/, makes it and its JSParser available via autoloader, and calls the parser on post-minified JS bits in JavaScriptMinifierTest. Unfortunately many of the test cases don't appear to be valid in the first place... ..EE.EE.EEE. Time: 0 seconds, Memory: 23.50Mb There were 7 errors: 1) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #2 ('\' Foo \\\' bar \\ baz \\\' quox \' .', '\' Foo \\\' bar \\ baz \\\' quox \'.') Exception: Parse error: Unexpected token; token 3 expected in file 'minify-test.js' on line 1 2) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #3 ('\\" Foo \\" bar n baz \\" quox " .', '\\" Foo \\" bar n baz \\" quox ".') Exception: Parse error: Illegal token in file 'minify-test.js' on line 1 3) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #5 ('/ Foo \\/ bar [ / \\] / ] baz / .', '/ Foo \\/ bar [ / \\] / ] baz /.') Exception: Parse error: Unexpected token; token 3 expected in file 'minify-test.js' on line 1 4) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #10 ('return x;', 'return x;') Exception: Parse error: Invalid return in file 'minify-test.js' on line 1 5) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #12 ('continue x;', 'continue x;') Exception: Parse error: Invalid continue in file 'minify-test.js' on line 1 6) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #13 ('break x;', 'break x;') Exception: Parse error: Invalid break in file 'minify-test.js' on line 1 7) JavaScriptMinifierTest::testJavaScriptMinifierOutput with data set #30 ('return/ x/g', 'return/ x/g') Exception: Parse error: Invalid return in file 'minify-test.js' on line 1 The return/continue/break bits appear to be problematic because they don't appear in a legitimate context (return must be in a function, continue/break must be in a looping construct). The quoted strings... ugh I haven't tried to decipher those yet. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #21 from Brion Vibber 2011-07-06 17:45:30 UTC --- I've done a little prelim poking at a custom parser for my own edification (mostly to make sure I better understood the ECMAScript specs) but it's a relatively low priority for me. It looks like jsmin+'s parser is indeed legit per description at http://crisp.tweakblogs.net/blog/1665/a-new-javascript-minifier-jsmin+.html -- that's a good catch, paul! On quick peek, looks like it's got a standalone JSParser class that produces a parse tree (and throws exceptions on parse error) which could be called explicitly, or simply used via the JSMinPlus class which calls through to the parser, then flattens the parse tree into minified source. License is... MPL 1.1/GPL 2.0/LGPL 2.1 so it should be fine to integrate into MediaWiki (yay!) It's probably worth comparing jsmin+'s minification qualities to our current home-grown minifier; it sounds like just swapping to jsmin+ as a minifier might be more robust (since it should be able to detect a JavaScript parse failure and throw an error that lets us skip the bad file) but we could also use just the parser as a verifier. If nothing else, I'd strongly recommend grabbing it and using it for the minification unit tests (to verify that minified source parses correctly). The library doesn't appear to have been updated since May 2009 and its parser is based on the JS-based implementation in https://github.com/mozilla/narcissus/wiki -- so it might need/want some slight tweaks for newer ECMAScript standard updates (though it's unlikely we'll be using any such new features in our code!) -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #20 from Mark A. Hershberger 2011-07-06 16:54:18 UTC --- Did you guys decide to use jsmin* or did brion write his own? How close are we to closing this? -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #19 from P.Copp 2011-06-29 07:28:12 UTC --- (In reply to comment #18) > Since I'm brushing up on my low-level parsing skills anyway ;) I'm making a > quick stab at building a PHP-based recursive-descent parser following the > lexical & syntactical grammars in the ECMAScript standard -- this should be > able to determine pretty cleanly what does/doesn't parse, and could be invoked > at page save time to give feedback on identifying syntax errors in editable > .js > pages. Just to save you a bit of work, note that there is already JSMinPlus [1][2], which is pretty much a JS parser, written in pure PHP. It should be easy to adapt it a bit to make it a syntax checker. [1] Desription: http://crisp.tweakblogs.net/blog/1856/jsmin+-version-13.html [2] Source: http://files.tweakers.net/jsminplus/jsminplus.zip -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 Brion Vibber changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #18 from Brion Vibber 2011-06-29 01:44:15 UTC --- Since I'm brushing up on my low-level parsing skills anyway ;) I'm making a quick stab at building a PHP-based recursive-descent parser following the lexical & syntactical grammars in the ECMAScript standard -- this should be able to determine pretty cleanly what does/doesn't parse, and could be invoked at page save time to give feedback on identifying syntax errors in editable .js pages. May or may not end up reasonable enough to actually run it within ResourceLoader; like minification, the result can be cached so it might actually be reasonable to do. :) -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 --- Comment #17 from Krinkle 2011-06-28 23:32:59 UTC --- Just for the record, having a JSLint or JSHint (that is GPL) compatible in PHP doesn't (directly) help this bug getting fixed. Many lints and hints are good practices or could cause constructions not to work, or exceptions to be thrown. But just checking syntax errors probably requires a different (possibly simpler) kind of validator. This could be derived from JSHint or JSLint-ish tools, though. But the kind of thing we should probably check for: * Unclosed braces, brackets, quotes, apostrophes and parentheses. * Closing too many of the above. * Anything else ? -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 28626] JS syntax errors should not affect other modules in a load.php request
https://bugzilla.wikimedia.org/show_bug.cgi?id=28626 Krinkle changed: What|Removed |Added Summary|JS syntax errors affect |JS syntax errors should not |other modules in|affect other modules in a |ResourceLoader |load.php request -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l