r97636 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97635‎ | r97636 | r97637 >
Date:15:55, 20 September 2011
Author:robin
Status:resolved (Comments)
Tags:
Comment:
Re-do several things of r96798 in preparation of re-doing the rest (there's a bug somewhere that needs fixing).
* Do additional validation and is_array() check in LanguageConverter
* Make redirects be in content language again (remove from Title->getPageLanguage())
* Pass title object from ParserCache to ParserOptions->optionsHash
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOptions.php (modified) (history)
  • /trunk/phase3/languages/LanguageConverter.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Article.php
@@ -1050,13 +1050,14 @@
10511051 * @return string containing HMTL with redirect link
10521052 */
10531053 public function viewRedirect( $target, $appendSubtitle = true, $forceKnown = false ) {
1054 - global $wgOut, $wgLang, $wgStylePath;
 1054+ global $wgOut, $wgStylePath;
10551055
10561056 if ( !is_array( $target ) ) {
10571057 $target = array( $target );
10581058 }
10591059
1060 - $imageDir = $wgLang->getDir();
 1060+ $lang = $this->getTitle()->getPageLanguage();
 1061+ $imageDir = $lang->getDir();
10611062
10621063 if ( $appendSubtitle ) {
10631064 $wgOut->appendSubtitle( wfMsgHtml( 'redirectpagesub' ) );
@@ -1072,7 +1073,7 @@
10731074 }
10741075
10751076 $nextRedirect = $wgStylePath . '/common/images/nextredirect' . $imageDir . '.png';
1076 - $alt = $wgLang->isRTL() ? '←' : '→';
 1077+ $alt = $lang->isRTL() ? '←' : '→';
10771078 // Automatically append redirect=no to each link, since most of them are redirect pages themselves.
10781079 foreach ( $target as $rt ) {
10791080 $link .= Html::element( 'img', array( 'src' => $nextRedirect, 'alt' => $alt ) );
Index: trunk/phase3/includes/parser/ParserOptions.php
@@ -274,9 +274,11 @@
275275 * settings.
276276 *
277277 * @since 1.17
 278+ * @param $forOptions Array
 279+ * @param $title Title: will be used to get the page content language
278280 * @return \string Page rendering hash
279281 */
280 - public function optionsHash( $forOptions ) {
 282+ public function optionsHash( $forOptions, $title = null ) {
281283 global $wgContLang, $wgRenderHashAppend;
282284
283285 $confstr = '';
Index: trunk/phase3/includes/Title.php
@@ -4376,9 +4376,6 @@
43774377 if ( $this->getNamespace() == NS_SPECIAL ) {
43784378 // special pages are in the user language
43794379 return $wgLang;
4380 - } elseif ( $this->isRedirect() ) {
4381 - // the arrow on a redirect page is aligned according to the user language
4382 - return $wgLang;
43834380 } elseif ( $this->isCssOrJsPage() ) {
43844381 // css/js should always be LTR and is, in fact, English
43854382 return wfGetLangObj( 'en' );
Index: trunk/phase3/languages/LanguageConverter.php
@@ -158,7 +158,7 @@
159159 // not memoized (i.e. there return value is not cached) since
160160 // new information might appear during processing after this
161161 // is first called.
162 - if ( $req ) {
 162+ if ( $this->validateVariant( $req ) ) {
163163 return $req;
164164 }
165165 return $this->mMainLanguageCode;
@@ -1369,14 +1369,16 @@
13701370 // then we check its fallback variants.
13711371 $variantFallbacks =
13721372 $this->mConverter->getVariantFallbacks( $variant );
1373 - foreach ( $variantFallbacks as $variantFallback ) {
1374 - // if current variant's fallback exist in flags
1375 - if ( isset( $this->mVariantFlags[$variantFallback] ) ) {
1376 - // then convert <text to convert> to fallback language
1377 - $this->mRules =
1378 - $this->mConverter->autoConvert( $this->mRules,
1379 - $variantFallback );
1380 - break;
 1373+ if( is_array( $variantFallbacks ) ) {
 1374+ foreach ( $variantFallbacks as $variantFallback ) {
 1375+ // if current variant's fallback exist in flags
 1376+ if ( isset( $this->mVariantFlags[$variantFallback] ) ) {
 1377+ // then convert <text to convert> to fallback language
 1378+ $this->mRules =
 1379+ $this->mConverter->autoConvert( $this->mRules,
 1380+ $variantFallback );
 1381+ break;
 1382+ }
13811383 }
13821384 }
13831385 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r97637r97636: ParserCache meant to be committed already toorobin15:58, 20 September 2011
r97757* follow-up r97636: decrease indentation & mention revision per Nikerabbit...robin19:30, 21 September 2011
r97849Re-do r96798 ("LanguageConverter now depends on the page content language"), ...robin20:31, 22 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r96798(Bug 30364) LanguageConverter should depend on the page content language inst...robin17:56, 11 September 2011

Comments

#Comment by Nikerabbit (talk | contribs)   08:23, 21 September 2011
+							$this->mRules =
+								$this->mConverter->autoConvert( $this->mRules,
+																$variantFallback );

Can you do something about that huge indentation?

#Comment by Nikerabbit (talk | contribs)   08:25, 21 September 2011

In optionsHash it would be nice to document in what revision the new param was introduced.

#Comment by SPQRobin (talk | contribs)   19:31, 21 September 2011

Both done in r97757.

Status & tagging log