Because it nukes everything not in $wgDefaultUserOptions and not starting with gadget- or userjs-, this script would just destroy all GP's local overrides that are regular key names with -local-exception appended.
|mediawiki/extensions/GlobalPreferences : master||Hook into cleanupPreferences.php to prevent deletion of local exceptions|
|mediawiki/core : master||Remove gadget special-case from preferences cleanup|
|mediawiki/extensions/Gadgets : master||Prevent gadget preferences from being cleaned up|
|mediawiki/core : master||Add DeleteUnknownPreferences hook|
|mediawiki/core : master||Exclude GlobalPreferences' local-exception prefs from cleanup|
I've made a patch that adds a check for the *-local-exception preferences, but I rather think this should be done by adding a DeleteUnknownPreferences hook here that extensions could use to add their own bits to the where clause (just because it feels odd to have extension-specific code for Gadgets and GlobalPreferences in core).
I think a hook like this https://gerrit.wikimedia.org/r/#/c/423597/ would be better than listing exceptions for Gadgets and GlobalPreferences in core.
Alternatively, https://gerrit.wikimedia.org/r/#/c/422834/ is the one-line change that adds the exception to core. That doesn't cleanup the preferences if GlobalPreferences is uninstalled though (which is maybe okay, given that's the status quo with regard to Gadgets).
If the hook idea is accepted, I'll make the required change to Gadgets as well as GlobalPreferences.
@Samwilson @kaldari I don't particularly mind the hook itself, but I it took me by surprise. The fact that the clean-up script doesn't know about all valid preferences sounds to me like a bug, and adding this hook works might be a workaround for a rather narrow and specific use case. I worry that may lead to tech debt.
Could you briefly summarise the how and why that results in core not knowing about all preferences? I haven't yet thought about it much , but at first glance, I would expect the existing GetPreferences hook to suffice in ensuring extra preferences are known. That hook isn't pretty, but I assumed the improvements from T178449 would cover this use case. Thanks for your time.
I think the main problem, and the (slightly indirect) reason that this hook needs to exist, is that it is not possible to determine valid preference names without doing so for a particular user. This is why the cleanup script treats anything that is $wgDefaultUserOptions as 'unknown' (along with userjs- preferences).
The script could conceivably iterate through all users and find all known preferences that way, but at some point preference definitions will not be tied to users, so that'd be a short-term solution too. I've still got doing that on my radar, but haven't yet got it all figured out.
You're right in that this hook is perhaps not the most elegant way to fix the problem, but it does at least remove knowledge of Gadgets (and potentially GlobalPreferences) from core, which feels like a bit of an improvement.
@Samwilson Thanks, treating them as patterns for "extra" preference keys, makes perfect sense. And indeed, if it remains a LIKE-pattern kind of thing, I agree it'd be improved to have extensions supply those patterns (instead of core). So consider my concern solved for the moment.
Aside from userjs- though (which indeed is entirely wildcard), it seems to me that Gadgets and GlobalPreferences keys are not wildcards? For Gadgets, all keys are centrally defined and known. For GlobalPreferences, I see a few static keys, and the rest are static additions to existing keys, all of which would be known to preferences factory on the current wiki. Just out of curiosity, which keys are there in GlobalPreferences that are only known in specific user's context?
There aren't preferences any with variable names (that depend on the user), but the presence of a few depend on the user (e.g. those to do with email are not present for users who don't have sendemail) and so with the current system we can't retrieve all the preference names.
I have started on a patch that makes the $user parameter of PreferencesFactory::getFormDescriptor() optional (so that when it's not provided, one can get the list of all preferences, without the info ones, and with all of those that are conditional). Then, we'd be able to ask PreferencesFactory what preferences exist.