ExtensionProcessor should prevent duplicate definitions for ResourceModuleSkinStyles
Open, MediumPublic
Actions

Assigned To

None

Authored By

	Jdforrester-WMF
	Jun 7 2017, 5:27 PM

Description

There can be only 1 styles override for a combination of (module name) and (skin name).

If two extensions or skins try to define the same ResourceModuleSkinStyles override, then ExtensionProcessor should throw an exception.

Right now, when ExtensionProcessor is reading the second extension or skin, it is removing the styles from the first one.

This is a mistake. We should throw an error when this happens, so that a developer can take a look and decide which one should be removed.

This test could have helped prevent T167216.

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	registration: Avoid duplicate definitions for ResourceModuleSkinStyles	mediawiki/core	master	+7 -0
	registration: Prevent duplicate definitions of ResourceModuleSkinStyles	mediawiki/core	master	+8 -0

Customize query in gerrit

Related Objects

Mentioned Here: T167216: $wgResourceModuleSkinStyles customizations are not being applied for Vector because MobileFrontend accidentally overrides them

Event Timeline

Jdforrester-WMF created this task.Jun 7 2017, 5:27 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 7 2017, 5:27 PM

Krinkle triaged this task as Medium priority.Jul 8 2017, 3:40 AM

Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.

Restricted Application added a project: Performance-Team. · View Herald TranscriptApr 6 2019, 8:24 PM

@D3r1ck01 Perhaps you'd like to try this one.

Change 503337 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] registration: Prevent duplicate definitions of ResourceModuleSkinStyles

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

gerritbot added a project: Patch-For-Review.Apr 12 2019, 12:22 PM

Change 503379 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] registration: Avoid duplicate definitions for ResourceModuleSkinStyles

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

@D3r1ck01

Good scenario 1:

We have two skins. They both define their own styles in their own skin, for a module "ext.foo".

Vector/skin.json#wgResourceModuleSkinStyles. "vector" . "ext.foo"
MonoBook/skin.json#wgResourceModuleSkinStyles. "monobook" . "ext.foo"

Good scenario 2:

We have two extensions. They both define a different module.

ParserFunctions/extension.json # wgResourceModules . "ext.aaa"
Cite/extension.json # wgResourceModules . "ext.bbb"

Good scenario 3:

We have an extension and a skin. The extension defines a module. The skin adds their styles to that module

ParserFunctions/extension.json # wgResourceModules . "ext.aaa"
Vector/skin.json # wgResourceModuleSkinStyles . "vector" . "ext.aaa"

Bad scenario 1:

We have two extensions. They both define the same module via "ResourceModules" in their extension.json.

ParserFunctions/extension.json # ResourceModules . "ext.foo"
Cite/extension.json # ResourceModules . "ext.foo"

This must give an error.

Bad scenario 2:

We have an extension. It defines the same module name that already exists in MediaWiki core.

mediawiki-core/Resources.php # "mediawiki.foo"
Cite/extension.json # ResourceModules . "mediawiki.foo"

This must give an error.

Bad scenario 3:

We have two skins. They both define styles for module "ext.foo" in the same skin "x".

Vector/skin.json # wgResourceModuleSkinStyles . "x" . "ext.foo"
MonoBook/skin.json # wgResourceModuleSkinStyles . "x" . "ext.foo"

This must give an error.

Krinkle renamed this task from ExtensionProcessor should prevent duplicate definitions for ResourceModuleSkinStyles, throwing an exception if they happen to ExtensionProcessor should prevent duplicate definitions for ResourceModuleSkinStyles.Apr 12 2019, 7:44 PM

Krinkle updated the task description. (Show Details)

Thanks a lot! The current state of the patch handles all these scenarios. Test suite accompanying it shortly :), thanks a lot for your inputs @Krinkle.

Krinkle assigned this task to xSavitar.Apr 13 2019, 3:35 PM

Krinkle removed a project: good first task.

Change 503337 abandoned by D3r1ck01:
registration: Prevent duplicate definitions of ResourceModuleSkinStyles

Reason:
In favor of Idf6c02d60f12765dfa109

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

Restricted Application added a project: User-xSavitar. · View Herald TranscriptMay 20 2019, 9:42 PM

xSavitar moved this task from Backlog to Doing [WIP] on the User-xSavitar board.May 20 2019, 9:52 PM

Krinkle added a project: patch-welcome.Jun 6 2019, 9:42 AM

Krinkle removed a project: Patch-For-Review.Jul 18 2019, 1:30 AM

xSavitar removed xSavitar as the assignee of this task.May 6 2020, 11:49 AM

xSavitar removed a project: User-xSavitar.

Krinkle added a project: Developer Productivity.May 20 2020, 5:30 PM

Krinkle edited projects, added Sustainability (Incident Followup); removed Developer Productivity.

• Gilles assigned this task to aaron.May 20 2020, 5:33 PM

• Gilles added a project: Platform Team Workboards (Clinic Duty Team).

It looks like this is a request for code review from Platform Engineering? Assigning to Platform Team Workboards (External Code Reviews), but please respond if the request is for something else. In the future, please tag Platform Engineering (instead of Platform Team Workboards (Clinic Duty Team)) and comment with the nature of the request. Thanks!