Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

resourceloader: Fail gracefully if a LESS message is not found

Browse source 
The message cache is originally meant for mw.messages in JS,
which expects non-existent messages to be cleanly omitted.
There is minimal server-side and client-side handling in place for this.

For LESS, however, we were assuming that the blob is complete,
thus not feeding anything to the LESS compiler, thus leading to
a run-time failure where the LESS file can't be parsed at all
due to an undeclared variable.

This could happen sometimes during development or after upgrading
a wiki with a stale LocalisationCache that is still being updated
at that time.

Bug: T267785
Change-Id: I60ff4eb7dce1fee56470acc177afd29ee14b764f
This commit is contained in:
Timo Tijhof 2020-11-26 02:45:14 +00:00 committed by James D. Forrester
parent 92f5a8520b
commit 8ad97d7c32
6 changed files with 50 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
includes/resourceloader/MessageBlobStore.php
View file

@ -228,6 +228,8 @@ class MessageBlobStore implements LoggerAwareInterface {
$messages = [];
foreach ( $module->getMessages() as $key ) {
$value = $this->fetchMessage( $key, $lang );
// If the message does not exist, omit it from the blob so that
// client-side mw.message may do its own existence handling.
if ( $value !== null ) {
$messages[$key] = $value;
}

14
includes/resourceloader/ResourceLoaderLessVarFileModule.php
View file

@ -108,13 +108,19 @@ class ResourceLoaderLessVarFileModule extends ResourceLoaderFileModule {
* @return array LESS variables
*/
protected function getLessVars( ResourceLoaderContext $context ) {
$vars = parent::getLessVars( $context );
$blob = parent::getMessageBlob( $context );
$messages = $this->pluckFromMessageBlob( $blob, $this->lessVariables );
$vars = parent::getLessVars( $context );
foreach ( $messages as $msgKey => $value ) {
$vars['msg-' . $msgKey] = self::wrapAndEscapeMessage( $value );
// It is important that we iterate the declared list from $this->lessVariables,
// and not $messages since in the case of undefined messages, the key is
// omitted entirely from the blob. This emits a log warning for developers,
// but we must still carry on and produce a valid LESS variable declaration,
// to avoid a LESS syntax error (T267785).
foreach ( $this->lessVariables as $msgKey ) {
$vars['msg-' . $msgKey] = self::wrapAndEscapeMessage( $messages[$msgKey] ?? "⧼${msgKey}⧽" );
}
return $vars;
}
}

3
tests/phpunit/data/less/less-messages-exist.css Normal file
View file

@ -0,0 +1,3 @@
.unit-tests {
content: 'March 14';
}

3
tests/phpunit/data/less/less-messages-nonexist.css Normal file
View file

@ -0,0 +1,3 @@
.unit-tests {
content: '⧼pieday⧽';
}

3
tests/phpunit/data/less/less-messages.less Normal file
View file

@ -0,0 +1,3 @@
.unit-tests {
content: '@{msg-pieday}';
}

29
tests/phpunit/includes/resourceloader/ResourceLoaderLessVarFileModuleTest.php
View file

@ -2,6 +2,7 @@
/**
* @group ResourceLoader
* @covers ResourceLoaderLessVarFileModule
*/
class ResourceLoaderLessVarFileModuleTest extends ResourceLoaderTestCase {
@ -41,4 +42,32 @@ class ResourceLoaderLessVarFileModuleTest extends ResourceLoaderTestCase {
$method->setAccessible( true );
$this->assertEquals( $expected, $method->invoke( null, $msg ) );
}
public function testLessMessagesFound() {
$context = $this->getResourceLoaderContext( 'qqx' );
$basePath = __DIR__ . '/../../data/less';
$module = new ResourceLoaderLessVarFileModule( [
'localBasePath' => $basePath,
'styles' => [ 'less-messages.less' ],
'lessMessages' => [ 'pieday' ],
] );
$module->setMessageBlob( '{"pieday":"March 14"}', 'qqx' );
$styles = $module->getStyles( $context );
$this->assertStringEqualsFile( $basePath . '/less-messages-exist.css', $styles['all'] );
}
public function testLessMessagesFailGraceful() {
$context = $this->getResourceLoaderContext( 'qqx' );
$basePath = __DIR__ . '/../../data/less';
$module = new ResourceLoaderLessVarFileModule( [
'localBasePath' => $basePath,
'styles' => [ 'less-messages.less' ],
'lessMessages' => [ 'pieday' ],
] );
$module->setMessageBlob( '{"something":"Else"}', 'qqx' );
$styles = $module->getStyles( $context );
$this->assertStringEqualsFile( $basePath . '/less-messages-nonexist.css', $styles['all'] );
}
}