Last modified: 2011-02-08 21:57:00 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 26931 - JavaScript minification breaks production when line starts with slash in block comments
JavaScript minification breaks production when line starts with slash in bloc...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.18.x
All All
: Normal critical (vote)
: ---
Assigned To: Trevor Parscal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-25 14:00 UTC by Krinkle
Modified: 2011-02-08 21:57 UTC (History)
3 users (show)

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


Attachments

Description Krinkle 2011-01-25 14:00:24 UTC
The following code
<pre>
/**
 * Foobar:
 * {
 * 'ltr' : {
 * //Multiple rules with configurable operators
 * 'iphone' : false
 * }
 * 
 * @return Boolean true lorem ipsum
 */
</pre>
Is minified as:
<pre>
/**
*Foobar:
*{
*'ltr':{
*//Multiple rules with configurable operators
*'iphone':false
*}
*
*@return Boolean true lorem ipsum
*/
</pre>

The following line specifically is where it all breaks:
<pre>
*//Multiple rules with configurable operators
</pre>

Because it is considered as the closure of the block comment " */ " and the (unexpected) start of a regex.

"Error: unterminated regular expression literal"
Source File: http://translatewiki.net/w/load.php?debug=false&lang=fi&modules=ext.liquidThreads%7Cjquery.autoEllipsis%7Cjquery.checkboxShiftClick%7Cjquery.client%7Cjquery.collapsibleTabs%7Cjquery.cookie%7Cjquery.delayedBind%7Cjquery.highlightText%7Cjquery.makeCollapsible%7Cjquery.placeholder%7Cjquery.suggestions%7Cjquery.tabIndex%7Cjquery.ui.button%7Cjquery.ui.core%7Cjquery.ui.dialog%7Cjquery.ui.draggable%7Cjquery.ui.mouse%7Cjquery.ui.position%7C

Thanks to TranslateWiki Nikerabbit for reporting this on IRC.

I'm confused though that this is production output, why are there comments in production output ?
That empty lines were insterted in place of comments (before the minifier switch by Trevor[0]) to keep track of the line number (although that only applies to the first file of all loaded modules) was one, but keeping all comments ? We might as well disable the minifier then.

--
Krinkle

[0] http://lists.wikimedia.org/pipermail/wikitech-l/2011-January/051308.html
Comment 1 Trevor Parscal 2011-01-25 18:16:18 UTC
This output does not appear to reflex the latest version of the minifier. I tested running the raw versions of these modules through the latest version and no such errors are present.

I think the old minified versions are still in cache, and need to be purged.
Comment 2 Krinkle 2011-01-25 18:44:57 UTC
TWN's sandwiki has been updated to the lastest trunk 2 minutes ago:

http://translatewiki.net/sandwiki/load.php?debug=false&lang=fi&modules=ext.liquidThreads%7Cjquery.client

Search for "*//"
It is still removing the space between * and //, causing the comment block to end early and a regex to be started in a wrong place:

<pre>
/**
*Checks the current browser against a support map object to determine if the browser has been black-listed or
*not.If the browser was not configured specifically it is assumed to work.It is assumed that the body
*element is classified as either"ltr"or"rtl".If neither is set,"ltr"is assumed.
*
*A browser map is in the following format:
*{
*'ltr':{
*//Multiple rules with configurable operators
*'msie':[['>=',7],['!=',9]],
*//Blocked entirely
*'iphone':false
*},
*'rtl':{
*//Test against a string
*'msie':[['!==','8.1.2.3']],
*//RTL rules do not fall through to LTR rules,you must explicity set each of them
*'iphone':false
*}
*}
*
*@param map Object of browser support map
*
*@return Boolean true if browser known or assumed to be supported,false if blacklisted
*/
</pre>

If it is going to keep comments in production mode, it should not touch touch those lines at all.
Comment 3 Trevor Parscal 2011-01-25 18:47:16 UTC
Yeah, I actually am working on a patch - I found a way to reproduce..
Comment 4 Trevor Parscal 2011-01-25 19:01:10 UTC
This should be resolved in r80979 - can someone please update tw.n and verify?
Comment 5 Krinkle 2011-01-25 19:03:30 UTC
Looks fine now. Thx.
Should this be fixed upstream by the way ?
Comment 6 Trevor Parscal 2011-01-25 19:08:12 UTC
Yes, in a few days, if this stablizes a bit, I will push back upstream our code - which the author should be able to make some use of in his JavaScriptPacker.

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


Navigation
Links