Page MenuHomePhabricator

[upstream] @import in LESS should try relative path before consulting global import paths
Closed, ResolvedPublic

Description

Author: gabriel.birke

Description:
If the .less files of a custom skin mimic the file structure of Vector (styles.less containing @import statements for files in the skins/mystyle/components directory or called "variables"), they won't be loaded. Instead, the files from the skins/vector directory will be loaded.

This is because of the path precedence in $wgResourceLoaderLESSImportPaths. First, the LESS compiler is initialized with the import directories from wgResourceLoaderLESSImportPaths and only in the course of compiling the file from the custom skin, the path of the custom skin is added to the importDirs variable of the LESS compiler.

This behavior breaks the reasonable expectation of a skin developer that the included .less files are read from the skin directory.

Proposed workaround (if no code solution is possible): Put a comment in styles.less of the Vector skin, with a warning to choose a different file names than those of Vector in your own templates.


Version: 1.23.0
Severity: minor
See Also:
https://github.com/leafo/lessphp/issues/555

Details

Reference
bz60368

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 2:57 AM
bzimport set Reference to bz60368.
bzimport added a subscriber: Unknown Object (MLST).

The Vector path should not be put into $wgResourceLoaderLESSImportPaths at all. At least not in DefaultSettings. Took me quite a while to figure out, why all of a sudden my Bootstrap based skin did not work anymore. Turns out that Bootstrap also contains a variables.less file, but with a totally incompatible content of course.

The only reason this works for the stock MW is that only Vector uses LESS so far.

IMHO the solution should be to only include paths of modules that are actually loaded.

<rant>Quote from the discussion on gerrit (https://gerrit.wikimedia.org/r/99524/): "Submit a follow up - there are no deployments for some time so we have time to perfect this." Somebody remind we what was the reason for switching from SVN to git and reviews again?</rant>

Change 132440 had a related patch set uploaded by Daniel Friesen:
Add Vector to the LESS import path from within VectorBeta

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

Change 132441 had a related patch set uploaded by Daniel Friesen:
Remove Vector from the default LESS import path

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

Change 132447 had a related patch set uploaded by Daniel Friesen:
Backport Iee47bdc23630e02ccfcbd28496ec5268892eb629 to 1.23.

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

Change 132498 had a related patch set uploaded by Daniel Friesen:
ResourceLoader: Allow individual modules to define their own additional less import paths.

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

Change 132447 abandoned by Krinkle:
Backport Iee47bdc23630e02ccfcbd28496ec5268892eb629 to 1.23.

Reason:
Please maintain a useful summary and a matching Change-Id for the backport commit. The easiest way is to create this commit using cherry-pick. If it doesn't apply cleanly, manually re-use the same commit message.

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

Change 132812 had a related patch set uploaded by MarkAHershberger:
Remove Vector from the default LESS import path

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

Change 132812 merged by jenkins-bot:
Remove Vector from the default LESS import path

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

This is a bug in the php-port of less that we use. Both other ports as well as the official js implementation do it the right way and allow local path resolution to take precedence.

Upstream bug report:
https://github.com/leafo/lessphp/issues/555

Can't we just forget about lessc and replace it? Development seems to have stopped and new LESS features are not recognized.

See https://www.mediawiki.org/wiki/Requests_for_comment/Change_LESS_compilation_library

Change 132498 abandoned by Krinkle:
ResourceLoader: Allow individual modules to define their own additional less import paths.

Reason:
Closing for now. bug 64595 was fixed and bug 60368 is an upstream bug we mitigate by not adding any more import paths.

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

Krinkle changed the task status from Open to Stalled.Apr 28 2015, 6:43 PM
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.
Krinkle set Security to None.
Krinkle moved this task from Backlog to Reported Upstream on the Upstream board.
Krinkle removed a subscriber: Unknown Object (MLST).
Krinkle closed this task as Resolved.Aug 5 2016, 1:54 AM
Krinkle claimed this task.

rMWea862efa75a7: Replace leafo/lessphp with oyejorge/less.php

As of this commit, we no longer use the leafo/lessphp library in ResourceLoader.

Both the official javascript implementation, as well as the https://github.com/oyejorge/less.php port handle this correctly.

And the library we use now (oyejorge/less.php) does not have this bug.