$strContent = self::filter( $filter, $strContent );
}
- $out .= $strContent;
+ if ( $context->getOnly() === 'scripts' ) {
+ // Use a linebreak between module scripts (T162719)
+ $out .= $this->ensureNewline( $strContent );
+ } else {
+ $out .= $strContent;
+ }
} catch ( Exception $e ) {
$this->outputErrorAndLog( $e, 'Generating module package failed: {exception}' );
if ( !$context->getDebug() ) {
$stateScript = self::filter( 'minify-js', $stateScript );
}
- $out .= $stateScript;
+ // Use a linebreak between module script and state script (T162719)
+ $out = $this->ensureNewline( $out ) . $stateScript;
}
} else {
if ( count( $states ) ) {
return $out;
}
+ /**
+ * Ensure the string is either empty or ends in a line break
+ * @param string $str
+ * @return string
+ */
+ private function ensureNewline( $str ) {
+ $end = substr( $str, -1 );
+ if ( $end === false || $end === "\n" ) {
+ return $str;
+ }
+ return $str . "\n";
+ }
+
/**
* Get names of modules that use a certain message.
*
$scripts = $this->getScriptURLsForDebug( $context );
} else {
$scripts = $this->getScript( $context );
- // rtrim() because there are usually a few line breaks
- // after the last ';'. A new line at EOF, a new line
- // added by ResourceLoaderFileModule::readScriptFiles, etc.
+ // Make the script safe to concatenate by making sure there is at least one
+ // trailing new line at the end of the content. Previously, this looked for
+ // a semi-colon instead, but that breaks concatenation if the semicolon
+ // is inside a comment like "// foo();". Instead, simply use a
+ // line break as separator which matches JavaScript native logic for implicitly
+ // ending statements even if a semi-colon is missing.
+ // Bugs: T29054, T162719.
if ( is_string( $scripts )
&& strlen( $scripts )
- && substr( rtrim( $scripts ), -1 ) !== ';'
+ && substr( $scripts, -1 ) !== "\n"
) {
- // Append semicolon to prevent weird bugs caused by files not
- // terminating their statements right (T29054)
- $scripts .= ";\n";
+ $scripts .= "\n";
}
}
$content['scripts'] = $scripts;
[
[ 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ],
"<script>(window.RLQ=window.RLQ||[]).push(function(){"
- . "mw.test.baz({token:123});mw.loader.state({\"test.quux\":\"ready\"});"
+ . "mw.test.baz({token:123});\nmw.loader.state({\"test.quux\":\"ready\"});"
. "});</script>"
],
];
return [
[
"mw.foo()",
- "mw.foo();\n",
+ "mw.foo()\n",
],
[
"mw.foo();",
- "mw.foo();",
+ "mw.foo();\n",
],
[
"mw.foo();\n",
],
[
"mw.foo()\n",
- "mw.foo()\n;\n",
+ "mw.foo()\n",
],
[
"mw.foo()\n// mw.bar();",
- "mw.foo()\n// mw.bar();",
+ "mw.foo()\n// mw.bar();\n",
+ ],
+ [
+ "mw.foo()\n// mw.bar()",
+ "mw.foo()\n// mw.bar()\n",
],
[
"mw.foo()// mw.bar();",
- "mw.foo()// mw.bar();",
+ "mw.foo()// mw.bar();\n",
],
];
}
'modules' => [
'foo' => 'foo()',
],
- 'expected' => "foo();\n" . 'mw.loader.state( {
+ 'expected' => "foo()\n" . 'mw.loader.state( {
"foo": "ready"
} );',
- 'minified' => "foo();" . 'mw.loader.state({"foo":"ready"});',
+ 'minified' => "foo()\n" . 'mw.loader.state({"foo":"ready"});',
'message' => 'Script without semi-colon',
],
[
'foo' => 'foo()',
'bar' => 'bar()',
],
- 'expected' => "foo();\nbar();\n" . 'mw.loader.state( {
+ 'expected' => "foo()\nbar()\n" . 'mw.loader.state( {
"foo": "ready",
"bar": "ready"
} );',
- 'minified' => "foo();bar();" . 'mw.loader.state({"foo":"ready","bar":"ready"});',
+ 'minified' => "foo()\nbar()\n" . 'mw.loader.state({"foo":"ready","bar":"ready"});',
'message' => 'Two scripts without semi-colon',
],
[
'modules' => [
'foo' => "foo()\n// bar();"
],
- // FIXME: Invalid code (T162719)
- 'expected' => "foo()\n// bar();" . 'mw.loader.state( {
+ 'expected' => "foo()\n// bar();\n" . 'mw.loader.state( {
"foo": "ready"
} );',
- // FIXME: Invalid code (T162719)
- 'minified' => "foo()" . 'mw.loader.state({"foo":"ready"});',
- 'message' => 'Script with semi-colon in comment',
+ 'minified' => "foo()\n" . 'mw.loader.state({"foo":"ready"});',
+ 'message' => 'Script with semi-colon in comment (T162719)',
],
];
$ret = [];
);
$response = $rl->makeModuleResponse( $context, $modules );
- $this->assertCount( 0, $rl->getErrors(), 'Error count' );
+ $this->assertSame( [], $rl->getErrors(), 'Errors' );
$this->assertEquals( $expected, $response, $message ?: 'Response' );
}
$this->assertCount( 1, $errors );
$this->assertRegExp( '/Ferry not found/', $errors[0] );
$this->assertEquals(
- 'foo();bar();mw.loader.state( {
+ "foo();\nbar();\n" . 'mw.loader.state( {
"ferry": "error",
"foo": "ready",
"bar": "ready"