Page MenuHomePhabricator

Clarify whether VisualEditor requires TemplateData
Closed, ResolvedPublic

Description

When running MediaWiki/core ResourcesTest with VisualEditor but without TemplateData, a test fails:

1) ResourcesTest::testMissingMessages

Message 'templatedata-doc-subpage' required by 'ext.visualEditor.mwtransclusion' must exist
Failed asserting that false is true.
/workspace/src/tests/phpunit/structure/ResourcesTest.php:121

That comes from VisualEditor:

extension.json
{
    "ResourceModules": {
        "ext.visualEditor.mwtransclusion": {
            "group": "visualEditorA",
            "messages": [
                "quotation-marks",
                "redirectedfrom",
                "templatedata-doc-subpage",
                "visualeditor-changedesc-mwtransclusion",
...

quotation-marks and redirectedfrom come from MediaWiki core. templatedata-doc-subpage is not available unless TemplateData is. On CI that forces us to add TemplateData as a dependency for extensions that solely depends on VisualEditor (T389998).

TemplateData shows the message is intentionally not translated:

i18n/en.json:   "templatedata-doc-subpage": "doc",
i18n/qqq.json:  "templatedata-doc-subpage": "{{notranslate}}\nSub-page of template where documentation is stored by convention on the wiki.",

My aim is to have the ResourcesTests to pass for VisualEditor without TemplateData (unless it is really required).

Should VisualEditor requires TemplateData in extension.json?

Should templatedata-doc-subpage be renamed visualeditor-templatedata-doc-subpage and included in VisualEditor? That removes the message dependency of ext.visualEditor.mwtransclusion.

It looks like the message is used to check whether TemplateData is available (instead of mw.loader.getState( 'ext.templateData.templateDiscovery' ) which has the message templatedata-doc-subpage.

I am a bit at lost :] My aim would be, for CI, to avoid adding TemplateData whenever VisualEditor is a dependency.

Event Timeline

The specific check was added in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/483205 back in 2019 by @Jdforrester-WMF. On some cursory inspection, at the time that message wasn't included in any RL module in TemplateData (template discovery module is new, as of earlier this year), so there was nothing to check/load to get access to it client-side. I imagine it could be refactored now, but it made sense at the time. 🤷🏻

Thank you for the additional details @DLynch

I have a similar issue with for example:

  • Kartographer having "messages": [ "project-localized-name-commonswiki" ], a message provided by WikimediaMessages.
  • Translate having its ext.translate.ve requiring ext.visualEditor.mwcore

In the code, I imagine that is checked using mw.loader.getState() to find out whether the other extension is loaded.

For the resource loader definition tests/phpunit/structure/ResourcesTest.php unconditionally test it. I guess to verify that the resources actually exist, which is certainly a good thing. That kind of goes against my aim of making them optional and even if they were optional, we would still want to check whether they are valid.

Change #1189187 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] Load TemplateData message conditionally

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

Translate having its ext.translate.ve requiring ext.visualEditor.mwcore

ext.translate.ve is only ever loaded by config.attributes.VisualEditor.PluginModules, so having a dependency on VE there seems conceptually fine.

Change #1189187 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Load TemplateData message conditionally

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

Jdforrester-WMF assigned this task to Esanders.

Resolved: No.

I have confirmed my use case (CodeMirror + VisualEditor without TemplateData) has been fixed.

Thank you very much @Esanders, your patch has fixed 9 out of the 84 issues I am investigating 🎉

Change #1189349 had a related patch set uploaded (by Hashar; author: Esanders):

[mediawiki/extensions/VisualEditor@REL1_39] Load TemplateData message conditionally

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

Change #1189358 had a related patch set uploaded (by Hashar; author: Esanders):

[mediawiki/extensions/VisualEditor@REL1_43] Load TemplateData message conditionally

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

Change #1189365 had a related patch set uploaded (by Hashar; author: Esanders):

[mediawiki/extensions/VisualEditor@REL1_44] Load TemplateData message conditionally

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

Change #1189358 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@REL1_43] Load TemplateData message conditionally

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

Change #1189365 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@REL1_44] Load TemplateData message conditionally

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