Last modified: 2014-07-20 18:56:26 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T62368, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 60368 - [upstream] @import in LESS should try relative path before consulting global import paths
[upstream] @import in LESS should try relative path before consulting global ...
Status: NEW
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.23.0
All All
: Normal minor (vote)
: ---
Assigned To: Nobody - You can work on this!
: upstream
Depends on:
Blocks: 64595
  Show dependency treegraph
 
Reported: 2014-01-23 12:48 UTC by Gabriel Birke
Modified: 2014-07-20 18:56 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Gabriel Birke 2014-01-23 12:48:48 UTC
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.
Comment 1 s7eph4n 2014-02-04 21:39:01 UTC
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>
Comment 2 Gerrit Notification Bot 2014-05-09 15:32:41 UTC
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
Comment 3 Gerrit Notification Bot 2014-05-09 15:33:12 UTC
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
Comment 4 Gerrit Notification Bot 2014-05-09 16:04:25 UTC
Change 132447 had a related patch set uploaded by Daniel Friesen:
Backport Iee47bdc23630e02ccfcbd28496ec5268892eb629 to 1.23.

https://gerrit.wikimedia.org/r/132447
Comment 5 Gerrit Notification Bot 2014-05-09 19:54:17 UTC
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
Comment 6 Gerrit Notification Bot 2014-05-10 18:04:01 UTC
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
Comment 7 Gerrit Notification Bot 2014-05-11 15:09:57 UTC
Change 132812 had a related patch set uploaded by MarkAHershberger:
Remove Vector from the default LESS import path

https://gerrit.wikimedia.org/r/132812
Comment 8 Gerrit Notification Bot 2014-05-11 15:17:20 UTC
Change 132812 merged by jenkins-bot:
Remove Vector from the default LESS import path

https://gerrit.wikimedia.org/r/132812
Comment 9 Krinkle 2014-05-15 22:13:27 UTC
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
Comment 10 s7eph4n 2014-05-27 19:43:14 UTC
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
Comment 11 Gerrit Notification Bot 2014-07-20 18:56:26 UTC
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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links