Page MenuHomePhabricator

ExtensionProcessor should prevent duplicate definitions for ResourceModuleSkinStyles
Open, MediumPublic

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.

Event Timeline

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
Krinkle added a project: good first task.
Krinkle added subscribers: D3r1ck01, Krinkle.

@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

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".

OK
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.

OK
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

OK
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 D3r1ck01.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-D3r1ck01. · View Herald TranscriptMay 20 2019, 9:42 PM
D3r1ck01 moved this task from Backlog to Doing [WIP] on the User-D3r1ck01 board.May 20 2019, 9:52 PM
D3r1ck01 removed D3r1ck01 as the assignee of this task.May 6 2020, 11:49 AM
D3r1ck01 removed a project: User-D3r1ck01.
CCicalese_WMF added a subscriber: CCicalese_WMF.

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!

matmarex removed a subscriber: matmarex.May 20 2020, 7:25 PM
Krinkle added a comment.EditedMay 21 2020, 12:56 AM

This is a task Aaron would like to write a patch for while on clinic duty next time.

Krinkle removed aaron as the assignee of this task.Mon, Jul 20, 8:25 PM
Krinkle added a subscriber: aaron.