resourceloader: Fix broken getRequest/getDirection in derived context

getDirection() isn't a simple getter value like the others. It actually
is tightly coupled with getLanguage() and lazy-initialised.

When calling setLanguage(), we shouldn't reset direction back to the
parent class but make sure getDirection() will recompute it based
on the local value.

Added regression test (which fails without this patch).

The parent getDirection() looks for $this->request, but the subclass
doesn't assign that member in the constructor. getRequest() forwards
it accordingly, so make sure getRequest() is also used internally.

Change-Id: Ifec703647368c3bb58748288ed754aaaf3730e19

diff --git a/includes/resourceloader/DerivativeResourceLoaderContext.php b/includes/resourceloader/DerivativeResourceLoaderContext.php
index 85d5434..5967537 100644 (file)
--- a/includes/resourceloader/DerivativeResourceLoaderContext.php
+++ b/includes/resourceloader/DerivativeResourceLoaderContext.php
@@ -76,7 +76,7 @@ class DerivativeResourceLoaderContext extends ResourceLoaderContext {
        public function setLanguage( $language ) {
                $this->language = $language;
                // Invalidate direction since it is based on language
-               $this->direction = self::INHERIT_VALUE;
+               $this->direction = null;
                $this->hash = null;
        }
 
@@ -84,6 +84,9 @@ class DerivativeResourceLoaderContext extends ResourceLoaderContext {
                if ( $this->direction === self::INHERIT_VALUE ) {
                        return $this->context->getDirection();
                }
+               if ( $this->direction === null ) {
+                       $this->direction = Language::factory( $this->getLanguage() )->getDir();
+               }
                return $this->direction;
        }
 

diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php
index a22a437..2c52c2e 100644 (file)
--- a/includes/resourceloader/ResourceLoaderContext.php
+++ b/includes/resourceloader/ResourceLoaderContext.php
@@ -160,7 +160,7 @@ class ResourceLoaderContext {
        public function getLanguage() {
                if ( $this->language === null ) {
                        // Must be a valid language code after this point (bug 62849)
-                       $this->language = RequestContext::sanitizeLangCode( $this->request->getVal( 'lang' ) );
+                       $this->language = RequestContext::sanitizeLangCode( $this->getRequest()->getVal( 'lang' ) );
                }
                return $this->language;
        }
@@ -170,7 +170,7 @@ class ResourceLoaderContext {
         */
        public function getDirection() {
                if ( $this->direction === null ) {
-                       $this->direction = $this->request->getVal( 'dir' );
+                       $this->direction = $this->getRequest()->getVal( 'dir' );
                        if ( !$this->direction ) {
                                // Determine directionality based on user language (bug 6100)
                                $this->direction = Language::factory( $this->getLanguage() )->getDir();

diff --git a/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php b/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php
index 08c340e..0d11f62 100644 (file)
--- a/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php
+++ b/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php
@@ -34,6 +34,9 @@ class DerivativeResourceLoaderContextTest extends PHPUnit_Framework_TestCase {
 
                $derived->setLanguage( 'nl' );
                $this->assertEquals( $derived->getLanguage(), 'nl' );
+
+               $derived->setLanguage( 'he' );
+               $this->assertEquals( $derived->getDirection(), 'rtl' );
        }
 
        public function testSetModules() {