From: Ori Livneh <ori@wikimedia.org>
Date: Sat, 15 Aug 2015 01:00:30 +0000 (-0700)
Subject: Only convert boolean true/false to !0/!1
X-Git-Tag: 1.31.0-rc.0~10399^2
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=3ecec5434edd01da865a7b66c9729a44fc861133;p=lhc%2Fweb%2Fwiklou.git

Only convert boolean true/false to !0/!1

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
---

diff --git a/includes/libs/JavaScriptMinifier.php b/includes/libs/JavaScriptMinifier.php
index cb282eb537..141a5153d4 100644
--- a/includes/libs/JavaScriptMinifier.php
+++ b/includes/libs/JavaScriptMinifier.php
@@ -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;
diff --git a/tests/phpunit/includes/libs/JavaScriptMinifierTest.php b/tests/phpunit/includes/libs/JavaScriptMinifierTest.php
index 13908b9e62..d23534ed67 100644
--- a/tests/phpunit/includes/libs/JavaScriptMinifierTest.php
+++ b/tests/phpunit/includes/libs/JavaScriptMinifierTest.php
@@ -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,