Page MenuHomePhabricator

Add a hook for $wgDefaultUserOptions
Closed, ResolvedPublic

Description

Hi,

I was just playing with a preferences experiment while running into this bug. It appears the way User->getOption works as of 1.18+, boolean preferences that are on by default cannot be set to false.

A good example can be found on translatewiki.net. Go to Special:Preferences#mw-prefsection-gadgets and look at the "No-op" gadget. If you uncheck that checkbox and save the preferences (which results in a success-msg "Your preferences have been saved."), then go back to the Gadgets tab and you'll find yourself seeing the checkbox still being checked.

Although I haven't been able to fix it, I believe the problem is caused by the code for '$defaultOverride' in User::getOption.

It is used in the preferences like this:

$default = FooBar::hasAwesomeness(); // e.g. true or false
$user->getOption( 'foobar', $default );

Inside User::getOption is code that looks very broken to me, no idea how this got passed code review (assuming it already has). See http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/User.php?revision=96442&view=markup&pathrev=96442#l2130

English verbalization of that function as of right now:

  • CODE: If User::mOptions is null
  • NOTE: OK. mOptions is a lazy-load cache for all options, not just this one
  • call getDefaultOptions() and store it in User::mOptions
  • NOTE: OK. We're lazy loading the options
  • CODE: If $default is not an empty string
    • CODE: Return $default
  • NOTE: This makes no friggin sense at all. For one, this is inside the cache null check which is pretty much unrelated to this. Secondly, why is $default being checked for empty string ? This arguments defaults to false and moreover implies (without documentation) that a default may not be an empty string.

EFFECT:

As a result the preferences panel is fairly broken. Some of the default preferences cannot be changed, and several other weird things happen (ie. when options are cached defaults are handled differently than when they're not cached..)


Version: 1.20.x
Severity: major
URL: http://translatewiki.net/w/i.php?title=Special:Preferences&success#mw-prefsection-gadgets

Details

Reference
bz30940

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:48 PM
bzimport set Reference to bz30940.

Origin: r18334.

Reason for blocking 1.18: Although this bug has been in core since 2006. It remained unused by any deployed code until now. However defaulting preferences to true happens for a few preferences and extensions (such as Gadgets with [default]) now, so it's a bigger problem now.

CC'ing hashar as it's his revision ;)

Changing bug summary to be more specific, as I've been unable to reproduce other cases.

The problem turns out to be a lot simpler, the Gadgets extension is not setting $wgDefaultUserOptions at all. Instead it's using the 'default' key in the HTMLForm array. and this value set to true based on Gadget->isEnabled, which fetches from User->getOption(key, /*fallback*/ isEnabledByDefault). So 'default' is true both if it was enabled by user or by default. That's fine in most cases, but it needs the true site default value to be in $wgDefaultUserOptions in order to know that user_properties row should be removed when the value is missing upon submisison.

The reason Gadgets has not / cannot set $wgDefaultUserOptions, is because it's a global variable and the extension won't know the preference names until it's initialized (because gadgets are variable and set by site admins, it has to query those first before it can know the default values).

So there has to be a hook in order to allow extensions to set $wgDefaultUserOptions in a better way. Still blocking deployment as this affects not only Gadgets 2.0, but the 1.18 branch as well.

Krinkle: adding drama to bugzilla since 2011 :-)

(In reply to comment #5)

Krinkle: adding drama to bugzilla since 2011 :-)

Hehe, well. I like being able to refer to stuff with a ticket id instead of a (set of) revision id(s).

It surely makes life easier :-)