r69185 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r69184‎ | r69185 | r69186 >
Date:13:34, 8 July 2010
Author:nikerabbit
Status:resolved (Comments)
Tags:
Comment:
Introduce $wgBetterDirectionality that lets us work on support for rtl ui in ltr wiki and vice versa.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/Html.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Vector.php
@@ -682,7 +682,7 @@
683683 if ( !is_array( $elements ) ) {
684684 $elements = array( $elements );
685685 // If there's a series of elements, reverse them when in RTL mode
686 - } else if ( $wgContLang->isRTL() ) {
 686+ } else if ( wfUILang()->isRTL() ) {
687687 $elements = array_reverse( $elements );
688688 }
689689 // Render elements
Index: trunk/phase3/includes/GlobalFunctions.php
@@ -561,6 +561,11 @@
562562 return $wgContLang;
563563 }
564564
 565+function wfUILang() {
 566+ global $wgBetterDirectionality;
 567+ return wfGetLangObj( $wgBetterDirectionality ? false: true );
 568+}
 569+
565570 /**
566571 * Get a message from anywhere, for the current user language.
567572 *
Index: trunk/phase3/includes/OutputPage.php
@@ -2099,9 +2099,7 @@
21002100 }
21012101 $sk->setupUserCss( $this );
21022102
2103 - $dir = $wgContLang->getDir();
2104 - $htmlAttribs = array( 'lang' => $wgContLanguageCode, 'dir' => $dir );
2105 - $ret = Html::htmlHeader( $htmlAttribs );
 2103+ $ret = Html::htmlHeader( array( 'lang' => wfUILang()->getCode() ) );
21062104
21072105 if ( $this->getHTMLTitle() == '' ) {
21082106 $this->setHTMLTitle( wfMsg( 'pagetitle', $this->getPageTitle() ) );
@@ -2156,6 +2154,7 @@
21572155 }
21582156
21592157 # Class bloat
 2158+ $dir = wfUILang()->getDir();
21602159 $bodyAttrs['class'] = "mediawiki $dir";
21612160
21622161 if ( $wgLang->capitalizeAllNouns() ) {
@@ -2427,8 +2426,7 @@
24282427 */
24292428 protected function styleLink( $style, $options ) {
24302429 if( isset( $options['dir'] ) ) {
2431 - global $wgContLang;
2432 - $siteDir = $wgContLang->getDir();
 2430+ $siteDir = wfUILang()->getDir();
24332431 if( $siteDir != $options['dir'] ) {
24342432 return '';
24352433 }
Index: trunk/phase3/includes/Html.php
@@ -609,7 +609,10 @@
610610 $attribs["xmlns:$tag"] = $ns;
611611 }
612612 }
613 - return $ret . Html::openElement( 'html', $attribs ) . "\n";
 613+ $html = Html::openElement( 'html', $attribs );
 614+ if ( $html ) $html .= "\n";
 615+ $ret .= $html;
 616+ return $ret;
614617 }
615618
616619 /**
Index: trunk/phase3/includes/SkinTemplate.php
@@ -297,9 +297,12 @@
298298 $tpl->setRef( 'scriptpath', $wgScriptPath );
299299 $tpl->setRef( 'serverurl', $wgServer );
300300 $tpl->setRef( 'logopath', $wgLogo );
301 - $tpl->setRef( 'lang', $wgContLanguageCode );
302 - $tpl->set( 'dir', $wgContLang->getDir() );
303 - $tpl->set( 'rtl', $wgContLang->isRTL() );
 301+
 302+ $lang = wfUILang();
 303+ $tpl->set( 'lang', $lang->getCode() );
 304+ $tpl->set( 'dir', $lang->getDir() );
 305+ $tpl->set( 'rtl', $lang->isRTL() );
 306+
304307 $tpl->set( 'capitalizeallnouns', $wgLang->capitalizeAllNouns() ? ' capitalize-all-nouns' : '' );
305308 $tpl->set( 'showjumplinks', $wgUser->getOption( 'showjumplinks' ) );
306309 $tpl->set( 'username', $wgUser->isAnon() ? null : $this->username );
@@ -415,6 +418,11 @@
416419 $tpl->set( 'bottomscripts', $this->bottomScripts() );
417420
418421 $printfooter = "<div class=\"printfooter\">\n" . $this->printSource() . "</div>\n";
 422+ global $wgBetterDirectionality;
 423+ if ( $wgBetterDirectionality ) {
 424+ $realBodyAttribs = array( 'lang' => $wgContLanguageCode, 'dir' => $wgContLang->getDir() );
 425+ $out->mBodytext = Html::rawElement( 'div', $realBodyAttribs, $out->mBodytext );
 426+ }
419427 $out->mBodytext .= $printfooter . $this->generateDebugHTML();
420428 $tpl->setRef( 'bodytext', $out->mBodytext );
421429
Index: trunk/phase3/includes/DefaultSettings.php
@@ -2282,6 +2282,12 @@
22832283 */
22842284 $wgEdititis = false;
22852285
 2286+/**
 2287+ * Experimental better directionality support.
 2288+ */
 2289+$wgBetterDirectionality = false;
 2290+
 2291+
22862292 /** @} */ # End of output format settings }
22872293
22882294 /*************************************************************************//**

Follow-up revisions

RevisionCommit summaryAuthorDate
r70412Follow up to r69185: add documentation for wfUILang()nikerabbit20:39, 3 August 2010
r81620Followup to r69185, putting the "dir" attribute back on the HTML root elementrobla03:18, 7 February 2011

Comments

#Comment by Ævar Arnfjörð Bjarmason (talk | contribs)   16:27, 8 July 2010

When should wfUILang() be used and when should you use $wgContLang? This needs better docs.

#Comment by Happy-melon (talk | contribs)   18:04, 17 December 2010
+	return wfGetLangObj( $wgBetterDirectionality ? false: true );

Why not just

+	return wfGetLangObj( !$wgBetterDirectionality );

Your code comment still doesn't really clarify what the function does: returns the Content language by default, but the UI language in 'dev mode', AFAICT. You actually don't want people using this everywhere in the UI, but only with care.

#Comment by Nikerabbit (talk | contribs)   18:08, 17 December 2010

The intention was that we could incrementally change to UI to use wfUILang() until it is good enough to be switched on by default. I've haven't had time to work on it since then, though.

#Comment by Reedy (talk | contribs)   03:31, 6 January 2011

Logic was simplified at some other time...

Does this need to stay FIXME?

#Comment by Happy-melon (talk | contribs)   12:39, 6 January 2011

r70482, to be precise. Fine by me now.

#Comment by Catrope (talk | contribs)   16:37, 8 January 2011
+		$html = Html::openElement( 'html', $attribs );
+		if ( $html ) $html .= "\n";

Why in the world would the return value of Html::openElement evaluate as false? Was this a debugging change or something?

-		$dir = $wgContLang->getDir();
-		$htmlAttribs = array( 'lang' => $wgContLanguageCode, 'dir' => $dir );
-		$ret = Html::htmlHeader( $htmlAttribs );
+		$ret = Html::htmlHeader( array( 'lang' => wfUILang()->getCode() ) );

Did you intentionally remove the dir attribute from the <html> tag here? Why?

#Comment by Nikerabbit (talk | contribs)   17:01, 8 January 2011

With empty attribs in html5 mode it might return empty string or something. The dir attrib seems to be an oversight and should be restored.

#Comment by Catrope (talk | contribs)   22:10, 3 February 2011

Then please restore the dir attr.

#Comment by RobLa-WMF (talk | contribs)   03:25, 7 February 2011

Restored in r81620. I wasn't able to see the effect of r81620 in the browser I tried (Chrome), even having switched the preferred language in my browser to Arabic while leaving the language of the wiki to English.

#Comment by TheDJ (talk | contribs)   10:21, 7 February 2011

I think you didn't see a difference, because all skins now set explicit directionality with CSS styling. I was already wondering why that was needed all of a sudden.

#Comment by Platonides (talk | contribs)   21:30, 30 June 2011

Shouldn't vector be always using $wgLang ?

Status & tagging log