r87212 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r87211‎ | r87212 | r87213 >
Date:19:46, 1 May 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
Added wgIsMainPage (Title->isMainPage) to mw.config
* Instead of ugly javascript construction to compare href-attributes or re-constructing proper pagenames, let's use Title->isMainPage which does this much better
* Kept function for compatibility. mw.util.isMainPage() was never released, should probably be removed before 1.18 branch point.
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.util/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
——@@ -2662,6 +2662,7 @@
26632663 'wgUserGroups' => $this->getUser()->getEffectiveGroups(),
26642664 'wgCategories' => $this->getCategories(),
26652665 'wgBreakFrames' => $this->getFrameOptions() == 'DENY',
 2666+ 'wgIsMainPage' => $title->isMainPage(),
26662667 );
26672668 if ( $wgContLang->hasVariants() ) {
26682669 $vars['wgUserVariant'] = $wgContLang->getPreferredVariant();
Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.js
——@@ -272,32 +272,12 @@
273273
274274 /**
275275 * Checks wether the current page is the wiki's main page.
276 - * This function requires the document to be ready!
277276 *
278 - * @param alsoRelated Boolean value, if true this function also returns true if the current page is
279 - * in an associated namespace page of the main page rather than the main page itself (eg. talk page)
280277 * @return Boolean
 278+ * @deprecated to be removed in 1.18: Use wgIsMainPage in mw.config instead.
281279 */
282 - 'isMainPage' : function( alsoRelated ) {
283 - var isRelatedToMainpage = false;
284 -
285 - // Don't insert colon between namespace and title if the namespace is empty (eg. main namespace)
286 - var namespace = mw.config.get( 'wgFormattedNamespaces' )[mw.config.get( 'wgNamespaceNumber' )];
287 - namespace = namespace ? namespace + ':' : '';
288 -
289 - // We can't use (wgMainPageTitle == wgPageName) since the latter is escaped (underscores) and has other
290 - // slight variations that make comparison harder.
291 - var isTheMainPage = mw.config.get( 'wgMainPageTitle' ) === ( namespace + mw.config.get( 'wgTitle' ) );
292 -
293 - // Also check for the title in related namespaces ?
294 - if ( typeof alsoRelated !== 'undefined' && alsoRelated === true ) {
295 - var tabLink = $( '#ca-talk' ).prev().find( 'a:first' ).attr( 'href' );
296 - isRelatedToMainpage = tabLink === mw.util.wikiGetlink( mw.config.get( 'wgMainPageTitle' ) );
297 -
298 - return isRelatedToMainpage || isTheMainPage;
299 - }
300 -
301 - return isTheMainPage;
 280+ 'isMainPage' : function() {
 281+ return mw.config.get( 'wgIsMainPage' );
302282 },
303283
304284

Follow-up revisions

RevisionCommit summaryAuthorDate
r90099Only define 'isMainPage' if we are on the main page, the new mw.config librar...krinkle23:29, 14 June 2011
r90102Using true instead of calling isMainPage() again....krinkle01:11, 15 June 2011

Comments

#Comment by Duplicatebug (talk | contribs)   10:07, 7 May 2011

isMainPage() handles also "isRelatedToMainpage", which is now missing

#Comment by Krinkle (talk | contribs)   13:03, 7 May 2011

That's correct. However this method was never released in any stable release, or any beta or release-candidate. So compatibility should not be an issue as much.

Besides, the isRelatedToMainpage was buggy at best, and the way I did it wasn't very pretty either. Nor was it requested via BugZilla, if you think such functionality would be useful, feel free to open a ticket and be sure to refer to this revision and leave an example usecase as well :). Thanks!

#Comment by Duplicatebug (talk | contribs)   20:12, 7 May 2011

No problem, I do not think, it is necessary.

#Comment by Brion VIBBER (talk | contribs)   23:17, 7 June 2011

Honestly, exposing 'wgIsMainPage' seems.... kinda lame and pollutes the global var space. How about if only the page that *is* the main page actually bothers to export information saying 'by the way I AM the main page'?

#Comment by Brion VIBBER (talk | contribs)   00:20, 8 June 2011

(I'd rather keep the function and not the variable. If the variable's needed to feed the function, the variable should be private or hidden.)

#Comment by Krinkle (talk | contribs)   00:26, 8 June 2011

The global space is only legacy name. It's deprecated as of 1.17 since we're exporting everything as a Map in mw.config, which, for now, stashes it's key/value pairs onto window but there's a configuration option for this already and I guess we're gonna flip it some time soon in trunk (1.20?).

Anyway, mw.config defaults to null if the key is unknown (and throw ReferenceErrors exceptions about undefined variables), so in a way it's already "a function that uses a private variable that is only defined on the main page". mw.config either returns that variable or null. 

if ( mw.config.get( 'isMainPage' ) { ..

If you think that's good enough I'll go ahead and make the change.

#Comment by Krinkle (talk | contribs)   00:27, 8 June 2011

that should be "and never throws ReferenceErrors exceptions about undefined variables" (in contrary to normal global variables)

#Comment by Krinkle (talk | contribs)   23:31, 14 June 2011

Okay done in r90099.

  • Server only outputs the variable if the current page actually is the main page
  • mediaWiki.config instance of Map takes care of the handling when it's not defined.
#Comment by Brion VIBBER (talk | contribs)   17:43, 21 June 2011

'good enough' says me. :) Marking resolved.

Status & tagging log