r81528 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81527‎ | r81528 | r81529 >
Date:16:52, 4 February 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
Use wfMessage() instead of wfMsgGetKey()
Modified paths:
  • /trunk/phase3/includes/media/MediaTransformOutput.php (modified) (history)
  • /trunk/phase3/includes/parser/CoreParserFunctions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialAllmessages.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/CoreParserFunctions.php
——@@ -86,8 +86,7 @@
8787 static function intFunction( $parser, $part1 = '' /*, ... */ ) {
8888 if ( strval( $part1 ) !== '' ) {
8989 $args = array_slice( func_get_args(), 2 );
90 - $message = wfMsgGetKey( $part1, true, $parser->getOptions()->getUserLang(), false );
91 - $message = wfMsgReplaceArgs( $message, $args );
 90+ $message = wfMessage( $part1, $args )->inLanguage( $parser->getOptions()->getUserLang() )->plain();
9291 $message = $parser->replaceVariables( $message ); // like MessageCache::transform()
9392 return $message;
9493 } else {
Index: trunk/phase3/includes/media/MediaTransformOutput.php
——@@ -212,8 +212,8 @@
213213 $htmlArgs = array_map( 'htmlspecialchars', $args );
214214 $htmlArgs = array_map( 'nl2br', $htmlArgs );
215215
216 - $this->htmlMsg = wfMsgReplaceArgs( htmlspecialchars( wfMsgGetKey( $msg, true ) ), $htmlArgs );
217 - $this->textMsg = wfMsgReal( $msg, $args );
 216+ $this->htmlMsg = wfMessage( $msg )->rawParams( $htmlArgs )->escaped();
 217+ $this->textMsg = wfMessage( $msg )->rawParams( $htmlArgs )->text();
218218 $this->width = intval( $width );
219219 $this->height = intval( $height );
220220 $this->url = false;
Index: trunk/phase3/includes/specials/SpecialAllmessages.php
——@@ -265,10 +265,12 @@
266266 ( $descending && ( $key < $offset || !$offset ) || !$descending && $key > $offset ) &&
267267 ( ( $this->prefix && preg_match( $this->prefix, $key ) ) || $this->prefix === false )
268268 ){
 269+ $actual = wfMessage( $key )->inLanguage( $this->langcode )->plain();
 270+ $default = wfMessage( $key )->inLanguage( $this->langcode )->useDatabase( false )->plain();
269271 $result->result[] = array(
270272 'am_title' => $key,
271 - 'am_actual' => wfMsgGetKey( $key, /*useDB*/true, $this->langcode, false ),
272 - 'am_default' => wfMsgGetKey( $key, /*useDB*/false, $this->langcode, false ),
 273+ 'am_actual' => $actual,
 274+ 'am_default' => $default,
273275 'am_customised' => $customised,
274276 'am_talk_exists' => isset( $statuses['talks'][$key] )
275277 );

Follow-up revisions

RevisionCommit summaryAuthorDate
r82501Fix ArticleTablesTest after r81528. It needs to set both $wgLanguageCode and ...platonides00:44, 20 February 2011

Comments

#Comment by Platonides (talk | contribs)   23:25, 18 February 2011

Breaks ArticleTablesTest::testbug14404 It now seems to be checking $wgLanguageCode, not $wgContLang which is what the test varies (both variables would be equivalent in a real wiki).

#Comment by IAlex (talk | contribs)   12:27, 19 February 2011

The ParserOptions has its user language set to $wgLanguageCode when saving a page; the difference comes from this:

  • wfMsgGetKey() calls wfGetLangObj() that checks for $wgLanguageCode (whatever its value is, 'fr' in my case) and return $wgContLang (Spanish).
  • Message::inLanguage() calls Language::factory() directly when receiving a string and you get the language you set in LocalSettings.php.

This means the test passed "by accident" before this revision. The test must set $wgLanguageCode appropriately; otherwise this makes the test pass when the content language defined in LocalSettings.php is "es" but fail otherwise.

#Comment by Platonides (talk | contribs)   00:47, 20 February 2011

Thanks for the pointers. I have reviewed and Message::inLanguage() does work correctly. I had thought that perhaps it did not use $wgContLang when available. Turned out it was better than wfGetLangObj()! I fixed the test in r82501.

This shows how much do people run the tests.

#Comment by Platonides (talk | contribs)   22:57, 20 February 2011
The test must set $wgLanguageCode appropriately; otherwise this makes the test pass when the content language defined in LocalSettings.php is "es" but fail otherwise

No, it worked before. It's funny. The article will set the ParserOptions to $wgLanguageCode (as part of the fix that it was supposedly testing). Then it compared the ParserOptions language to $wgLanguageCode, and returned $wgContLang which was a different one.

#Comment by Mormegil (talk | contribs)   15:45, 10 December 2012

See https://bugzilla.wikimedia.org/show_bug.cgi?id=42914

Status & tagging log