Page MenuHomePhabricator

Extension configuration settings that have 2d (or greater) arrays do not work in extension registration
Closed, ResolvedPublic

Description

Hi I am trying to convert confirmaccount to the new extension registration but there is a problem with configs. some are not supported please see file for ones that do not work or doint work properly

https://gerrit.wikimedia.org/r/#/c/208523/19/ConfirmAccount.config.php

and for configs that have mutple arrays like this

		"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
			}
		},

do not work properly because if you try to override the setting value it just wont work and causes it to stop working or keep working the way it is without the value doing anything.

Must configs that I have converted to extension registration wont allow localsettings.php to override if I set for example for bio to not need 50 words but instead turn off min word. this is a bug because it work in the normal php file without extension.json.

Related Objects

Event Timeline

Paladox created this task.May 15 2015, 4:32 PM
Paladox raised the priority of this task from to Needs Triage.
Paladox updated the task description. (Show Details)
Paladox added subscribers: Paladox, Legoktm, Florian.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 15 2015, 4:32 PM
Legoktm renamed this task from Configs that have multiple arrays do not work in extension registration to Extension configuration settings that have 2d (or greater) arrays do not work in extension registration.May 18 2015, 6:40 AM

I discussed this with Tim at the hackathon. He suggested that I look at how LocalisationCache has multiple ways to merge the different types of arrays. Similarly ExtensionProcessor / ExtensionRegistry should support the config setting adding some metadata on how these settings should be merged. https://gerrit.wikimedia.org/r/214306 might be a prerequisite to doing that.

Hi I will test this with Confirmaccount using the patch that converts it to extension.json. it may not work because sometimes I may do it wrong or it may work. would need at least one other person to try it to say it works.

I get this error
Fatal error: Call-time pass-by-reference has been removed in /home/simplera/public_html/en/includes/registration/ExtensionRegistry.php on line 127

Shows from error log on the patch this error

08:05:56 127 | ERROR | Call-time pass-by-reference calls are prohibited
08:05:56 | | (Generic.Functions.CallTimePassByReference.NotAllowed)
08:05:56 130 | ERROR | Call-time pass-by-reference calls are prohibited
08:05:56 | | (Generic.Functions.CallTimePassByReference.NotAllowed)

https://integration.wikimedia.org/ci/job/mediawiki-core-phpcs-HEAD/16440/console

Seems that changing setting for extension/skin in localsetting is not working or isent overriding the default setting. this needs to be fixed since that would be classed as extension registration is broken. since no configuration will override it.

Paladox triaged this task as Unbreak Now! priority.Jun 4 2015, 3:57 PM

Setting as unbreak since it doesent work and needs fixing.

Paladox added a subscriber: Bawolff.Jun 4 2015, 3:58 PM
Paladox added a comment.EditedJun 4 2015, 5:14 PM

And setting for example

$wgConfirmAccountRequestFormItems['Biography']['minWords'] = false;

in localsetting doesent work if you use extension.json for that extension. it just disables the whole bit. It should actually just disable the limit not disable it.

Aklapper lowered the priority of this task from Unbreak Now! to High.Jun 4 2015, 8:26 PM

Setting as unbreak since it doesent work and needs fixing.

@Paladox: By that definition every task would have "unbreak now" priority. Please read the priority definitions to not get your Phabricator account blocked due to repeated misuse of the Priority field. Thank you.

Krinkle added a subscriber: Krinkle.Jun 4 2015, 8:31 PM

Setting as unbreak since it doesent work and needs fixing.

This is not unbreak now because it didn't break. It never worked. Nothing in this description that previously worked is now broken. You're trying to use something that doesn't exist yet. The old approach works fine, just keep that for now if JSON isn't working for you.

This comment was removed by Paladox.

Well if it was never broken and never implanted then this could be a feature report to suggest that be added to extension registration.

Aklapper lowered the priority of this task from High to Normal.Jun 4 2015, 8:41 PM

Workarround is to do something like

  1. Which form elements to show at Special:RequestAccount
		$features = array(
			# Let users make names other than their "real name"
			'UserName'        => array( 'enabled' => true ),
			# Real name of user
			'RealName'        => array( 'enabled' => true ),
			# Biographical info
			'Biography'       => array( 'enabled' => true, 'minWords' => 50 ),
			# Interest checkboxes (defined in MediaWiki:requestaccount-areas)
			'AreasOfInterest' => array( 'enabled' => true ),
			# CV/resume attachment option
			'CV'              => array( 'enabled' => true ),
			# Additional non-public info for reviewer
			'Notes'           => array( 'enabled' => true ),
			# Option to place web URLs that establish the user
			'Links'           => array( 'enabled' => true ),
			# Terms of Service checkbox
			'TermsOfService'  => array( 'enabled' => true ),
		);

		// Eww, do a 2d array merge so we don't wipe out settings
		global $wgConfirmAccountRequestFormItems;
		if ( $wgConfirmAccountRequestFormItems ) {
			foreach ( $features as $name => $settings ) {
				if ( isset( $wgConfirmAccountRequestFormItems[$name] ) ) {
					$wgConfirmAccountRequestFormItems[$name] += $settings;
				} else {
					$wgConfirmAccountRequestFormItems[$name] = $settings;
				}
			}
		} else {
			$wgConfirmAccountRequestFormItems = $features;
		}

WikiEditor has to do that too. And CollapsibleVector

Legoktm closed this task as Resolved.Aug 13 2015, 6:42 AM
Legoktm claimed this task.

This was fixed in rMW1ebb0f565966: registration: Overhaul merging of globals. To merge 2d settings, you need to set "_merge_strategy": "array_plus_2d" (for example: rEVEDea34d938c889: Revert "Revert "Replace wgVisualEditorNamespaces with an associative array"" in VE).

Ok should wikieditor be updated to do this.

Ok should wikieditor be updated to do this.

Yep, https://gerrit.wikimedia.org/r/#/c/240961/

Thanks. I've opened a bug about converting extensions that do this.