Page MenuHomePhabricator

Extension registration does not let you use a global to override a setting to a falsity value in LocalSettings.php
Closed, ResolvedPublic

Description

Hi trying to override settings in localsettings.php wont work un less it is not extension registration for extension/skin. Meanning the workaround is to change it in the extension.json or skin.json for now until it is fixed and you can set it in localsettings.php

Event Timeline

Paladox created this task.May 29 2015, 8:37 AM
Paladox raised the priority of this task from to Unbreak Now!.
Paladox updated the task description. (Show Details)
Paladox added subscribers: Paladox, Legoktm.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 29 2015, 8:37 AM

Setting as unbreak since it is broken.

to reproduce try turning off vector search bar or watchlist is localsettings.php.

Please provide an exact example and clear steps to reproduce the problem (so other people have an exact testcase), preferably as a bulleted list.

Paladox added a comment.EditedMay 30 2015, 12:28 PM

Ok sorry. I will try to make it clearer

Steps to reproduce

Download

  • Vector skin for mediawiki 1.26 from master branch

Install

  • Then install it by doing either
  • Adding wfLoadSkin( 'Vector' );
  • or require_once( "$IP/skins/Vector/Vector.php" );
  • then go to your website to check it works then go back to LocalSettings.php
  • And try turning of The search bar or watch list using these configuations
  • set these in the localsettings.php file
  • $wgVectorUseSimpleSearch = false;
  • $wgVectorUseIconWatch = false;
  • and then go to your site to see if it changes any thing if it doesent then there is a problem with mediawiki extension registration.

another user has reported this problem here https://github.com/paladox/Metrolook/issues/107

a temporary fallback is to set it in the extension.json/skin.json manually.

Which MediaWiki version is this about?

1.26 and 1.25, both that have the extension registration system that support both extension.json skin.json.

StasR added a subscriber: StasR.Jun 2 2015, 6:24 PM

I have this same problem. The skin settings in LocalSettings.php are not transferred to skin. MW 1.25.1.

Paladox set Security to None.Jun 2 2015, 11:27 PM
Aklapper lowered the priority of this task from Unbreak Now! to High.Jun 3 2015, 2:08 PM

Which skin(s)?

StasR added a comment.Jun 3 2015, 2:21 PM

Vector and Metrolook.

StasR added a comment.Jun 3 2015, 2:26 PM

oops. Vector settings also don't work with MW1.24 i.e. for Vector the problem is not in the new system of registration

Bawolff added a subscriber: Bawolff.Jun 3 2015, 2:29 PM

I was able to reproduce this on master with Vector.

In ExtensionRegistry::exportExtractedData there's a line:

if ( !isset( $GLOBALS[$key] ) || !$GLOBALS[$key] ) {
        $GLOBALS[$key] = $val;

So if $wgVectorUseIconWatch is false from LocalSettings.php, !$GLOBALS['wgVectorUseIconWatch'] is true, and its value gets overriden by ExtensionRegistration. This seems like a bug in Extension registration, and it should only be the isset part of the check but i'm not very familar with that part of the code.


Note, if you need to work around this bug right now, a very hacky work around would be to do:

`
$wgExtensionFunctions[] = 'wfSetVectorVar';
function wfSetVectorVar() {
 global $wgVectorUseIconWatch;
 $wgVectorUseIconWatch = false;
}

In LocalSettings.php

Bawolff renamed this task from Trying to override settings in localsettings.php doesent work if extension/skin uses the new extension registration to Extension registration does not let you use a global to override a setting to a falsity value in LocalSettings.php.Jun 3 2015, 2:30 PM
Bawolff removed a subscriber: Addshore.
Bawolff added a subscriber: Addshore.
Legoktm claimed this task.Jun 3 2015, 2:54 PM

This seems like a bug in Extension registration, and it should only be the isset part of the check but i'm not very familar with that part of the code.

It's checking for empty arrays which we can just replace instead of merging into. The check should be more specific to arrays...

Change 215634 had a related patch set uploaded (by Legoktm):
registration: Don't override boolean false settings

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

Change 215636 had a related patch set uploaded (by Brian Wolff):
registration: Don't override boolean false settings

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

Change 215634 merged by jenkins-bot:
registration: Don't override boolean false settings

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

Bawolff added a subscriber: demon.Jun 3 2015, 3:27 PM

cc'ing @demon , as he may want to be aware of this bug, as its present in 1.25.1

Change 215636 merged by jenkins-bot:
registration: Don't override boolean false settings

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

It works but then a problem comes in if it is false and then you want to set true in localsettings.php. And probly array having problems.

And should allow other settings like numbers, set false and true in an array.

Like

Confirmaccount {

bio{
enable: true,
min: 50
}
}

True and false work if you do

if ( !isset( $GLOBALS[$key] ) ) {

instead.

But doesent work in an array. array needs supporting.

Like

		"ConfirmAccountRequestFormItems": {
			"UserName": {
				"enabled": true
			},
			"RealName": {
				"enabled": true
			},
			"Biography": {
				"enabled": true,
				"minWords": 50
			},
			"AreasOfInterest": {
				"enabled": true
			},
			"CV": {
				"enabled": true
			},
			"Notes": {
				"enabled": true
			},
			"Links": {
				"enabled": true
			},
			"TermsOfService": {
				"enabled": true
			}
		},

@Paladox: Do you have an actual test case of things not working? I don't understand your comment.

@Bawolff what I mean is you carnt set true or false in an array. Not even numbers like 0 or 50.

and when you set false in the extension.json or skin.json and you set true in localsettings.php it wont work like it woulden before this patch for false. needs fixing to be able to freely set true or false in an array or not. But extension.json or skin.json should not override localsettings.php

Bawolff closed this task as Resolved.Jun 4 2015, 3:42 PM

I'm marking fixed for the original example being fixed. 2d arrays like ConfirmAccount is T99257. If there are further issues of things being overridden, please file separate bugs with example code, expected behaviour and actual behaviour.

But what about settings true in localsettings.php. now true doesent work only false as long as the extension has true for that setting on by default.

But what about settings true in localsettings.php. now true doesent work only false as long as the extension has true for that setting on by default.

Please provide a link to an extension.json in a repository that currently doesn't work.

Tested with metrolook latest release setting for example metrolook search bar setting to false in the skin.json and then setting it in localsettings to true does not work but if it was the other way around and it was set to false in the skin.json file and true in localsettings.php it works. source at https://github.com/paladox/Metrolook

I've tested with modifying the vector skins.json and it works fine.

When reporting bugs in the future, please provide step by steps to reproduce, with example code. See http://www.chiark.greenend.org.uk/~sgtatham/bugs.html

Oh sorry must be a problem with me.

This comment was removed by Paladox.
Paladox removed a subscriber: gerritbot.