r91432 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r91431‎ | r91432 | r91433 >
Date:01:26, 5 July 2011
Author:robin
Status:resolved (Comments)
Tags:
Comment:
(see bug 29712) Removing the defaultUserOptionOverrides in MessagesXx.php because it is broken (and has been for a long time) and secondly, it is only used for things that can be done otherwise:
* 'editfont' => 'sans-serif'; -> trivial (can be set in CSS)
* 'underline' => 0; by languages written in scripts that are hard to read with underlines -> now set in shared.css, which makes it work also when they are used as interface language (note that [lang="xx"] is not yet present on wikitext content by default but will be soon as part of my work on RTL improvements)
* 'quickbar' => 2; by RTL languages. For this, I introduced a new option (5) that sets left/right according to the directionality of your interface language (and automatic is better than setting it in MessagesXx files!). Note that the broken feature was in this case "corrected" by CSS flipping, causing preferences to say "left" while it was actually "right". This commit fixes that bug as well.
Third, it actually would have made more sense to have it them load for wgLang instead of wgContLang, but that's not possible because wgLang is itself dependent on preferences
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/LocalisationCache.php (modified) (history)
  • /trunk/phase3/includes/SkinLegacy.php (modified) (history)
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesAr.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesArc.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesCkb.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesFa.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesHe.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesKk_arab.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesLn.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesMzn.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesPnb.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesPs.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesUr.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesYi.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)
  • /trunk/phase3/skins/CologneBlue.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)
  • /trunk/phase3/skins/common/shared.css (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
@@ -911,6 +911,7 @@
912912 'qbsettings-fixedright',
913913 'qbsettings-floatingleft',
914914 'qbsettings-floatingright',
 915+ 'qbsettings-directionality',
915916 ),
916917 'preferences' => array(
917918 'preferences',
Index: trunk/phase3/skins/CologneBlue.php
@@ -26,7 +26,6 @@
2727 parent::setupSkinUserCss( $out );
2828 $out->addModuleStyles( 'skins.cologneblue' );
2929
30 - global $wgContLang;
3130 $qb = $this->qbSetting();
3231 $rules = array();
3332
@@ -48,9 +47,6 @@
4948 $rules[] = "body>#quickbar { position: fixed; right: 4px; top: 4px; overflow: auto ;bottom:4px;}"; # Hides from IE
5049 }
5150 $style = implode( "\n", $rules );
52 - if ( $wgContLang->getDir() === 'rtl' ) {
53 - $style = CSSJanus::transform( $style, true, false );
54 - }
5551 $out->addInlineStyle( $style );
5652 }
5753
Index: trunk/phase3/skins/Standard.php
@@ -25,7 +25,6 @@
2626 parent::setupSkinUserCss( $out );
2727 $out->AddModuleStyles( 'skins.standard' );
2828
29 - global $wgContLang;
3029 $qb = $this->qbSetting();
3130 $rules = array();
3231
@@ -44,9 +43,6 @@
4544 $rules[] = "#article, #mw-data-after-content { margin-right: 152px; margin-left: 4px; }";
4645 }
4746 $style = implode( "\n", $rules );
48 - if ( $wgContLang->getDir() === 'rtl' ) {
49 - $style = CSSJanus::transform( $style, true, false );
50 - }
5147 $out->addInlineStyle( $style );
5248 }
5349
Index: trunk/phase3/skins/common/shared.css
@@ -4,6 +4,12 @@
55 * another, but don't ignore the poor pre-Monobook users either.
66 */
77
 8+/* The scripts of these languages are very hard to read with underlines */
 9+[lang="ar"] a, [lang="ckb"] a, [lang="fa"] a, [lang="kk-arab"] a,
 10+[lang="mzn"] a, [lang="ps"] a, [lang="ur"] a {
 11+ text-decoration: none;
 12+}
 13+
814 /* Default style for semantic tags */
915 abbr, acronym, .explain {
1016 border-bottom: 1px dotted black;
Index: trunk/phase3/includes/User.php
@@ -1184,16 +1184,10 @@
11851185 * @return Array of String options
11861186 */
11871187 static function getDefaultOptions() {
1188 - global $wgNamespacesToBeSearchedDefault;
1189 - /**
1190 - * Site defaults will override the global/language defaults
1191 - */
1192 - global $wgDefaultUserOptions, $wgContLang, $wgDefaultSkin;
1193 - $defOpt = $wgDefaultUserOptions + $wgContLang->getDefaultUserOptionOverrides();
 1188+ global $wgNamespacesToBeSearchedDefault, $wgDefaultUserOptions, $wgContLang, $wgDefaultSkin;
11941189
1195 - /**
1196 - * default language setting
1197 - */
 1190+ $defOpt = $wgDefaultUserOptions;
 1191+ # default language setting
11981192 $variant = $wgContLang->getDefaultVariant();
11991193 $defOpt['variant'] = $variant;
12001194 $defOpt['language'] = $variant;
Index: trunk/phase3/includes/LocalisationCache.php
@@ -84,7 +84,7 @@
8585 'fallback', 'namespaceNames', 'bookstoreList',
8686 'magicWords', 'messages', 'rtl', 'capitalizeAllNouns', 'digitTransformTable',
8787 'separatorTransformTable', 'fallback8bitEncoding', 'linkPrefixExtension',
88 - 'defaultUserOptionOverrides', 'linkTrail', 'namespaceAliases',
 88+ 'linkTrail', 'namespaceAliases',
8989 'dateFormats', 'datePreferences', 'datePreferenceMigrationMap',
9090 'defaultDateFormat', 'extraUserToggles', 'specialPageAliases',
9191 'imageFiles', 'preloadedMessages', 'namespaceGenderAliases',
@@ -95,8 +95,7 @@
9696 * by a fallback sequence.
9797 */
9898 static public $mergeableMapKeys = array( 'messages', 'namespaceNames',
99 - 'dateFormats', 'defaultUserOptionOverrides', 'imageFiles',
100 - 'preloadedMessages',
 99+ 'dateFormats', 'imageFiles', 'preloadedMessages',
101100 );
102101
103102 /**
@@ -130,8 +129,7 @@
131130 /**
132131 * Keys which are loaded automatically by initLanguage()
133132 */
134 - static public $preloadedKeys = array( 'dateFormats', 'namespaceNames',
135 - 'defaultUserOptionOverrides' );
 133+ static public $preloadedKeys = array( 'dateFormats', 'namespaceNames' );
136134
137135 /**
138136 * Constructor.
@@ -602,11 +600,6 @@
603601 # Decouple the reference to prevent accidental damage
604602 unset($page);
605603
606 - # Fix broken defaultUserOptionOverrides
607 - if ( !is_array( $allData['defaultUserOptionOverrides'] ) ) {
608 - $allData['defaultUserOptionOverrides'] = array();
609 - }
610 -
611604 # Set the list keys
612605 $allData['list'] = array();
613606 foreach ( self::$splitKeys as $key ) {
@@ -616,11 +609,6 @@
617610 # Run hooks
618611 wfRunHooks( 'LocalisationCacheRecache', array( $this, $code, &$allData ) );
619612
620 - if ( is_null( $allData['defaultUserOptionOverrides'] ) ) {
621 - throw new MWException( __METHOD__.': Localisation data failed sanity check! ' .
622 - 'Check that your languages/messages/MessagesEn.php file is intact.' );
623 - }
624 -
625613 # Set the preload key
626614 $allData['preload'] = $this->buildPreload( $allData );
627615
Index: trunk/phase3/includes/SkinLegacy.php
@@ -54,6 +54,12 @@
5555 return 0;
5656 }
5757 $q = $wgUser->getOption( 'quickbar', 0 );
 58+ if( $q == 5 ) {
 59+ # 5 is the default, which chooses the setting
 60+ # depending on the directionality of your interface language
 61+ global $wgLang;
 62+ return $wgLang->isRTL() ? 2 : 1;
 63+ }
5864 return $q;
5965 }
6066
Index: trunk/phase3/includes/DefaultSettings.php
@@ -3079,7 +3079,7 @@
30803080 'numberheadings' => 0,
30813081 'previewonfirst' => 0,
30823082 'previewontop' => 1,
3083 - 'quickbar' => 1,
 3083+ 'quickbar' => 5,
30843084 'rcdays' => 7,
30853085 'rclimit' => 50,
30863086 'rememberpassword' => 0,
Index: trunk/phase3/languages/messages/MessagesPnb.php
@@ -16,12 +16,6 @@
1717 $fallback8bitEncoding = 'windows-1256';
1818
1919 $rtl = true;
20 -$defaultUserOptionOverrides = array(
21 - # Swap sidebar to right side by default
22 - 'quickbar' => 2,
23 - # Underlines seriously harm legibility. Force off:
24 - 'underline' => 0,
25 -);
2620
2721 $messages = array(
2822 # User preference toggles
Index: trunk/phase3/languages/messages/MessagesHe.php
@@ -20,10 +20,6 @@
2121 */
2222
2323 $rtl = true;
24 -$defaultUserOptionOverrides = array(
25 - # Swap sidebar to right side by default
26 - 'quickbar' => 2,
27 -);
2824
2925 $linkTrail = '/^([a-zא-ת]+)(.*)$/sDu';
3026 $fallback8bitEncoding = 'windows-1255';
Index: trunk/phase3/languages/messages/MessagesCkb.php
@@ -20,12 +20,6 @@
2121 $fallback8bitEncoding = 'windows-1256';
2222
2323 $rtl = true;
24 -$defaultUserOptionOverrides = array(
25 - # Swap sidebar to right side by default
26 - 'quickbar' => 2,
27 - # Underlines seriously harm legibility. Force off:
28 - 'underline' => 0,
29 -);
3024
3125 $digitTransformTable = array(
3226 '0' => '٠', # ٠
Index: trunk/phase3/languages/messages/MessagesPs.php
@@ -154,12 +154,6 @@
155155 );
156156
157157 $rtl = true;
158 -$defaultUserOptionOverrides = array(
159 - # Swap sidebar to right side by default
160 - 'quickbar' => 2,
161 - # Underlines seriously harm legibility. Force off:
162 - 'underline' => 0,
163 -);
164158
165159 $messages = array(
166160 # User preference toggles
Index: trunk/phase3/languages/messages/MessagesAr.php
@@ -54,12 +54,6 @@
5555 $fallback8bitEncoding = 'windows-1256';
5656
5757 $rtl = true;
58 -$defaultUserOptionOverrides = array(
59 - # Swap sidebar to right side by default
60 - 'quickbar' => 2,
61 - # Underlines seriously harm legibility. Force off:
62 - 'underline' => 0,
63 -);
6458
6559 /**
6660 * A list of date format preference keys which can be selected in user
Index: trunk/phase3/languages/messages/MessagesKk_arab.php
@@ -45,13 +45,6 @@
4646 ',' => '٬', # ٬
4747 );
4848
49 -$defaultUserOptionOverrides = array(
50 - # Swap sidebar to right side by default
51 - 'quickbar' => 2,
52 - # Underlines seriously harm legibility. Force off:
53 - 'underline' => 0,
54 -);
55 -
5649 $extraUserToggles = array(
5750 'nolangconversion'
5851 );
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -23,8 +23,6 @@
2424
2525 /**
2626 * Is the language written right-to-left?
27 - * Note that right-to-left languages generally also specify
28 - * $defaultUserOptionOverrides = array( 'quickbar' => 2 );
2927 */
3028 $rtl = false;
3129
@@ -45,11 +43,6 @@
4644 $separatorTransformTable = null;
4745
4846 /**
49 - * Overrides for the default user options. This is mainly used by RTL languages.
50 - */
51 -$defaultUserOptionOverrides = array();
52 -
53 -/**
5447 * Extra user preferences which will be shown in Special:Preferences as
5548 * checkboxes. Extra settings in derived languages will automatically be
5649 * appended to the array of the fallback languages.
@@ -1739,12 +1732,13 @@
17401733 'opensearch-desc' => '{{SITENAME}} ({{CONTENTLANGUAGE}})', # do not translate or duplicate this message to other languages
17411734
17421735 # Quickbar
1743 -'qbsettings' => 'Quickbar',
1744 -'qbsettings-none' => 'None',
1745 -'qbsettings-fixedleft' => 'Fixed left',
1746 -'qbsettings-fixedright' => 'Fixed right',
1747 -'qbsettings-floatingleft' => 'Floating left',
1748 -'qbsettings-floatingright' => 'Floating right',
 1736+'qbsettings' => 'Quickbar',
 1737+'qbsettings-none' => 'None',
 1738+'qbsettings-fixedleft' => 'Fixed left',
 1739+'qbsettings-fixedright' => 'Fixed right',
 1740+'qbsettings-floatingleft' => 'Floating left',
 1741+'qbsettings-floatingright' => 'Floating right',
 1742+'qbsettings-directionality' => 'Fixed, depending on the directionality of your language',
17491743
17501744 # Preferences page
17511745 'preferences' => 'Preferences',
Index: trunk/phase3/languages/messages/MessagesYi.php
@@ -135,11 +135,6 @@
136136 'Withoutinterwiki' => array( 'בלעטער_אָן_אינטערוויקי' ),
137137 );
138138
139 -$defaultUserOptionOverrides = array(
140 - # Swap sidebar to right side by default
141 - 'quickbar' => 2,
142 -);
143 -
144139 $magicWords = array(
145140 'redirect' => array( '0', '#ווייטערפירן', '#הפניה', '#REDIRECT' ),
146141 'notoc' => array( '0', '__קיין_אינהאלט_טאבעלע__', '__ללא_תוכן_עניינים__', '__ללא_תוכן__', '__NOTOC__' ),
Index: trunk/phase3/languages/messages/MessagesUr.php
@@ -19,12 +19,6 @@
2020
2121 $fallback8bitEncoding = 'windows-1256';
2222 $rtl = true;
23 -$defaultUserOptionOverrides = array(
24 - # Swap sidebar to right side by default
25 - 'quickbar' => 2,
26 - # Underlines seriously harm legibility. Force off:
27 - 'underline' => 0,
28 -);
2923
3024 $namespaceNames = array(
3125 NS_MEDIA => 'زریعہ',
Index: trunk/phase3/languages/messages/MessagesFa.php
@@ -174,12 +174,6 @@
175175 $fallback8bitEncoding = 'windows-1256';
176176
177177 $rtl = true;
178 -$defaultUserOptionOverrides = array(
179 - # Swap sidebar to right side by default
180 - 'quickbar' => 2,
181 - # Underlines seriously harm legibility. Force off:
182 - 'underline' => 0,
183 -);
184178
185179
186180 /**
Index: trunk/phase3/languages/messages/MessagesMzn.php
@@ -19,12 +19,6 @@
2020 $fallback8bitEncoding = 'windows-1256';
2121
2222 $rtl = true;
23 -$defaultUserOptionOverrides = array(
24 - # Swap sidebar to right side by default
25 - 'quickbar' => 2,
26 - # Underlines seriously harm legibility. Force off:
27 - 'underline' => 0,
28 -);
2923
3024 $namespaceNames = array(
3125 NS_MEDIA => 'مه‌دیا',
Index: trunk/phase3/languages/messages/MessagesLn.php
@@ -13,10 +13,6 @@
1414
1515 $fallback = 'fr';
1616
17 -$defaultUserOptionOverrides = array(
18 - 'editfont' => 'sans-serif', # poor font support
19 -);
20 -
2117 $linkPrefixExtension = true;
2218
2319 # Same as the French (bug 8485)
Index: trunk/phase3/languages/messages/MessagesArc.php
@@ -94,11 +94,6 @@
9595
9696 $rtl = true;
9797
98 -$defaultUserOptionOverrides = array(
99 - # Swap sidebar to right side by default
100 - 'quickbar' => 2,
101 -);
102 -
10398 $messages = array(
10499 # User preference toggles
105100 'tog-underline' => 'ܪܫܘܡ ܣܪܛܐ ܬܚܝܬ ܐܣܘܪܐ:',
Index: trunk/phase3/languages/Language.php
@@ -504,7 +504,8 @@
505505 $this->getMessage( 'qbsettings-fixedleft' ),
506506 $this->getMessage( 'qbsettings-fixedright' ),
507507 $this->getMessage( 'qbsettings-floatingleft' ),
508 - $this->getMessage( 'qbsettings-floatingright' )
 508+ $this->getMessage( 'qbsettings-floatingright' ),
 509+ $this->getMessage( 'qbsettings-directionality' )
509510 );
510511 }
511512
@@ -553,13 +554,6 @@
554555 /**
555556 * @return array
556557 */
557 - function getDefaultUserOptionOverrides() {
558 - return self::$dataCache->getItem( $this->mCode, 'defaultUserOptionOverrides' );
559 - }
560 -
561 - /**
562 - * @return array
563 - */
564558 function getExtraUserToggles() {
565559 return self::$dataCache->getItem( $this->mCode, 'extraUserToggles' );
566560 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r91433Follow-up r91432: I always forget release notes..robin01:51, 5 July 2011
r93288SkinLegacy:...robin17:20, 27 July 2011
r96261* Mark underline css as !important. It should override other css (such as the...robin21:50, 4 September 2011
r96659Replace localisation cache check removed in r91432 with another checkrobin12:49, 9 September 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:33, 5 July 2011
        if ( is_null( $allData['defaultUserOptionOverrides'] ) ) {	 
                         throw new MWException( __METHOD__.': Localisation data failed sanity check! ' .	 
                                 'Check that your languages/messages/MessagesEn.php file is intact.' );	 
                 }

Should this be replace with another check instead?

#Comment by Reedy (talk | contribs)   11:42, 9 September 2011

Ping. Can you reply to/address the comment Niklas made above? Thanks!

#Comment by SPQRobin (talk | contribs)   12:25, 9 September 2011

I don't know enough of LocalisationCache to know if this needs replacement, or with what it should be replaced...

#Comment by Nikerabbit (talk | contribs)   12:26, 9 September 2011

I think replacing the key in it with some other key should work fine. It's just there for defensive programming, catching errors which should never happen.

#Comment by SPQRobin (talk | contribs)   12:55, 9 September 2011

Okay, done in r96659.

#Comment by Raymond (talk | contribs)   19:54, 5 July 2011

Dunno if related to this revision but appears today the first time at Translatewiki:

PHP Warning: Invalid argument supplied for foreach() in /www/w/skins/CologneBlue.php on line 238
#Comment by SPQRobin (talk | contribs)   22:11, 5 July 2011

Hmm, that's strange.. I don't know how this commit could cause that.

#Comment by SPQRobin (talk | contribs)   17:30, 27 July 2011

In any case, it will no longer return a warning as of r93288 (I got the warning on my localhost as well).

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

You removed 'editfont' => 'sans-serif', from MessageLn, but I see no replacement for it.

Have you checked that the rules you have added in shared.css about link underlining don't prevent users from changing that setting to whatever they want?

#Comment by SPQRobin (talk | contribs)   21:24, 25 August 2011

editfont: No, I didn't add a replacement because imho it is not needed, as it didn't work (and I don't think it's that important) plus ln.wikipedia has added it on their MediaWiki:Common.css anyway.

CSS underlining: Apparently setting the underline option to 'always' has no effect because it is overridden by the CSS. Users can however override the CSS in their personal CSS (or on site CSS). Maybe we should just leave out the CSS I added (or just remove the underline preference as it can/should be done in personal CSS)?

#Comment by SPQRobin (talk | contribs)   21:53, 4 September 2011

Should be OK in r96261 by marking the css of the user option as !important.

Status & tagging log