r49331 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r49330‎ | r49331 | r49332 >
Date:05:27, 9 April 2009
Author:rememberthedot
Status:resolved (Comments)
Tags:
Comment:
(bug 1553) Lowercase navigation headings in German
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesDe.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)
  • /trunk/phase3/skins/monobook/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/monobook/main.css
——@@ -889,6 +889,47 @@
890890 z-index: 3;
891891 }
892892
 893+/* Override text-transform on languages where capitalization is significant */
 894+.portlet h5[lang|=bar],
 895+.portlet h6[lang|=bar],
 896+#p-personal ul[lang|=bar],
 897+#p-cactions ul[lang|=bar] li a,
 898+.portlet h5[lang|=de],
 899+.portlet h6[lang|=de],
 900+#p-personal ul[lang|=de],
 901+#p-cactions ul[lang|=de] li a,
 902+.portlet h5[lang|=gsw],
 903+.portlet h6[lang|=gsw],
 904+#p-personal ul[lang|=gsw],
 905+#p-cactions ul[lang|=gsw] li a,
 906+.portlet h5[lang|=ksh],
 907+.portlet h6[lang|=ksh],
 908+#p-personal ul[lang|=ksh],
 909+#p-cactions ul[lang|=ksh] li a,
 910+.portlet h5[lang|=lb],
 911+.portlet h6[lang|=lb],
 912+#p-personal ul[lang|=lb],
 913+#p-cactions ul[lang|=lb] li a,
 914+.portlet h5[lang|=nds],
 915+.portlet h6[lang|=nds],
 916+#p-personal ul[lang|=nds],
 917+#p-cactions ul[lang|=nds] li a,
 918+.portlet h5[lang|=pdc],
 919+.portlet h6[lang|=pdc],
 920+#p-personal ul[lang|=pdc],
 921+#p-cactions ul[lang|=pdc] li a,
 922+.portlet h5[lang|=pdt],
 923+.portlet h6[lang|=pdt],
 924+#p-personal ul[lang|=pdt],
 925+#p-cactions ul[lang|=pdt] li a
 926+.portlet h5[lang|=pfl],
 927+.portlet h6[lang|=pfl],
 928+#p-personal ul[lang|=pfl],
 929+#p-cactions ul[lang|=pfl] li a {
 930+ text-transform: none;
 931+}
 932+
 933+
893934 /* TODO: #t-iscite is only used by the Cite extension, come up with some
894935 * system which allows extensions to add to this file on the fly
895936 */
Index: trunk/phase3/skins/MonoBook.php
——@@ -64,7 +64,9 @@
6565 * @access private
6666 */
6767 function execute() {
 68+ global $wgLang;
6869 global $wgRequest;
 70+ $wgLangCode = $wgLang->getCode();
6971 $this->skin = $skin = $this->data['skin'];
7072 $action = $wgRequest->getText( 'action' );
7173
——@@ -135,14 +137,14 @@
136138 <div id="p-cactions" class="portlet">
137139 <h5><?php $this->msg('views') ?></h5>
138140 <div class="pBody">
139 - <ul>
 141+ <ul lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>">
140142 <?php foreach($this->data['content_actions'] as $key => $tab) {
141143 echo '
142144 <li id="' . Sanitizer::escapeId( "ca-$key" ) . '"';
143145 if( $tab['class'] ) {
144146 echo ' class="'.htmlspecialchars($tab['class']).'"';
145147 }
146 - echo'><a href="'.htmlspecialchars($tab['href']).'"';
 148+ echo '><a href="'.htmlspecialchars($tab['href']).'"';
147149 # We don't want to give the watch tab an accesskey if the
148150 # page is being edited, because that conflicts with the
149151 # accesskey on the watch checkbox. We also don't want to
——@@ -163,7 +165,7 @@
164166 <div class="portlet" id="p-personal">
165167 <h5><?php $this->msg('personaltools') ?></h5>
166168 <div class="pBody">
167 - <ul>
 169+ <ul lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>">
168170 <?php foreach($this->data['personal_urls'] as $key => $item) { ?>
169171 <li id="<?php echo Sanitizer::escapeId( "pt-$key" ) ?>"<?php
170172 if ($item['active']) { ?> class="active"<?php } ?>><a href="<?php
——@@ -249,10 +251,12 @@
250252
251253 /*************************************************************************************************/
252254 function searchBox() {
 255+ global $wgLang;
253256 global $wgUseTwoButtonsSearchForm;
 257+ $wgLangCode = $wgLang->getCode();
254258 ?>
255259 <div id="p-search" class="portlet">
256 - <h5><label for="searchInput"><?php $this->msg('search') ?></label></h5>
 260+ <h5 lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>"><label for="searchInput"><?php $this->msg('search') ?></label></h5>
257261 <div id="searchBody" class="pBody">
258262 <form action="<?php $this->text('wgScript') ?>" id="searchform"><div>
259263 <input type='hidden' name="title" value="<?php $this->text('searchtitle') ?>"/>
——@@ -272,9 +276,11 @@
273277
274278 /*************************************************************************************************/
275279 function toolbox() {
 280+ global $wgLang;
 281+ $wgLangCode = $wgLang->getCode();
276282 ?>
277283 <div class="portlet" id="p-tb">
278 - <h5><?php $this->msg('toolbox') ?></h5>
 284+ <h5 lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>"><?php $this->msg('toolbox') ?></h5>
279285 <div class="pBody">
280286 <ul>
281287 <?php
——@@ -332,10 +338,12 @@
333339
334340 /*************************************************************************************************/
335341 function languageBox() {
 342+ global $wgLang;
 343+ $wgLangCode = $wgLangCode;
336344 if( $this->data['language_urls'] ) {
337345 ?>
338346 <div id="p-lang" class="portlet">
339 - <h5><?php $this->msg('otherlanguages') ?></h5>
 347+ <h5 lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>"><?php $this->msg('otherlanguages') ?></h5>
340348 <div class="pBody">
341349 <ul>
342350 <?php foreach($this->data['language_urls'] as $langlink) { ?>
——@@ -351,9 +359,11 @@
352360
353361 /*************************************************************************************************/
354362 function customBox( $bar, $cont ) {
 363+ global $wgLang;
 364+ $wgLangCode = $wgLang->getCode();
355365 ?>
356366 <div class='generated-sidebar portlet' id='<?php echo Sanitizer::escapeId( "p-$bar" ) ?>'<?php echo $this->skin->tooltip('p-'.$bar) ?>>
357 - <h5><?php $out = wfMsg( $bar ); if (wfEmptyMsg($bar, $out)) echo $bar; else echo $out; ?></h5>
 367+ <h5 lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>"><?php $out = wfMsg( $bar ); if (wfEmptyMsg($bar, $out)) echo htmlspecialchars($bar); else echo htmlspecialchars($out); ?></h5>
358368 <div class='pBody'>
359369 <?php if ( is_array( $cont ) ) { ?>
360370 <ul>
Index: trunk/phase3/includes/Skin.php
——@@ -1870,8 +1870,7 @@
18711871 if( strpos( $line, '*' ) !== 0 )
18721872 continue;
18731873 if( strpos( $line, '**') !== 0 ) {
1874 - $line = trim( $line, '* ' );
1875 - $heading = $line;
 1874+ $heading = trim( $line, '* ' );
18761875 if( !array_key_exists( $heading, $bar ) ) $bar[$heading] = array();
18771876 } else {
18781877 if( strpos( $line, '|' ) !== false ) { // sanity check
——@@ -1912,4 +1911,4 @@
19131912 wfProfileOut( __METHOD__ );
19141913 return $bar;
19151914 }
1916 -}
\ No newline at end of file
 1915+}
Index: trunk/phase3/languages/messages/MessagesDe.php
——@@ -2591,14 +2591,7 @@
25922592 'standard.css' => '/* CSS an dieser Stelle wirkt sich auf den Klassik-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
25932593 'nostalgia.css' => '/* CSS an dieser Stelle wirkt sich auf den Nostalgie-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
25942594 'cologneblue.css' => '/* CSS an dieser Stelle wirkt sich auf den Kölnisch-Blau-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
2595 -'monobook.css' => '/* CSS an dieser Stelle wirkt sich auf den Monobook-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */
2596 -/* Kleinschreibung nicht erzwingen */
2597 -.portlet h5,
2598 -.portlet h6,
2599 -#p-personal ul,
2600 -#p-cactions li a {
2601 - text-transform: none;
2602 -}',
 2595+'monobook.css' => '/* CSS an dieser Stelle wirkt sich auf den Monobook-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
26032596 'myskin.css' => '/* CSS an dieser Stelle wirkt sich auf den MySkin-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
26042597 'chick.css' => '/* CSS an dieser Stelle wirkt sich auf den Küken-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
26052598 'simple.css' => '/* CSS an dieser Stelle wirkt sich auf den Einfach-Skin aus. Für allgemeingültige Skin-Anpassungen bitte [[MediaWiki:Common.css]] bearbeiten. */',
Index: trunk/phase3/RELEASE-NOTES
——@@ -333,6 +333,7 @@
334334 * (bug 17948) Maintenance scripts now exit(0) or exit(1) as appropriate
335335 * (bug 18377) Time in Enhanced ChangesList lacking localisation
336336 * (bug 12998) Allow <sup>, <sub>, etc. in DISPLAYTITLE
 337+* (bug 1553) Lowercase navigation headings in German
337338
338339 == API changes in 1.15 ==
339340 * (bug 16858) Revamped list=deletedrevs to make listing deleted contributions

Follow-up revisions

RevisionCommit summaryAuthorDate
r49350Follow-up to r49331, reduce redundancy in language attribute generation in Mo...rememberthedot19:06, 9 April 2009
r51924(bug 19209) Follow-up to r49331: CSS for nds should not apply for nds-nl.siebrand20:37, 15 June 2009
r52017Follow-up to r49331: Moved decapitalization code to "a Messages*.php property...rememberthedot04:26, 17 June 2009

Comments

#Comment by Werdna (talk | contribs)   05:30, 9 April 2009

Nitpick: Please describe the change you made, rather than the bug you fixed, in your commit message.

#Comment by Brion VIBBER (talk | contribs)   15:16, 9 April 2009

This seems to rely on attribute selectors, which IIRC don't work in at least some versions of Internet Explorer. Can you confirm functioning in at least IE7 and 8?

The use of a $wgLangCode *local* variable seems very poor form and is very confusing. Needs fix or revert.

Sticking a bunch of hardcoded stuff like <h5 lang="<?php echo $wgLangCode; ?>" xml:lang="<?php echo $wgLangCode; ?>"> seems pretty ugly; it's a lot of repetitive code which is likely to be error-prone, and further doesn't follow coding standards for confirming that output is escaped. (In this case it should be pre-validated, but it's bad practice.) The repeated code should be encapsulated if it's needed.

#Comment by Remember the dot (talk | contribs)   19:07, 9 April 2009

Attribute selectors work in both IE7 and IE8, just not in IE6. We discussed this on bugzilla and couldn't see a better way to do it. German IE6 users that can't be bothered to upgrade will just have to suffer with the status quo of no capitalization.

Good point about encapsulation. In r49350 I've moved all the code for generating the language attributes into a private function langAttributes(). Feel free to move the function around or rename it if you'd like.

#Comment by Werdna (talk | contribs)   07:45, 23 April 2009

Seems mostly fixed in r49350 and follow-up r49359.

#Comment by Siebrand (talk | contribs)   06:09, 16 June 2009

Per TimStarling on r51924: "Please move this language list out of CSS entirely. Use a Messages*.php property, a <body> class and a descendant selector, like we do for RTL. Otherwise it will be a maintenance problem.". Marking 'fixme' again.

#Comment by Tim Starling (talk | contribs)   09:02, 16 August 2009

Fixed by rememberthedot in r52017.

Status & tagging log