Extension registration does not prefix path in "ResourceLoaderLESSImportPaths" entries
Closed, InvalidPublic

Description

Currently it is treated as a normal global, but it needs path prefixing in front.

Legoktm created this task.Aug 6 2015, 11:52 PM
Legoktm updated the task description. (Show Details)
Legoktm raised the priority of this task from to Needs Triage.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 6 2015, 11:52 PM

So...the doc comment for this says:

* Extensions need not (and should not) register paths in
* $wgResourceLoaderLESSImportPaths. The import path includes the path of the
* currently compiling LESS file, which allows each extension to freely import
* files from its own tree.

I see usages in Flow and MobileFrontend. If extensions "should not" be using this, we shouldn't support it in extension registration.

Flow and MobileFrontend have variables files. For MobileFrontend this would mean a mass replacement of
@import "minerva.variables";
to
@import "minerva.variables";

import "../../minerva.less/variables.less" (relative file path urls will differ) which although inconvenient/a pain in the ass is not the end of the world.

I guess my question is why would we not want to share LESS (what about extensions such as Gather that make use of the Minerva skin variables to give a consistent styling in that skin?) I think skins should certainly be able to add to global import paths, albeit maybe in a more controlled way.

The config documentation should be updated to be less vague if we do not want people to update this:
/need not (and should not)/ => /should not/

ori added a comment.Apr 25 2016, 11:58 PM

albeit maybe in a more controlled way.

That's crucial, right? Otherwise it's no different from extensions using global scope to share variables in PHP. There is no namespace policy in effect to prevent collisions, and no mechanism for declaring or versioning cross-extension LESS dependencies. These things always seem a little fussy until you actually need them, at which point it is too late.

Legoktm claimed this task.Jun 22 2016, 10:00 PM

Change 295647 had a related patch set uploaded (by Legoktm):
registration: Remove broken ResourceLoaderLESSImportPaths support

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

Krinkle added a subscriber: Krinkle.Jul 6 2016, 2:15 AM

I think skins should certainly be able to add to global import paths, albeit maybe in a more controlled way.

Global import paths (for all skins) would cause (and have caused) unintuitive dependencies between projects, and a high maintenance burden.

Related: T62368: [upstream] @import in LESS should try relative path before consulting global import paths

Any stylesheet not explicitly part of a Skin (e.g. Vector) should be skin-agnostic. That does not mean skins shouldn't be able to share information with those other stylesheets, but it means it should be in a generic way, not bound to a specific skin. We'd standardise a set of variable names that different OOUI themes and/or MediaWiki skins can set (and that extensions can include in their own stylesheets). And the import statement would end up resolving based on the skin relevant to the current request context (already supported by ResourceLoader).

Related:

Change 295647 merged by jenkins-bot:
registration: Remove broken ResourceLoaderLESSImportPaths support

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

Krinkle closed this task as Resolved.Jul 6 2016, 5:57 PM
Jdlrobson reopened this task as Open.EditedJul 6 2016, 6:17 PM

Reopening given https://phabricator.wikimedia.org/T108271#1694029 doesn't seem to have been addressed and this is still being used in MobileFrontend here (now in a undocumented fashion):
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileFrontend.hooks.php#L1419

Reopening given https://phabricator.wikimedia.org/T108271#1694029 doesn't seem to have been addressed

I think both Ori and Krinkle addressed it...

and this is still being used in MobileFrontend here (now in a undocumented fashion):
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileFrontend.hooks.php#L1419

So we need a separate tech-debt bug to clean up MobileFrontend?

Krinkle closed this task as Invalid.Jul 19 2016, 7:27 PM

This task is specific to a bug in extension registry (extension.json). Which became obsolete when the feature was removed from the extension registry in 0625f4e161. Closing this task as such.

There isn't an issue currently with Flow and MobileFrontend's use of this variable. Those are working fine. Do note that T140807 also calls for deprecating and removing this feature in general.

Restricted Application removed a subscriber: Liuxinyu970226. · View Herald TranscriptJul 19 2016, 7:27 PM