Page MenuHomePhabricator

Defaults of `$wgFlaggedRevsNamespaces` settings can not be overwritten properly
Open, Needs TriagePublic

Description

Since "FlaggedRevs" has been migrated to extension registration (extension.json), the value of $wgFlaggedRevsNamespaces can not be set properly in the LocalSettings.php.

FlaggedRevs sets NS_MAIN, NS_FILE and NS_TEMPLATE as default for $wgFlaggedRevsNamespaces[1]. Unfortunately as array_merge is used as a "merge strategy" for this setting by default, there is no way to remove any of these again.

e.g. setting

wfLoadExtension( 'FlaggedRevs' );
$wgFlaggedRevsNamespaces = [ NS_HELP ];

in the LocalSettings.php will result in $wgFlaggedRevsNamespaces === [ NS_MAIN, NS_FILE, NS_TEMPLATE, NS_HELP ] instead of $wgFlaggedRevsNamespaces === [ NS_HELP ] on runtime.

Therefore admins can not disable FlaggedRevs for NS_MAIN, NS_FILE or NS_TEMPLATE anymore.

Tested with

If any possible a fix of this should be backported to REL1_35 of this extension.

[1] https://github.com/wikimedia/mediawiki-extensions-FlaggedRevs/blob/91cae85cd39dafe31d17f001648904c70b03e3dc/extension.json#L428-L434

Event Timeline

Osnard created this task.Sep 7 2020, 9:15 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 7 2020, 9:15 AM

Change 625776 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/extensions/FlaggedRevs@master] Use array_plus merge strategy to allow overriding default values

https://gerrit.wikimedia.org/r/625776

Ammarpad added a subscriber: Ammarpad.EditedSep 8 2020, 10:51 AM

Actually I don't think the ExtensionRegistration system supports this. None of the five supported merge strategies appear to allow this, so this will require patch in core. So for now consider the above patch as not a full solution, but at least it would allow this inelegant trick to work (as proposed by Robert Vogel)

$wgFlaggedRevsNamespaces = [ NS_HELP, -99, -99 ];

You'll have to remember to add or remove some dummy replacements though, whenever the extension's default changes

Thanks a lot! Can you please make sure this finds its way to REL1_35 also?

Another instance of T142663: ExtensionRegistry does not allow complete override of config variables e.g. flat arrays

I thought I had left a comment on this ticket earlier, in general I don't like encouraging full overrides. Defaults should be sane, and full overrides end up require needing more maintenance later on. If the defaults aren't sane, that's a different problem. Flat arrays have always been problematic since you need to write some amount of code to be able to remove a single item from them, and we are trying to move away from code in configuration.

Thanks @Reedy, so it appears this has simply been declined.

Thanks a lot! Can you please make sure this finds its way to REL1_35 also?

I will cherry-pick it if approved. Especially given the full solution has already been declined T142663.