r74625 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r74624‎ | r74625 | r74626 >
Date:14:10, 11 October 2010
Author:catrope
Status:ok (Comments)
Tags:
Comment:
When reading an old-style user_options blob, use the default value for any preferences not set in the blob. This'll hopefully fix bug 25416 ($options['editfont'] not set even though editfont is a legit pref)
Modified paths:
  • /trunk/phase3/includes/User.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/User.php
——@@ -2411,7 +2411,9 @@
24122412 $this->mOptionsLoaded = true;
24132413 $this->mOptionOverrides = array();
24142414
2415 - $this->mOptions = array();
 2415+ // If an option is not set in $str, use the default value
 2416+ $this->mOptions = self::getDefaultOptions();
 2417+
24162418 $a = explode( "\n", $str );
24172419 foreach ( $a as $s ) {
24182420 $m = array();

Comments

#Comment by Brion VIBBER (talk | contribs)   15:35, 12 October 2010

Sounds ok, but I'd recommend a near-future refactor of how options are stored in the mOptions array. It looks like both loading from the old user_options field and the user_properties table now jump through some hoops to include default options in the array; it might be cleaner to keep an array of the live/saved preferences, and fill in defaults on demand in getOption().

#Comment by Catrope (talk | contribs)   15:43, 12 October 2010

The problem I fixed was occurring when using the getOptions() (plural) getter, which returns an array of all options and their values. There's code assuming that all defined preferences will be present in that array (and rightly so, IMO), but for users with incomplete preference sets loaded from user_options, that was not the case. Filling in defaults dynamically in getOption() won't address that.

#Comment by Brion VIBBER (talk | contribs)   16:57, 12 October 2010

Sounds good for now then :D +1

Status & tagging log