Only convert boolean true/false to !0/!1
authorOri Livneh <ori@wikimedia.org>
Sat, 15 Aug 2015 01:00:30 +0000 (18:00 -0700)
committerOri Livneh <ori@wikimedia.org>
Sat, 15 Aug 2015 21:13:59 +0000 (14:13 -0700)
Fix-up for I5ab29b686b8. If we encounter stupid code like
`a.true = 1;` or `a = { true: 1 }`, we should not convert that to !0/!1.
Because JSMin barfs on such input, it is necessary to add another parameter to
the test method which specifies whether or not the minified JavaScript is
supposed to be valid JavaScript by the standards of JSMin.

Change-Id: Ib78c628147fdb95982d6e33e0ab298584fb63d0b

includes/libs/JavaScriptMinifier.php
tests/phpunit/includes/libs/JavaScriptMinifierTest.php

index cb282eb..141a515 100644 (file)
@@ -565,10 +565,13 @@ class JavaScriptMinifier {
                                $out .= ' ';
                                $lineLength++;
                        }
-                       if( $token === 'true' ) {
-                               $token = '!0';
-                       } elseif( $token === 'false' ) {
-                               $token = '!1';
+                       if (
+                               $type === self::TYPE_LITERAL
+                               && ( $token === 'true' || $token === 'false' )
+                               && ( $state === self::EXPRESSION || $state === self::PROPERTY_EXPRESSION )
+                               && $last !== '.'
+                       ) {
+                               $token = ( $token === 'true' ) ? '!0' : '!1';
                        }
 
                        $out .= $token;
index 13908b9..d23534e 100644 (file)
@@ -140,6 +140,13 @@ class JavaScriptMinifierTest extends PHPUnit_Framework_TestCase {
                        array( "5..toString();", "5..toString();" ),
                        array( "5...toString();", false ),
                        array( "5.\n.toString();", '5..toString();' ),
+
+                       // Boolean minification (!0 / !1)
+                       array( "var a = { b: true };", "var a={b:!0};" ),
+                       array( "var a = { true: 12 };", "var a={true:12};", false ),
+                       array( "a.true = 12;", "a.true=12;", false ),
+                       array( "a.foo = true;", "a.foo=!0;" ),
+                       array( "a.foo = false;", "a.foo=!1;" ),
                );
        }
 
@@ -147,15 +154,17 @@ class JavaScriptMinifierTest extends PHPUnit_Framework_TestCase {
         * @dataProvider provideCases
         * @covers JavaScriptMinifier::minify
         */
-       public function testJavaScriptMinifierOutput( $code, $expectedOutput ) {
+       public function testJavaScriptMinifierOutput( $code, $expectedOutput, $expectedValid = true ) {
                $minified = JavaScriptMinifier::minify( $code );
 
                // JSMin+'s parser will throw an exception if output is not valid JS.
                // suppression of warnings needed for stupid crap
-               MediaWiki\suppressWarnings();
-               $parser = new JSParser();
-               MediaWiki\restoreWarnings();
-               $parser->parse( $minified, 'minify-test.js', 1 );
+               if ( $expectedValid ) {
+                       MediaWiki\suppressWarnings();
+                       $parser = new JSParser();
+                       MediaWiki\restoreWarnings();
+                       $parser->parse( $minified, 'minify-test.js', 1 );
+               }
 
                $this->assertEquals(
                        $expectedOutput,