r91031 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r91030‎ | r91031 | r91032 >
Date:00:08, 29 June 2011
Author:demon
Status:ok (Comments)
Tags:
Comment:
(bug 29277) MediaWiki:Filepage.css not loaded on foreignwiki itself. Fixup to r68904. Yay Roan finally taught me how to use the resourceloader :D
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/ImagePage.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderFilePageModule.php (added) (history)
  • /trunk/phase3/resources/Resources.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
@@ -129,6 +129,7 @@
130130 namespaces (categories) when combined with $wgRawHtml.
131131 * Use content language in formatting of dates in revertpage message
132132 (rollback revert edit summary) and do not adjust for user timezone.
 133+* (bug 29277) MediaWiki:Filepage.css is also shown on the local wiki
133134
134135 === API changes in 1.19 ===
135136 * BREAKING CHANGE: action=watch now requires POST and token.
Index: trunk/phase3/includes/ImagePage.php
@@ -12,6 +12,9 @@
1313 */
1414 private $img;
1515 private $displayImg;
 16+ /**
 17+ * @var FileRepo
 18+ */
1619 private $repo;
1720 private $fileLoaded;
1821
@@ -161,11 +164,16 @@
162165 $wgOut->addWikiText( $this->makeMetadataTable( $formattedMetadata ) );
163166 $wgOut->addModules( array( 'mediawiki.action.view.metadata' ) );
164167 }
165 -
166 - $css = $this->repo->getDescriptionStylesheetUrl();
167 - if ( $css ) {
168 - $wgOut->addStyle( $css );
 168+
 169+ // Add remote Filepage.css
 170+ if( !$this->repo->isLocal() ) {
 171+ $css = $this->repo->getDescriptionStylesheetUrl();
 172+ if ( $css ) {
 173+ $wgOut->addStyle( $css );
 174+ }
169175 }
 176+ // always show the local local Filepage.css, bug 29277
 177+ $wgOut->addModuleStyles( 'filepage' );
170178 }
171179
172180 public function getRedirectTarget() {
Index: trunk/phase3/includes/resourceloader/ResourceLoaderFilePageModule.php
@@ -0,0 +1,11 @@
 2+<?php
 3+/*
 4+ * ResourceLoader definition for MediaWiki:Filepage.css
 5+ */
 6+class ResourceLoaderFilePageModule extends ResourceLoaderWikiModule {
 7+ protected function getPages( ResourceLoaderContext $context ) {
 8+ return array(
 9+ 'MediaWiki:Filepage.css' => array( 'type' => 'style' ),
 10+ );
 11+ }
 12+}
Property changes on: trunk/phase3/includes/resourceloader/ResourceLoaderFilePageModule.php
___________________________________________________________________
Added: svn:eol-style
113 + native
Index: trunk/phase3/includes/AutoLoader.php
@@ -635,6 +635,7 @@
636636 'ResourceLoader' => 'includes/resourceloader/ResourceLoader.php',
637637 'ResourceLoaderContext' => 'includes/resourceloader/ResourceLoaderContext.php',
638638 'ResourceLoaderFileModule' => 'includes/resourceloader/ResourceLoaderFileModule.php',
 639+ 'ResourceLoaderFilePageModule' => 'includes/resourceloader/ResourceLoaderFilePageModule.php',
639640 'ResourceLoaderModule' => 'includes/resourceloader/ResourceLoaderModule.php',
640641 'ResourceLoaderNoscriptModule' => 'includes/resourceloader/ResourceLoaderNoscriptModule.php',
641642 'ResourceLoaderSiteModule' => 'includes/resourceloader/ResourceLoaderSiteModule.php',
Index: trunk/phase3/resources/Resources.php
@@ -11,6 +11,7 @@
1212 'user.groups' => array( 'class' => 'ResourceLoaderUserGroupsModule' ),
1313 'user.options' => array( 'class' => 'ResourceLoaderUserOptionsModule' ),
1414 'user.tokens' => array( 'class' => 'ResourceLoaderUserTokensModule' ),
 15+ 'filepage' => array( 'class' => 'ResourceLoaderFilePageModule' ),
1516
1617 /* Skins */
1718

Follow-up revisions

RevisionCommit summaryAuthorDate
r91036MFT r91031: filepage.css fixesdemon00:37, 29 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r68904(bug 24212) Added MediaWiki:ImagePage.css which is also included on foreign c...btongminh19:54, 2 July 2010

Comments

#Comment by Krinkle (talk | contribs)   00:25, 29 June 2011

Since it's a regression afaig, tagging for backport (to 1.17wmf1 asap)

#Comment by 😂 (talk | contribs)   00:27, 29 June 2011

Not really a regression, but should be backported since it was a problem with the original implementation.

#Comment by Krinkle (talk | contribs)   00:37, 29 June 2011

(I'm re-marking this OK assuming your comment submission reset it to new by accident. Revert if it was on purpose)

#Comment by 😂 (talk | contribs)   00:38, 29 June 2011

Yeah, no edit conflicts on CR ;-)

#Comment by Bryan (talk | contribs)   07:34, 29 June 2011

That is one way to fix it, but I'm not sure if it's the proper one. The original bug was really WMF specific, because WMF does not have 'scriptUrl' set in $wgLocalFileRepo, causing FileRepo::getDescriptionStylesheetUrl() to return null. I would personally prefer to just fix WMF's definition of $wgLocalFileRepo.

#Comment by 😂 (talk | contribs)   11:31, 29 June 2011

The original way wasn't correct IMO because it was loading the stylesheet via an older style index.php call. This way it's resourceloaderified when it's on the local wiki.

#Comment by Krinkle (talk | contribs)   16:19, 29 June 2011

There's two seperate things:

  • Using resourceloader
  • it not being loaded on the repository itself

It it still using an index.php call on the remote wikis (as it probably should for now) to load it from commons. But on Commons itself it now uses ResourceLoader (which is itself probably a good thing, saves an additional request). But it does evade the cause of bug 29277, which still needs to be taken care of as a good practice.

#Comment by 😂 (talk | contribs)   16:40, 29 June 2011

I went ahead and set scriptDirUrl for $wgLocalFileRepo in CommonSettings :)

#Comment by Tim Starling (talk | contribs)   07:25, 6 July 2011

The impact on non-Wikimedia websites seems a bit too small to bother backporting it.

#Comment by Krinkle (talk | contribs)   00:11, 28 July 2011

This also fixed bug 28084 (Create Filepage.css module and use it)

#Comment by Krinkle (talk | contribs)   06:02, 4 August 2011

Also fixed bug 28083.

Status & tagging log