Last modified: 2011-03-14 14:26:09 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 27054 - JS minification bug in ResourceLoader: ambiguous implied end of statement at end of file can break parsing of subsequent file
JS minification bug in ResourceLoader: ambiguous implied end of statement at ...
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
All All
: Normal normal (vote)
: ---
Assigned To: Trevor Parscal
Depends on:
  Show dependency treegraph
Reported: 2011-01-31 00:42 UTC by Brion Vibber
Modified: 2011-03-14 14:26 UTC (History)
1 user (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---

Proposed patch: Add a semicolon to each script before combining (486 bytes, patch)
2011-02-21 18:26 UTC, P.Copp

Description Brion Vibber 2011-01-31 00:42:17 UTC
Noticed this structure while cleaning up SVGEdit; like bug 27046 this involves code that works fine with debug=false, but can end up breaking with minification. Unlike that bug, the whitespace is being preserved ok -- but the breakage occurs nonetheless.

JS allows the end of a statement to be implied in many circumstances, leaving out the final semicolon. This is IMO sloppy, but is standard and is pretty frequent.

I found that the way we're concatenating scripts together can make this break:

Script one:

    var barf = {

Script two:
    (function() {

Concatenated script:
    var barf = {
    (function() {

This concatenated script actually gets parsed & run the same as:

    var barf = {
    }(function() {

that is to say, the wonderful world of JS tries to make a function call on the result of the object literal "{}". This naturally fails.

(Note that the whitespace stripping actually doesn't make a difference here -- you can have as many or few spaces, line breaks, or comments as you like here.

Recommended fix: minification system needs to know when concatenation changes how statement boundaries are parsed and add an explicit semicolon, or else throw an explicit and clear error message.

Workaround: add an explicit semicolon on ambiguous statements.

I've also found that the error from this happening seems to be hidden from the JS console, but I'm not sure if this is just my other wonky stuff going on or not. But it is devilishly hard to track down!

Haven't double-checked with the JSMin system to see if this was already done, but didn't come across this issue before.
Comment 1 Brion Vibber 2011-01-31 00:42:41 UTC
I mean "works fine with debug=true" blahhhhh :D
Comment 2 P.Copp 2011-02-21 18:26:23 UTC
Created attachment 8187 [details]
Proposed patch: Add a semicolon to each script before combining

The fix is quite simple: Every script has to end in a statement anyway, so just add a semicolon after each file, which is in the worst case superfluous, but only adds 1 byte per module to the output.
Comment 3 Roan Kattouw 2011-03-14 14:26:09 UTC
Patch applied in r83897. Thanks!

Note You need to log in before you can comment on or make changes to this bug.