r105812 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105811‎ | r105812 | r105813 >
Date:18:46, 11 December 2011
Author:dantman
Status:resolved (Comments)
Tags:
Comment:
Introduce a Language::getHtmlLang method which returns the Bcp47 formatted language code and update code around core to output lang tags using this method instead of getCode so that we properly output things like lang="en-CA" instead of lang="en-ca"
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
——@@ -2244,7 +2244,7 @@
22452245 'cols' => $this->getUser()->getOption( 'cols' ),
22462246 'rows' => $this->getUser()->getOption( 'rows' ),
22472247 'readonly' => 'readonly',
2248 - 'lang' => $pageLang->getCode(),
 2248+ 'lang' => $pageLang->getHtmlCode(),
22492249 'dir' => $pageLang->getDir(),
22502250 );
22512251 $this->addHTML( Html::element( 'textarea', $params, $source ) );
——@@ -2381,7 +2381,7 @@
23822382 $this->addModuleStyles( 'mediawiki.legacy.wikiprintable' );
23832383 }
23842384
2385 - $ret = Html::htmlHeader( array( 'lang' => $this->getLanguage()->getCode(), 'dir' => $userdir, 'class' => 'client-nojs' ) );
 2385+ $ret = Html::htmlHeader( array( 'lang' => $this->getLanguage()->getHtmlCode(), 'dir' => $userdir, 'class' => 'client-nojs' ) );
23862386
23872387 if ( $this->getHTMLTitle() == '' ) {
23882388 $this->setHTMLTitle( $this->msg( 'pagetitle', $this->getPageTitle() ) );
Index: trunk/phase3/includes/SkinTemplate.php
——@@ -288,9 +288,9 @@
289289 $tpl->setRef( 'logopath', $wgLogo );
290290 $tpl->setRef( 'sitename', $wgSitename );
291291
292 - $contentlang = $wgContLang->getCode();
 292+ $contentlang = $wgContLang->getHtmlCode();
293293 $contentdir = $wgContLang->getDir();
294 - $userlang = $this->getLanguage()->getCode();
 294+ $userlang = $this->getLanguage()->getHtmlCode();
295295 $userdir = $this->getLanguage()->getDir();
296296
297297 $tpl->set( 'lang', $userlang );
——@@ -420,7 +420,7 @@
421421 in_array( $request->getVal( 'action', 'view' ), array( 'view', 'historysubmit' ) ) &&
422422 ( $title->exists() || $title->getNamespace() == NS_MEDIAWIKI ) ) {
423423 $pageLang = $title->getPageLanguage();
424 - $realBodyAttribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
 424+ $realBodyAttribs = array( 'lang' => $pageLang->getHtmlCode(), 'dir' => $pageLang->getDir(),
425425 'class' => 'mw-content-'.$pageLang->getDir() );
426426 $out->mBodytext = Html::rawElement( 'div', $realBodyAttribs, $out->mBodytext );
427427 }
Index: trunk/phase3/includes/Skin.php
——@@ -775,7 +775,7 @@
776776 if ( $forContent ) {
777777 $msg = $msgObj->inContentLanguage()->text();
778778 if ( $this->getLanguage()->getCode() !== $wgContLang->getCode() ) {
779 - $msg = Html::rawElement( 'span', array( 'lang' => $wgContLang->getCode(), 'dir' => $wgContLang->getDir() ), $msg );
 779+ $msg = Html::rawElement( 'span', array( 'lang' => $wgContLang->getHtmlCode(), 'dir' => $wgContLang->getDir() ), $msg );
780780 }
781781 return $msg;
782782 } else {
——@@ -1392,7 +1392,7 @@
13931393 }
13941394
13951395 $notice = Html::rawElement( 'div', array( 'id' => 'localNotice',
1396 - 'lang' => $wgContLang->getCode(), 'dir' => $wgContLang->getDir() ), $notice );
 1396+ 'lang' => $wgContLang->getHtmlCode(), 'dir' => $wgContLang->getDir() ), $notice );
13971397 wfProfileOut( __METHOD__ );
13981398 return $notice;
13991399 }
Index: trunk/phase3/languages/Language.php
——@@ -3450,6 +3450,14 @@
34513451 }
34523452
34533453 /**
 3454+ * Get the code in Bcp47 format which we can use
 3455+ * inside of html lang="" tags.
 3456+ */
 3457+ function getHtmlCode() {
 3458+ return wfBcp47( $this->getCode() );
 3459+ }
 3460+
 3461+ /**
34543462 * @param $code string
34553463 */
34563464 function setCode( $code ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r105925Followup r105812:...dantman20:09, 12 December 2011

Comments

#Comment by Nikerabbit (talk | contribs)   19:26, 11 December 2011

@since!

Are you planning to expand the method like in bug 32483?

#Comment by Dantman (talk | contribs)   19:40, 11 December 2011

Not right now, no. The fact we never wfBcp47'ed lang codes the way they are supposed to has just been bothering me for awhile.

This method can be updated later to use whatever method will handle the code conversions when it's added.

#Comment by Hashar (talk | contribs)   11:41, 12 December 2011

wfBcp47 is named with some more upper cased characters: wfBCP47.

Running wfBCP47 should only be done when changing the language code, not on each call.

en-CA and en-ca are both fully valid. BCP 47 does not enforce any case sensitivity so it is really useless to do that kind of formatting.


Anyway, the most important point is: why do you introduce getHtmlCode() when ->mCode is already a valid BCP47 language code? :-)

#Comment by Dantman (talk | contribs)   18:12, 12 December 2011

Because while lowercase is valid for BCP 47 the BCP 47 recommendation is the 'en-CA' form not the 'en-ca' form, and I'm picky about what goes out in our lang="" tags.

#Comment by Dantman (talk | contribs)   20:10, 12 December 2011

Fixed in r105925, however if someone sets mCode directly (it's not private) things will get screwy.

#Comment by Hashar (talk | contribs)   09:04, 13 December 2011

By looking again at the Language code: getCode() is supposed to return a language code according to RFC 3066. That RFC was once BCP47 and got obsoleted with time.

So I think we should drop getHtmlCode() and just normalize mCode when it is set(). That would also avoid having two codes looking exactly the same.

#Comment by Dantman (talk | contribs)   17:50, 13 December 2011

Are we going to rename things like MessagesSimple.php to MessagesEn_x_simple.php when we add in the cleanup of our lang codes? And setup something to deal with transitioning user preferences?

getCode should always return the code that we actually use internally for language files, preferences, etc... otherwise something is likely going to break.

#Comment by Nikerabbit (talk | contribs)   17:53, 13 December 2011

I agree, and isn't mCode already normalised to our internal format?

Status & tagging log