r90743 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90742‎ | r90743 | r90744 >
Date:23:01, 24 June 2011
Author:robin
Status:resolved (Comments)
Tags:
Comment:
Directionality improvements as part of bug 6100 (under $wgBetterDirectionality):
* Add CSS that should fix all LTR/LTR text on both LTR/RTL wikis, for the editsection link, TOC, and lists (ul/ol).
* Add a class mw-content-ltr/rtl for that.
* Change the divs on CategoryPage so it works wit that CSS.
This is the last relatively major issue for bug 6100.
Modified paths:
  • /trunk/phase3/includes/CategoryPage.php (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/ImagePage.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/skins/common/shared.css (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ImagePage.php
——@@ -117,7 +117,8 @@
118118 # but this section (the actual wikitext) should be in page content language
119119 $pageLang = $this->mTitle->getPageLanguage();
120120 $wgOut->addHTML( Xml::openElement( 'div', array( 'id' => 'mw-imagepage-content',
121 - 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir() ) ) );
 121+ 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
 122+ 'class' => 'mw-content-'.$pageLang->getDir() ) ) );
122123 parent::view();
123124 $wgOut->addHTML( Xml::closeElement( 'div' ) );
124125 } else {
Index: trunk/phase3/includes/SkinTemplate.php
——@@ -462,7 +462,8 @@
463463 in_array( $action, array( 'view', 'render', 'print' ) ) &&
464464 ( $this->getTitle()->exists() || $this->getTitle()->getNamespace() == NS_MEDIAWIKI ) ) {
465465 $pageLang = $this->getTitle()->getPageLanguage();
466 - $realBodyAttribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir() );
 466+ $realBodyAttribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
 467+ 'class' => 'mw-content-'.$pageLang->getDir() );
467468 $out->mBodytext = Html::rawElement( 'div', $realBodyAttribs, $out->mBodytext );
468469 }
469470 }
Index: trunk/phase3/includes/EditPage.php
——@@ -2098,7 +2098,8 @@
20992099 global $wgBetterDirectionality;
21002100 if( $wgBetterDirectionality ) {
21012101 $pageLang = $this->mTitle->getPageLanguage();
2102 - $attribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir() );
 2102+ $attribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
 2103+ 'class' => 'mw-content-'.$pageLang->getDir() );
21032104 $previewHTML = Html::rawElement( 'div', $attribs, $previewHTML );
21042105 }
21052106 wfProfileOut( __METHOD__ );
Index: trunk/phase3/includes/CategoryPage.php
——@@ -179,10 +179,13 @@
180180 $r = wfMsgExt( 'category-empty', array( 'parse' ) );
181181 }
182182
183 - global $wgBetterDirectionality, $wgLang;
 183+ global $wgBetterDirectionality, $wgTitle;
184184 if( $wgBetterDirectionality ) {
185 - $langAttribs = array( 'lang' => $wgLang->getCode(), 'dir' => $wgLang->getDir() );
186 - $r = Html::rawElement( 'div', $langAttribs, $r );
 185+ $pageLang = $wgTitle->getPageLanguage();
 186+ $langAttribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir() );
 187+ # close the previous div, show the headings in user language,
 188+ # then open a new div with the page content language again
 189+ $r = '</div>' . $r . Html::openElement( 'div', $langAttribs );
187190 }
188191
189192 wfProfileOut( __METHOD__ );
——@@ -516,7 +519,8 @@
517520 global $wgBetterDirectionality, $wgTitle;
518521 if( $wgBetterDirectionality ) {
519522 $pageLang = $wgTitle->getPageLanguage();
520 - $attribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir() );
 523+ $attribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
 524+ 'class' => 'mw-content-'.$pageLang->getDir() );
521525 $list = Html::rawElement( 'div', $attribs, $list );
522526 }
523527
Index: trunk/phase3/skins/common/shared.css
——@@ -62,7 +62,9 @@
6363 float: right;
6464 margin-left: 5px;
6565 }
66 -
 66+/* Correct directionality when page dir is different from site/user dir */
 67+/* @noflip */.mw-content-ltr .editsection, .mw-content-rtl [dir="ltr"] .editsection { float: right; }
 68+/* @noflip */.mw-content-rtl .editsection, .mw-content-ltr [dir="rtl"] .editsection { float: left; }
6769 /**
6870 * File histories
6971 */
——@@ -614,6 +616,18 @@
615617 background-repeat: no-repeat;
616618 }
617619
 620+/* Correct directionality when page dir is different from site/user dir */
 621+/* @noflip */.mw-content-ltr ul, .mw-content-ltr ol,
 622+.mw-content-rtl [dir="ltr"] ul, .mw-content-rtl [dir="ltr"] ol {
 623+ margin: .3em 0 0 1.5em;
 624+}
 625+/* @noflip */.mw-content-rtl ul, .mw-content-rtl ol,
 626+.mw-content-ltr [dir="rtl"] ul, .mw-content-ltr [dir="rtl"] ol {
 627+ margin: .3em 1.5em 0 0;
 628+}
 629+/* @noflip */ {
 630+ margin: .3em 1.5em 0 0;
 631+}
618632 /* Localised ordered list numbering for some languages */
619633 ol:lang(bcc) li,
620634 ol:lang(bqi) li,
——@@ -640,6 +654,26 @@
641655 list-style-type: oriya;
642656 }
643657
 658+/* Correct directionality when page dir is different from site/user dir */
 659+/* @noflip */.mw-content-ltr .toc ul, .mw-content-ltr #toc ul,
 660+.mw-content-rtl [dir="ltr"] .toc ul, .mw-content-rtl [dir="ltr"] #toc ul {
 661+ text-align: left;
 662+ margin-left: 0;
 663+}
 664+/* @noflip */.mw-content-rtl .toc ul, .mw-content-rtl #toc ul,
 665+.mw-content-ltr [dir="rtl"] .toc ul, .mw-content-ltr [dir="rtl"] #toc ul {
 666+ text-align: right;
 667+ margin-right: 0;
 668+}
 669+/* @noflip */.mw-content-ltr .toc ul ul, .mw-content-ltr #toc ul ul,
 670+.mw-content-rtl [dir="ltr"] .toc ul ul, .mw-content-rtl [dir="ltr"] #toc ul ul {
 671+ margin: 0 0 0 2em;
 672+}
 673+/* @noflip */.mw-content-rtl .toc ul ul, .mw-content-rtl #toc ul ul,
 674+.mw-content-ltr [dir="rtl"] .toc ul ul, .mw-content-ltr [dir="rtl"] #toc ul ul {
 675+ margin: 0 2em 0 0;
 676+}
 677+
644678 /* tooltip styles */
645679 .mw-help-field-hint {
646680 display: none;
——@@ -737,3 +771,7 @@
738772 direction: ltr !important;
739773 unicode-bidi: embed;
740774 }
 775+
 776+#mw-revision-info, #mw-revision-nav {
 777+ direction: ltr;
 778+}
\ No newline at end of file

Sign-offs

UserFlagDate
Nikerabbitinspected10:57, 17 August 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r90762Fix r90743.robin10:03, 25 June 2011
r91315* Add release notes for my recent commits (bug 6100 and others like bugs 2803...robin22:50, 1 July 2011
r91340* Put float:left/right for images in shared.css with @noflip. This was incons...robin01:57, 2 July 2011
r91401Fix for r90337 and r90743: Use $this->title instead of $wgTitle, also grouppe...ialex08:40, 4 July 2011
r91518(bug 6100; follow-up to r91315) Being bold and removing $wgBetterDirectionali...robin02:26, 6 July 2011
r93017Fix margin of r90743 (margin is different between ul & ol)robin21:40, 24 July 2011
r93269Fix toc margin in modern skin per DieBuche on r90743robin14:38, 27 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r14495* (bug 6100) BiDi: different directionality for user interface and wiki conte...nikerabbit15:19, 31 May 2006
r44000(bug 6100) Strip Unicode BiDi embedding/override characters (U+202A - U+202E)...vyznev20:11, 27 November 2008
r60786Make LTR wgLang do the right thing on RTL wgContLang wikis. See bug 6100 and...mah09:32, 7 January 2010
r90264(part of bug 6100) Set the directionality based on user language instead of c...robin11:32, 17 June 2011
r90320Follow-up to r90265: directionality improvements as part of bug 6100 (under $...robin21:48, 17 June 2011
r90334Follow-up to r90265: directionality improvements as part of bug 6100 (under $...robin13:12, 18 June 2011
r90517* Improvements as part of bug 6100: Use wfUILang() instead of $wgContLang whe...robin10:14, 21 June 2011
r90581Directionality improvements as part of bug 6100 (under $wgBetterDirectionalit...robin13:10, 22 June 2011
r90734(bug 12406) Pages with names in RTL scripts are not listed correctly in Speci...robin20:25, 24 June 2011
r90742Directionality and language improvements as part of bug 6100 (under $wgBetter...robin22:10, 24 June 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:41, 25 June 2011 
+/* @noflip */ {
+	margin: .3em 1.5em 0 0;
+}

?

#Comment by SPQRobin (talk | contribs)   10:04, 25 June 2011

Whoops. Thanks for noticing. Fixed in r90762.

#Comment by Krinkle (talk | contribs)   16:13, 25 June 2011

Why is there both a seperate class and a dir-attribtue in between ?

Does that mean the page structure is like this ? 

<div class="mw-content-rtl">
  <foo dir="rtl">
   <span class="editsection"></span>
  </foo>
</div>

Wouldn't: 

<div class="mw-content" dir="rtl">
...
  <span class="editsection"></span>
...
</div>

make more sense ?

#Comment by SPQRobin (talk | contribs)   11:24, 26 June 2011

The structure is 

<div class="mw-content-rtl" dir="rtl">
Stuff like <span class="editsection"></span>
</div>

but the extra [dir="ltr/rtl"] is to make sure a div dir="ltr/rtl" within the wikitext works as well (which doesn't work without $wgBetterDirectionality). Ideally it shouldn't be there, but there are many pages on multilingual wikis that have an extra div dir="rtl" where editsection/TOC/lists currently don't display nicely.

#Comment by DieBuche (talk | contribs)   13:12, 27 July 2011

This broke the left toc margin for modern skin.

#Comment by SPQRobin (talk | contribs)   14:41, 27 July 2011

Fixed in r93269. Broken is a big word, it was just a minor margin issue :-)

#Comment by DieBuche (talk | contribs)   16:55, 27 July 2011

Thanks! Don't take the broken part to seriuosly :)

#Comment by Nikerabbit (talk | contribs)   10:58, 17 August 2011

Looks fine to me. Would be nice if someone could look over from css side and from rtl user perspective.

Status & tagging log