Page MenuHomePhabricator

$wgNamespacesWithSubpages can include missing namespaces, increasing VisualEditor JS config variables size
Closed, ResolvedPublic

Description

Noticed while investigating something else, the javascript mw.config setting 'wgVisualEditorConfig', which is loaded on every request on wikis where VisualEditor is installed, includes configuration of namespaces with subpages, and sometimes those namespaces don't exist.
Checked in the console:

var nsWithConfigForSubpages = Object.keys( mw.config.get( 'wgVisualEditorConfig' ).namespacesWithSubpages );
var existingNamespaces = Object.keys( mw.config.get( 'wgFormattedNamespaces' ) );
var missingNsWithConfig = nsWithConfigForSubpages.filter( function ( ns ) {
    return existingNamespaces.indexOf( ns ) === -1;
} );
// ['100', '101', '102', '103', '104', '105', '106', '107', '108', '109', '110', '111', '112', '113', '114', '115', '116', '117', '118', '119', '208', '209', '210', '830', '831']

As far as I can track down, these come from:
100-119: WMF config InitialiseSettings.php sets these to have subpages enabled by default, saying "extra namespaces usually want subpages"
208-209: used to be used for "Programs" and "Programs talk" namespaces that were removed in T61837
210: no idea
830-831: SecurePoll NS_SECUREPOLL and NS_SECUREPOLL_TALK, extension.json registration says its conditional and the namespaces are only enabled if $wgSecurePollUseNamespace is true, but the subpages settings are stored anyway

We can probably just remove the 208 and 209, but I would suggest having Setup.php filter $wgNamespacesWithSubpages to remove any that don't exist. For SecurePoll, maybe extension registration should also not update the other settings if the namespace is conditional? Currently only the addition to ExtensionNamespaces is conditional, handling of immovable/gendered/subpages/content/content model/protected namespaces is done regardless.

Or, if it doesn't matter for the rest of the code if the namespace exists or not, VisualEditor should filter out the missing namespaces instead.

Event Timeline

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

It's intentional that $wgNamespacesWithSubpages has namespaces that don't exist, because it's harmless to have them there. Nothing should be reading the value of namespaces which aren't configured. In PHP, it's a minimal increase in memory usage but doesn't take up extra runtime filtering those namespaces out.

That said, I think filtering them out before it reaches JS (so in VisualEditor) makes sense if the extra payload size merits it.

It's intentional that $wgNamespacesWithSubpages has namespaces that don't exist, because it's harmless to have them there. Nothing should be reading the value of namespaces which aren't configured. In PHP, it's a minimal increase in memory usage but doesn't take up extra runtime filtering those namespaces out.

That said, I think filtering them out before it reaches JS (so in VisualEditor) makes sense if the extra payload size merits it.

Okay, will filter by namespaces that exist before being added to VE config

Change 724793 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/VisualEditor@master] Filter JavaScript namespacesWithSubpages to only existing namespaces

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

Change 724793 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Filter JavaScript namespacesWithSubpages to only existing namespaces

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

matmarex assigned this task to DannyS712.
matmarex removed a project: Patch-For-Review.

Change 725300 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Rewrite ApiVisualEditor::getAvailableNamespaceIds()

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

Change 725300 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Rewrite ApiVisualEditor::getAvailableNamespaceIds()

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