r107860 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r107859‎ | r107860 | r107861 >
Date:01:58, 3 January 2012
Author:dantman
Status:ok (Comments)
Tags:
Comment:
Add RequestContextCreateSkin hook to allow extensions to override the loading of skins.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/context/RequestContext.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -1559,6 +1559,11 @@
15601560 'RecentChange_save': called at the end of RecentChange::save()
15611561 $recentChange: RecentChange object
15621562
 1563+'RequestContextCreateSkin': Called when RequestContext::getSkin creates a skin instance.
 1564+Can be used by an extension override what skin is used in certain contexts.
 1565+IContextSource $context: The RequestContext the skin is being created for.
 1566+&$skin: A variable reference you may set a Skin instance or string key on to override the skin that will be used for the context.
 1567+
15631568 'ResourceLoaderGetConfigVars': called at the end of
15641569 ResourceLoaderStartUpModule::getConfig(). Use this to export static
15651570 configuration variables to JavaScript. Things that depend on the current
Index: trunk/phase3/RELEASE-NOTES-1.19
@@ -241,6 +241,8 @@
242242 * (bug 31759) Undefined property notice in querypages API.
243243 * (bug 32495) API should allow purge by pageids.
244244 * (bug 33147) API examples should explain what they do.
 245+* Extensions can use the RequestContextCreateSkin hook to override what skin is
 246+ loaded in some contexts.
245247
246248 === Languages updated in 1.19 ===
247249
Index: trunk/phase3/includes/context/RequestContext.php
@@ -248,18 +248,34 @@
249249 if ( $this->skin === null ) {
250250 wfProfileIn( __METHOD__ . '-createskin' );
251251
252 - global $wgHiddenPrefs;
253 - if( !in_array( 'skin', $wgHiddenPrefs ) ) {
254 - # get the user skin
255 - $userSkin = $this->getUser()->getOption( 'skin' );
256 - $userSkin = $this->getRequest()->getVal( 'useskin', $userSkin );
257 - } else {
258 - # if we're not allowing users to override, then use the default
259 - global $wgDefaultSkin;
260 - $userSkin = $wgDefaultSkin;
 252+ $skin = null;
 253+ wfRunHooks( 'RequestContextCreateSkin', array( $this, &$skin ) );
 254+
 255+ // If the hook worked try to set a skin from it
 256+ if ( $skin instanceof Skin ) {
 257+ $this->skin = $skin;
 258+ } elseif ( is_string($skin) ) {
 259+ $this->skin = Skin::newFromKey( $skin );
261260 }
262261
263 - $this->skin = Skin::newFromKey( $userSkin );
 262+ // If this is still null (the hook didn't run or didn't work)
 263+ // then go through the normal processing to load a skin
 264+ if ( $this->skin === null ) {
 265+ global $wgHiddenPrefs;
 266+ if( !in_array( 'skin', $wgHiddenPrefs ) ) {
 267+ # get the user skin
 268+ $userSkin = $this->getUser()->getOption( 'skin' );
 269+ $userSkin = $this->getRequest()->getVal( 'useskin', $userSkin );
 270+ } else {
 271+ # if we're not allowing users to override, then use the default
 272+ global $wgDefaultSkin;
 273+ $userSkin = $wgDefaultSkin;
 274+ }
 275+
 276+ $this->skin = Skin::newFromKey( $userSkin );
 277+ }
 278+
 279+ // After all that set a context on whatever skin got created
264280 $this->skin->setContext( $this );
265281 wfProfileOut( __METHOD__ . '-createskin' );
266282 }

Comments

#Comment by Brion VIBBER (talk | contribs)   20:23, 3 January 2012

I'd recommend against this without a good reason; we should probably be moving more consistently towards allowing a consistent skin to sit in the background and have page contents change within it, making it less likely we'd want to swap skins around willy-nilly.

#Comment by Dantman (talk | contribs)   22:24, 3 January 2012

I've seen some sites in the past that have wanted to use a separate skin on their mainpage, etc... anyone doing that has resorted to core hacks I'd rather not see.

Though the key thing on my mind right now is mobile. Currently our MobileFrontend actually works by having the ENTIRE skin rendered, assuming that the skin works in a MonoBook/Vector like way, and then ripping the #content block out of the rendered skin output and using it. That's an absolutely horrible way to write Mobile code, and it looks like the inability to hook in and have some SkinMobile class loaded when displaying the mobile site is the reason MobileFrontend resorts to such a horrible way of working.

#Comment by Brion VIBBER (talk | contribs)   22:35, 3 January 2012

MobileFrontend is a port of a proxy-based postprocessor so isn't a Skin subclass at all.

#Comment by Brion VIBBER (talk | contribs)   22:38, 3 January 2012

Ok, this looks fairly reasonable on further reading and may be a useful hook for a future Skin-based MobileFrontend variant. Withdrawing objections. :D

Status & tagging log