r45512 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r45511‎ | r45512 | r45513 >
Date:19:19, 7 January 2009
Author:raymond
Status:reverted (Comments)
Tags:
Comment:
* Extend language::getLanguageName to return localized language names if available
* Therefore added a hook to catch them from an extension (successfully tested with the cldr extension (see next commit))
* Add a title tag to the interlanguage box entries. Only added when the title differ from the shwon text.
No change of behaviour until an extension is enabled.
Modified paths:
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/skins/Modern.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/MonoBook.php
@@ -334,9 +334,15 @@
335335 <h5><?php $this->msg('otherlanguages') ?></h5>
336336 <div class="pBody">
337337 <ul>
338 -<?php foreach($this->data['language_urls'] as $langlink) { ?>
 338+<?php foreach($this->data['language_urls'] as $langlink) {
 339+ // Add title tag only if differ from shown text
 340+ $titleTag = $langlink['title'] == $langlink['text']
 341+ ? ''
 342+ : 'title="' . htmlspecialchars( $langlink['title'] ) . '"';
 343+ ?>
339344 <li class="<?php echo htmlspecialchars($langlink['class'])?>"><?php
340 - ?><a href="<?php echo htmlspecialchars($langlink['href']) ?>"><?php echo $langlink['text'] ?></a></li>
 345+ ?><a href="<?php echo htmlspecialchars($langlink['href']) ?>"
 346+ <? echo $titleTag ?> > <?php echo $langlink['text'] ?></a></li>
341347 <?php } ?>
342348 </ul>
343349 </div>
Index: trunk/phase3/skins/Modern.php
@@ -332,9 +332,15 @@
333333 <h5><?php $this->msg('otherlanguages') ?></h5>
334334 <div class="pBody">
335335 <ul>
336 -<?php foreach($this->data['language_urls'] as $langlink) { ?>
 336+<?php foreach($this->data['language_urls'] as $langlink) {
 337+ // Add title tag only if differ from shown text
 338+ $titleTag = $langlink['title'] == $langlink['text']
 339+ ? ''
 340+ : 'title="' . htmlspecialchars( $langlink['title'] ) . '"';
 341+ ?>
337342 <li class="<?php echo htmlspecialchars($langlink['class'])?>"><?php
338 - ?><a href="<?php echo htmlspecialchars($langlink['href']) ?>"><?php echo $langlink['text'] ?></a></li>
 343+ ?><a href="<?php echo htmlspecialchars($langlink['href']) ?>"
 344+ <? echo $titleTag ?> > <?php echo $langlink['text'] ?></a></li>
339345 <?php } ?>
340346 </ul>
341347 </div><!-- pBody -->
Index: trunk/phase3/docs/hooks.txt
@@ -783,6 +783,10 @@
784784 &$result: Set this and return false to override the internal checks
785785 $user: User the password is being validated for
786786
 787+'LanguageGetLocalizedLanguageNames': Use to get localized language names
 788+&$languageNames: localized language names (array)
 789+$lang: laguage code (string)
 790+
787791 'LanguageGetMagic': Use this to define synonyms of magic words depending of the language
788792 $magicExtensions: associative array of magic words synonyms
789793 $lang: laguage code (string)
Index: trunk/phase3/includes/SkinTemplate.php
@@ -416,8 +416,13 @@
417417 if ( $nt ) {
418418 $language_urls[] = array(
419419 'href' => $nt->getFullURL(),
420 - 'text' => ($wgContLang->getLanguageName( $nt->getInterwiki()) != ''?$wgContLang->getLanguageName( $nt->getInterwiki()) : $l),
421 - 'class' => $class
 420+ 'text' => ( $wgContLang->getLanguageName( $nt->getInterwiki() ) != ''
 421+ ? $wgContLang->getLanguageName( $nt->getInterwiki() )
 422+ : $l ),
 423+ 'class' => $class,
 424+ 'title' => ( $wgLang->getLanguageNameLocalized( $nt->getInterwiki() ) != ''
 425+ ? $wgLang->getLanguageNameLocalized( $nt->getInterwiki() )
 426+ : $l )
422427 );
423428 }
424429 }
Index: trunk/phase3/languages/Language.php
@@ -419,14 +419,30 @@
420420 return wfMsgExt( $msg, array( 'parsemag', 'language' => $this ) );
421421 }
422422
423 - function getLanguageName( $code ) {
 423+ /**
 424+ * Get a language name
 425+ *
 426+ * @param $code String language code
 427+ * @return $localized boolean gets the localized language name
 428+ */
 429+ function getLanguageName( $code, $localized = false ) {
424430 $names = self::getLanguageNames();
425431 if ( !array_key_exists( $code, $names ) ) {
426432 return '';
427433 }
428 - return $names[$code];
 434+ if( $localized ) {
 435+ $languageNames = array();
 436+ wfRunHooks( 'LanguageGetLocalizedLanguageNames', array( &$languageNames, $this->getCode() ) );
 437+ return isset( $languageNames[$code] ) ? $languageNames[$code] : $names[$code];
 438+ } else {
 439+ return $names[$code];
 440+ }
429441 }
430442
 443+ function getLanguageNameLocalized( $code ) {
 444+ return self::getLanguageName( $code, true );
 445+ }
 446+
431447 function getMonthName( $key ) {
432448 return $this->getMessageFromDB( self::$mMonthMsgs[$key-1] );
433449 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r45515Fix for r45512: cache the result of LanguageGetLocalizedLanguageNames hook so...ialex20:20, 7 January 2009
r45829self revert r45512, r45513 and r45515 for now...raymond08:56, 17 January 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   18:22, 13 January 2009

Breaks compat with short tags off -- a short-form "<?" is used. Haven't fully checked other issues yet.

#Comment by Raymond (talk | contribs)   18:37, 13 January 2009

Related to <? echo $titleTag ?> when $titleTag is empty? Or another line?

Would it hurt if title in the output even if not differ from shown text?

#Comment by Brion VIBBER (talk | contribs)   18:30, 13 January 2009

Several general issues:

  • It seems weird to add a second parameter to getLanguageName() which totally changes its behavior, then make a different function to call it. Just make the separate function and stick with it, maybe?
  • It would be good to factor out common code in the language link list generation; if we're going to change the skins, I'd prefer to go ahead and reduce the amount of code duplication and future changes required
  • The hook seems oddly factored to me. If it's only going to be used to fetch a single item at a time, why copy and pass back a complete array of all known languages every time you call it? May as well just ask for one at a time in the hook.
#Comment by Raymond (talk | contribs)   08:58, 17 January 2009

Reverted with r45829. I need a bit more time to fix it.

Status & tagging log