Page MenuHomePhabricator

MonolingualTextInputWidget does not work outside of file pages
Open, Needs TriagePublic

Description

I just noticed that the AC/DC gadget (of which I’m the main developer) can’t add monolingual text statements when it’s loaded on a category page (but it can add them when loaded on a file page). The reason for this seems to be that MonolingualTextInputWidget initializes its ULS widget with a list of languages from the wbTermsLanguages config value:

this.language = new UlsWidget( {
	language: this.state.language,
	languages: mw.config.get( 'wbTermsLanguages' ),
	label: mw.message( 'wikibasemediainfo-monolingualtext-language-label' ).text()
} );

But that config value is only provided on file pages:

/**
 * @param \OutputPage $out
 * @param bool $isMediaInfoPage
 * @param string[] $termsLanguages Array with language codes as keys and autonyms as values
 * @param UserLanguageLookup $userLanguageLookup
 * @param DispatchingEntityViewFactory $entityViewFactory
 * @param array $jsConfigVars Variables to expose to JavaScript
 * @throws \OOUI\Exception
 */
public function doBeforePageDisplay(
	$out,
	$isMediaInfoPage,
	array $termsLanguages,
	UserLanguageLookup $userLanguageLookup,
	DispatchingEntityViewFactory $entityViewFactory,
	array $jsConfigVars = []
) {
	// Site-wide config
	$modules = [ 'wikibase.mediainfo.search' ];
	$moduleStyles = [];

	if ( $isMediaInfoPage ) {
		// ...
		$jsConfigVars = array_merge( $jsConfigVars, [
			// ...
			'wbTermsLanguages' => $termsLanguages,
			// ...
		] );
		// ...
	}

	$out->addJsConfigVars( $jsConfigVars );
	$out->addModuleStyles( $moduleStyles );
	$out->addModules( $modules );
}

I can work around this in the gadget by setting the wbTermsLanguages config before loading the WikibaseMediaInfo widgets, getting the list of languages from the API, but I’d rather avoid that.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 601337 had a related patch set uploaded (by Lucas Werkmeister; owner: Lucas Werkmeister):
[mediawiki/extensions/WikibaseMediaInfo@master] Move wbTermsLanguages from config to package module

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

I’ve verified on my local test wiki that the above change fixes this issue, at least as far as the AC/DC gadget is concerned.

Change 601337 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Move wbTermsLanguages from config to package module

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

LucasWerkmeister claimed this task.

Tested it on Beta, seems to be working.

Krinkle reopened this task as Open.EditedJun 3 2020, 10:48 PM
Krinkle added a subscriber: Krinkle.

Change 601337 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Move wbTermsLanguages from config to package module

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

I was happy to read this, but no longer when I realised it actually creates a new module just for this data key. See Guidelines). Move this to an existing module instead, which is where the power of package files truly comes through, as it allows virtual files to be added to any module.

The WikibaseMediaInfo extension is currently excessive at having 13 distinct module bundle entrypoints (stats). This should be reduced further, not increased.

From a quick glance, it seems the two modules that depend on this have lots of modules in common. Could it be shipped through one of those?

The config is used in the filePageDisplay and statements modules, and apparently filePageDisplay directly depends on statements, so I guess the config could be merged into the statements module.

(While looking at this, I also noticed that WBMI has several separate .styles modules – .filepage.styles, .mediasearch.styles, .statements.styles – and both lists them as dependencies of the respective non-styles module and directly adds them via addModuleStyles in PHP. That looks like it could be simplified to some extent as well.)

Change 602194 had a related patch set uploaded (by Lucas Werkmeister; owner: Lucas Werkmeister):
[mediawiki/extensions/WikibaseMediaInfo@master] Merge config module into statements module

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

It turns out that this issue also affected the upload wizard, not just AC/DC:



Hopefully that should be fixed with the next train.

Change 602194 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Merge config module into statements module

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