Page MenuHomePhabricator

score/extension.json should list VE dependency or move to conditional
Open, LowestPublic

Description

Without VE installed...

88) ResourcesTest::testMissingDependencies
The module 'ext.visualEditor.mwcore' required by 'ext.score.visualEditor' must exist
Failed asserting that an array contains 'ext.visualEditor.mwcore'.

/var/www/wiki/mediawiki/core/tests/phpunit/structure/ResourcesTest.php:82
/var/www/wiki/mediawiki/core/tests/phpunit/MediaWikiTestCase.php:400
/var/www/wiki/mediawiki/core/maintenance/doMaintenance.php:111

The extension doesn't have a dependency on VE defined in extension.json. It should either do so, or ext.score.visualEditor in ResourceModules should be conditionally defined if VE is installed. Same for VisualEditorPluginModules

Event Timeline

Reedy created this task.Apr 3 2017, 12:56 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 3 2017, 12:56 AM
Ebe123 added a subscriber: Ebe123.Aug 6 2017, 11:06 PM

extension.json:32:

"dependencies": [
  "ext.visualEditor.mwcore"
],

Is there still this error? VisualEditor at least seems to be listed as a dependency.

Reedy added a comment.Aug 7 2017, 12:14 AM

There is a dependency on RL module, sure, but if that isn't there, it'll either silently fail, or possibly break in non obvious ways. And also give unit test fails like above

See https://www.mediawiki.org/wiki/Manual:Extension_registration#Requirements_.28Dependencies.29 for starters; if the extension can't run without VE at all, it should be added like that. If it can run without VE, but in a "crippled" mode, only add the RL module dependancy if VE is loaded, see https://www.mediawiki.org/wiki/Manual:Extension_registration#Check.2C_if_an_extension_is_loaded_without_actually_require_it

CI was fixed 2 years ago in https://github.com/wikimedia/integration-config/commit/5bc7a81aa7d4e0798de6144ea6858ef7b68d7ed3

Ebe123 changed the task status from Duplicate to Declined.Aug 7 2017, 6:35 PM

Math declined the bug when applicable to their extension: T88231. Other than the error, the extension works even when VE is not installed.

Reedy added a comment.Aug 7 2017, 9:18 PM

I'm not sure that because another extension marked it as declined for it, means you can mark it declined for yours

You're causing a test failure for people not using the VE

Ebe123 reopened this task as Stalled.Aug 7 2017, 9:51 PM
Ebe123 triaged this task as Lowest priority.

There doesn't seem to be a solution using the ExtensionRegistry, as the VE plugin is exclusively JS/CSS. A solution would have to come from VE. The only solution was outdated from 2015 (MobileFrontend). Anyways, only tests fail (and VE), and none of the core functionality is missed. A solution is not really required, and would only come from VE or mediawiki, not from here. I do see your point however.

Reedy added a comment.Aug 8 2017, 6:13 AM

It's not exclusively js/CSS. It has php code, so the class_exists would work. But extension registry would work regardless as there is an extension.json and that's used as the entry point

Aklapper changed the task status from Stalled to Open.Aug 15 2020, 4:01 PM

I'm boldly reopening per last comment, as tasks shouldn't be stalled forever and as there is some new comment here. (If this ticket is still an issue nowadays.)