Page MenuHomePhabricator

User preferences are inconsistently stored (bool/int as default, string for overrides)
Open, NormalPublic

Description

Just look at my mw.user.options table added below. 1 vs true vs "1". I'm in favor of using numbers over booleans here, but why do we have string values for numbers ?

Worse yet, check the inconsistency in Gadgets. Gadgets enabled by default return 1 and gadgets enabled by the user return "1".

We should really do a bit of clean up here, and possibly even cleanup the existing values in tables, this makes coding JS against these pages more fragile than needed.

aftv5-last-filter: null
ccmeonemails: 0
cols: "120"
date: "mdy"
diffonly: "1"
disablemail: 0
disablesuggest: 0
echo-email-format: "html"
echo-email-frequency: 0
echo-notify-show-link: true
echo-show-alert: true
echo-subscriptions-email-article-linked: false
echo-subscriptions-email-edit-thank: false
echo-subscriptions-email-edit-user-talk: 1
echo-subscriptions-email-mention: false
echo-subscriptions-email-other: false
echo-subscriptions-email-page-review: false
echo-subscriptions-email-reverted: false
echo-subscriptions-email-system: true
echo-subscriptions-web-article-linked: "1"
echo-subscriptions-web-edit-thank: true
echo-subscriptions-web-edit-user-talk: true
echo-subscriptions-web-mention: true
echo-subscriptions-web-other: true
echo-subscriptions-web-page-review: true
echo-subscriptions-web-reverted: true
echo-subscriptions-web-system: true
editfont: "default"
editondblclick: 0
editsection: 1
editsectiononrightclick: 0
enotifminoredits: 0
enotifrevealaddr: 0
enotifusertalkpages: 1
enotifwatchlistpages: 0
ep_bulkdelcourses: true
ep_bulkdelorgs: false
ep_showdyk: true
ep_showtoplink: false
extendwatchlist: 0
fancysig: "1"
flaggedrevseditdiffs: true
flaggedrevssimpleui: 1
flaggedrevsstable: 0
flaggedrevsviewdiffs: false
forceeditsummary: "1"
gadget-BugStatusUpdate: "1"
gadget-DRN-wizard: 1
gadget-HotCat: "1"
gadget-Navigation_popups: "1"
gadget-NoAnimations: "1"
gadget-PrintOptions: "1"
gadget-ReferenceTooltips: 1
gadget-UTCLiveClock: "1"
gadget-addsection-plus: "1"
gadget-charinsert: 1
gadget-contribsrange: "1"
gadget-edittop: "1"
gadget-exlinks: "1"
gadget-metadata: "1"
gadget-mySandbox: 1
gadget-purgetab: "1"
gadget-teahouse: 1
gadget-widensearch: "1"
gadget-wikEd: "1"
gadget-wikEdDiff: "1"
gender: "male"
gettingstarted-task-toolbar-show-intro: ""
hideminor: 0
hidepatrolled: 0
imagesize: 2
justify: 0
language: "en"
math: "6"
mfWatchlistFilter: "all"
mfWatchlistView: "feed"
minordefault: 0
newpageshidepatrolled: 0
nocache: 0
noconvertlink: 0
norollbackdiff: 0
numberheadings: 0
previewonfirst: 0
previewontop: 1
rcdays: 7
rclimit: 50
rcshowwikidata: "1"
rememberpassword: 0
rows: "30"
searchNs0: true
searchNs1: false
searchNs2: false
searchNs3: false
searchNs4: false
searchNs5: false
searchNs6: false
searchNs7: false
searchNs8: false
searchNs9: false
searchNs10: false
searchNs11: false
searchNs12: false
searchNs13: false
searchNs14: "1"
searchNs15: false
searchNs100: false
searchNs101: false
searchNs108: false
searchNs109: false
searchNs446: false
searchNs447: false
searchNs710: false
searchNs711: false
searchNs828: false
searchNs829: false
searchlimit: 20
showhiddencats: "1"
showjumplinks: 1
shownumberswatching: 1
showtoc: 1
showtoolbar: 1
skin: "vector"
stubthreshold: "2000"
thumbsize: 4
timecorrection: "ZoneInfo|120|Europe/Amsterdam"
uls-preferences: ""
underline: 2
usebetatoolbar: 1
usebetatoolbar-cgd: 1
useeditwarning: 1
uselivepreview: 0
usenewrc: "1"
userjs-curationtoolbar: "hidden"
variant: "en"
vector-collapsiblenav: 1
vector-simplesearch: 1
visualeditor-betatempdisable: 0
visualeditor-enable: 1
watchcreations: 1
watchdefault: 0
watchdeletion: 0
watchlistdays: 3
watchlisthideanons: 0
watchlisthidebots: 0
watchlisthideliu: 0
watchlisthideminor: 0
watchlisthideown: 0
watchlisthidepatrolled: 0
watchmoves: 0
wikilove-enabled: 1
wllimit: 250
wlshowwikibase: "1"

See Also:
T29471: WikiEditor toolbar shows up unexpectedly

Details

Reference
bz52542

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 1:52 AM
bzimport set Reference to bz52542.
bzimport added a subscriber: Unknown Object (MLST).
TheDJ created this task.Aug 5 2013, 11:41 AM

Values that are actually saved in the database (ie, non-default ones) are always strings, and apparently usually equal to "1" (but for boolean prefs any truthy value is technically possible, especially if one uses the API to set them).

Values that are default ones are equal to whatever the programmer decided, and as you can see they might be 1/0, true/false, "1"/"0" or null. You should just check for truthiness in this case.

I'm not sure how to solve this, or whether we actually want to solve this; normalizing values of boolean prefs to true/false just before building this list (in ResourceLoaderUserOptionsModule#getScript) seems like the best way.

The integer came from $wgDefaultSettings. Set it to string there and it should work, except gadget and maybe the searchNs, because that are dynamically preferences, where the default is not stored in $wgDefaultSettings.

He7d3r added a comment.EditedAug 23 2014, 4:51 PM

(In reply to Bartosz Dziewoński from comment #1)

...
normalizing values of boolean prefs to true/false just before building this
list (in ResourceLoaderUserOptionsModule#getScript) seems like the best way.

+1 for normalizing boolean preferences so that either

mw.user.options.get( 'anyBooleanPreference' ) === true
mw.user.options.get( 'anyBooleanPreference' ) === false

and nothing else.

The current behavior is *very* inconvenient:

  • If I do not set $wgDefaultUserOptions['usebetatoolbar']

(i.e., if I use WikiEditor's default):

    • If I don't change the preference mw.user.options.get( 'usebetatoolbar' ) === null
    • If I enable the preference mw.user.options.get( 'usebetatoolbar' ) === "1"
  • If I set $wgDefaultUserOptions['usebetatoolbar'] = 1 (as in WMF cluster[1]):
    • If I don't change the preference mw.user.options.get( 'usebetatoolbar' ) === 1
    • If I disable the preference mw.user.options.get( 'usebetatoolbar' ) === "0" (non-empty string == true!)
  • On English Wikipedia
    • If I don't change the preference mw.user.options.get( 'usebetatoolbar' ) === 1
    • If I disable the preference mw.user.options.get( 'usebetatoolbar' ) === "" (empty string!)

[1] https://github.com/wikimedia/operations-mediawiki-config/blob/8178a4994c0137f5171798f3e831c0492c8cbc06/wmf-config/CommonSettings.php#L1641

I have had similar difficulties and personally prefer booleans wherever possible. They're more reliable and less likely to be unpredictable. They are either true or false or they're invalid.

Krinkle renamed this task from Preferences/Gadgets values are unpredictably boolean, integers or other to User preferences are inconsistently stored (bool/int as default, string for overrides).Feb 14 2015, 1:21 AM
Krinkle updated the task description. (Show Details)
Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).
Krinkle added a comment.EditedFeb 14 2015, 1:36 AM

Preferences has a filter for some values. The search preferences, for example, are normalised to booleans. A few other are casted to integers.

However the filters are only applied on the way into the database. On the way out they become strings regardless and nothing takes care of that.

So aside from the filters not being available to extension-provided preferences (it's a hardcoded privilege for core's own handling), even for the core ones, there's no normalisation on the way out:

rclimit: 50
rows: "30"
searchNs0: true
searchNs1: false
..
searchNs10: "1"
searchNs11: false
searchNs12: false
searchNs13: false
searchNs14: "1"

Quite the mix up.

Few thoughts: Applying it on the way out would be a good start. Though it's not a a great system to use for extensions. We could extend the preference descriptor array with a kind property (for int, bool or str). Or perhaps something we can derive from the type property (for HTMLForm field type). Though the latter is extendable so we'd need to put it in the HTMLFormField class instead of simply having Preferences map the shortcuts to native types.

Jay8g added a subscriber: Jay8g.Oct 29 2015, 5:03 PM

Hello, we at Russian Wikipedia had months of interwiki gadgets not working for some users, because at first compact-language-links user option was set to "" when switched off, but after some change switching it on and off turns it to "0". (Was finally fixed here.) Those who switched the option off after that change had gadget not working, but the project engineers weren't able to reproduce. Other wikis which had borrowed code from us had the issue too, and they most probably won't update the code.

I have no idea why this task's priority is set to "Low". I don't know the site policy regarding changing priorities, I will try change to "Normal" at least, maybe needs "High".

Jack_who_built_the_house raised the priority of this task from Low to Normal.Nov 21 2016, 8:06 AM
Krinkle removed a subscriber: Krinkle.

Is it in scope for this task to tackle the storing of arrays in options? e.g. the differences between JSON strings (rcfilters-saved-queries), pipe-delimited (timecorrection), newline-delimited (email-blacklist), and what these should be exposed as?

Also, just to note that email-blacklist is another outlier and is actually an array.

@Samwilson I'd recommend tracking that as a separate task because it involves explicitly changing the format of a preference key, and none of the array-like use cases you mentioned are subject to normalisation inconsistencies.

This task is about inconsistencies within a single key. Where the same logical value of the same preference key can be int, int-ish string, or boolean, depending on how/if it got normalised, and where it came from (PHP default or MySQL row).