Remove the need for skin classes to have a hardcoded string as
skinname property value. This previously created the possibility
for the value to not match the skinname in the SkinFactory registry,
which creates confusing situations where message keys and load.php
urls are crafted with the internal skinname, but all other
handling (useskin, preferences, hooks, SkinFactory, ResourceLoader,
etc.) operate on the names in the registry.
We could enforce the matching by requiring a 1:1 relationship between
skinnames and Skin sub classes, but that is not backwards-compatible
with the 1:many map that wgValidSkinNames provides, and not compatible
SkinFactory either, which supports a factory function to return an
object. This makes a lot of sense and allows Skin-classees to be
re-used and composed with injection. If we do want to enforce 1:1,
we could validate it with a structure PHPUnit test, but instead this
change just uses the injected name from the constructor (passed by
ServiceWiring, previously unused).
The added unit test shows the new behaviour. Before this change,
getSkinName() on SkinFallback would always return 'fallback',
whereas now each instance of the class adheres to the registered
name (if it differs from the default).
Update the two direct uses of protected $skin->skinname to use
$skin->getSkinName() instead to enable sub-classes to optionally
implement an alternate source for the self-name (or to hardcode
it there as before).
Bug: T173546
Change-Id: I4383dcc3094da6e3c9ac12dc6c9311128db9db6e
* @ingroup Skins
*/
abstract class Skin extends ContextSource {
+ /**
+ * @var string|null
+ */
protected $skinname = null;
+
protected $mRelevantTitle = null;
protected $mRelevantUser = null;
}
/**
- * @return string Skin name
+ * @since 1.31
+ * @param string|null $skinname
+ */
+ public function __construct( $skinname = null ) {
+ if ( is_string( $skinname ) ) {
+ $this->skinname = $skinname;
+ }
+ }
+
+ /**
+ * @return string|null Skin name
*/
public function getSkinName() {
return $this->skinname;
$skin = $factory->makeSkin( 'testfallback' );
$this->assertInstanceOf( 'Skin', $skin );
$this->assertInstanceOf( 'SkinFallback', $skin );
+ $this->assertEquals( 'fallback', $skin->getSkinName() );
+ }
+
+ /**
+ * @covers Skin::__constructor
+ * @covers Skin::getSkinName
+ */
+ public function testGetSkinName() {
+ $skin = new SkinFallback();
+ $this->assertEquals( 'fallback', $skin->getSkinName(), 'Default' );
+ $skin = new SkinFallback( 'testname' );
+ $this->assertEquals( 'testname', $skin->getSkinName(), 'Constructor argument' );
}
/**