r68904 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r68903‎ | r68904 | r68905 >
Date:19:54, 2 July 2010
Author:btongminh
Status:resolved (Comments)
Tags:
Comment:
(bug 24212) Added MediaWiki:ImagePage.css which is also included on foreign client wikis.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/ImagePage.php (modified) (history)
  • /trunk/phase3/includes/Setup.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/filerepo/FileRepo.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ImagePage.php
@@ -152,6 +152,11 @@
153153 $wgOut->addHTML(
154154 "<script type=\"text/javascript\">attachMetadataToggle('mw_metadata', '$expand', '$collapse');</script>\n" );
155155 }
 156+
 157+ $css = $this->repo->getDescriptionStylesheetUrl();
 158+ if ( $css ) {
 159+ $wgOut->addStyle( $css );
 160+ }
156161 }
157162
158163 public function getRedirectTarget() {
Index: trunk/phase3/includes/Setup.php
@@ -84,6 +84,8 @@
8585 'class' => 'LocalRepo',
8686 'name' => 'local',
8787 'directory' => $wgUploadDirectory,
 88+ 'scriptDirUrl' => $wgScriptPath,
 89+ 'scriptExtension' => $wgScriptExtension,
8890 'url' => $wgUploadBaseUrl ? $wgUploadBaseUrl . $wgUploadPath : $wgUploadPath,
8991 'hashLevels' => $wgHashedUploadDirectory ? 2 : 0,
9092 'thumbScriptUrl' => $wgThumbnailScriptPath,
Index: trunk/phase3/includes/filerepo/FileRepo.php
@@ -372,6 +372,17 @@
373373 }
374374 }
375375 }
 376+
 377+ /**
 378+ * Get the URL of the stylesheet to apply to description pages
 379+ * @return string
 380+ */
 381+ function getDescriptionStylesheetUrl() {
 382+ if ( $this->scriptDirUrl ) {
 383+ return self::makeUrl( 'title=MediaWiki:ImagePage.css&' .
 384+ wfArrayToCGI( Skin::getDynamicStylesheetQuery() ) );
 385+ }
 386+ }
376387
377388 /**
378389 * Store a file to a given destination.
Index: trunk/phase3/includes/Skin.php
@@ -616,11 +616,7 @@
617617 // Per-site custom styles
618618 if( $wgUseSiteCss ) {
619619 global $wgHandheldStyle;
620 - $query = wfArrayToCGI( array(
621 - 'usemsgcache' => 'yes',
622 - 'ctype' => 'text/css',
623 - 'smaxage' => $wgSquidMaxage
624 - ) + $siteargs );
 620+ $query = wfArrayToCGI( self::getDynamicStylesheetQuery() );
625621 # Site settings must override extension css! (bug 15025)
626622 $out->addStyle( self::makeNSUrl( 'Common.css', $query, NS_MEDIAWIKI ) );
627623 $out->addStyle( self::makeNSUrl( 'Print.css', $query, NS_MEDIAWIKI ), 'print' );
@@ -666,6 +662,22 @@
667663
668664 wfProfileOut( __METHOD__ );
669665 }
 666+
 667+ /**
 668+ * Get the query to generate a dynamic stylesheet
 669+ *
 670+ * @return array
 671+ */
 672+ public static function getDynamicStylesheetQuery() {
 673+ global $wgSquidMaxage;
 674+ return array(
 675+ 'action' => 'raw',
 676+ 'maxage' => $wgSquidMaxage,
 677+ 'usemsgcache' => 'yes',
 678+ 'ctype' => 'text/css',
 679+ 'smaxage' => $wgSquidMaxage,
 680+ );
 681+ }
670682
671683 /**
672684 * Add skin specific stylesheets
Index: trunk/phase3/RELEASE-NOTES
@@ -100,6 +100,8 @@
101101 * texvc now supports \bcancel and \xcancel in addition to \cancel and \cancelto
102102 * Added scriptExtension setting to $wgForeignFileRepos
103103 * ForeignApiRepo uses scriptDirUrl if apiBase not set
 104+* (bug 24212) Added MediaWiki:ImagePage.css which is also included on foreign
 105+ client wikis.
104106
105107 === Bug fixes in 1.17 ===
106108 * (bug 17560) Half-broken deletion moved image files to deletion archive

Follow-up revisions

RevisionCommit summaryAuthorDate
r68968Follow-up r68904: Rename 'ImagePage.css' to 'Filepage.css': More generic name...raymond19:51, 3 July 2010
r91031(bug 29277) MediaWiki:Filepage.css not loaded on foreignwiki itself. Fixup to...demon00:08, 29 June 2011

Comments

#Comment by The Evil IP address (talk | contribs)   08:24, 5 July 2010

Should wikis like Commons really have the power to style the file pages of another wiki? I'm not sure they should be allowed this, since that's the decision of the wikis themselves IMHO. Commons users shouldn't force other wikis to use their favorite design if they don't like this.

#Comment by TheDJ (talk | contribs)   13:53, 5 July 2010

Why not ? I mean if local wiki's don't like it, they can always override the CSS right ?

#Comment by Bryan (talk | contribs)   16:40, 5 July 2010

Commons in fact can already do that if they really would want to. Inherent to having a shared repo is that some power is shared as well.


#Comment by The Evil IP address (talk | contribs)   12:53, 6 July 2010

Then, of course, it's not that bad if they could already do so anyway. But of course we need to remember that this doesn't only have good sides, for example a syntax error or someone adding a rule like "body { display: none; }" can do a lot of harm to 800+ wikis.

#Comment by 😂 (talk | contribs)   13:17, 6 July 2010

I'm with the Evil IP on this one. I really don't like the idea of one wiki controlling the styles of another. Sure, wikis can override a specific setting if they don't like it...but is that fair to ask them to do so?

Commons is a shared repo for hundreds of wikis...should their style decisions be forced downstream to hundreds of wikis who then must override Commons' choices if they don't like them?

Also, the body { display:none } vector is annoying on a single wiki...but downright disruptive when it's served to several hundred wikis.

#Comment by Bryan (talk | contribs)   15:00, 6 July 2010

Commons is a shared repo, so it makes perfect sense to have shared image pages styled the same across all wikis that use the repo.

#Comment by P858snake (talk | contribs)   01:01, 8 June 2011

There is a tad of a difference between "oh hey, lets use commons so we don't need to to dup file uploads" and "oh hey, lets give a remote wiki the ability to style content on our wiki"

#Comment by Bryan (talk | contribs)   14:53, 11 June 2011

That is a gross overstatement. The stylesheet is only loaded on image description pages of files that are actually on the foreign wiki. Local files and other pages are not affected.

#Comment by MZMcBride (talk | contribs)   19:30, 11 June 2011

I don't think it's a gross overstatement as much as it is simply imprecise. Peachey88 is correct that on those pages, the foreign wiki would be able to style the page pretty much however it wants. Read with a more narrow focus, I think that's what he meant by "style content." Not all content, no, but those pages could be radically different if the foreign wiki felt used some stupid code.

I agree (with the comment below) that some sort of toggle for this would be nice. I'm not sure what the default value should be, but I think making this configurable would be wise.

#Comment by TheDJ (talk | contribs)   14:21, 26 June 2011

As far as I'm concerned, there are toggles:

If you don't want external sources in 'control' of your wiki 1: Don't use instantcommons/foreignrepo


If you don't want external skinning by external wiki 2: Override in your own Wiki with stronger CSS statements.

#Comment by Platonides (talk | contribs)   23:32, 28 June 2011

Note that the shared repo already has html injection powers if you are fetching its description. But instead of including from there, I think the shared repo should list the headers that should be added. For instance a <source> tag is not properly rendered since the associated CSS is not present on the other sites.

#Comment by MZMcBride (talk | contribs)   15:21, 5 July 2010

Are you sure you like the name "ImagePage.css"? "FilePage.css" or "MediaPage.css" might make more sense.

#Comment by Raymond (talk | contribs)   15:22, 5 July 2010

Please read the follow-up r68968: I changed it to Filepage.css

#Comment by P858snake (talk | contribs)   01:02, 8 June 2011

We should probably have a switch to enable this, this feature doesn't exactly win my heart and I can see opposition to it as well in these comments.

Status & tagging log