Task description:

• Rescoped to not be never-ending but rather a specific major upgrade.
• Added a acceptance critera instead of "we did the task", which is undefined. Proposing to use the upstream test suite, like we do now.

@Catrope wrote at change 896197:

• Escape expressions inside of calc(), to prevent the Less compiler from trying to compute them at compile time. Currently this just affects one call to max(), but it could affect expressions like calc( @foo + @bar ) too if we add those in the future.

@Krinkle wrote in task description:

• v3.0.0 Changed: calc() exception. Ref https://lesscss.org/#operations-calc-exception

The ability to automatically exclude parts of CSS calc() from Less compilation, and to let the browser do that client-side, was indeed released in Less.js v3.0.0 and is in scope of this task. However, we backported that feature in T331688 and this has been released in less.php 4.0.0 and deployed to MediaWiki already.

However, this did not fully enable the Codex example. The upstream Less.js v3 (v3.13.1) and our less.php 4.0.0 took care of math operations and not functions like max(). Those are also unsupported upstream:

nobody:/Temp$ cat tmp.less 
.cdx {
    @a: 12px;
    @b: 2em;
    font-size: calc( max( @a, @b ) );
}

nobody:/Temp$ npm install less@3.13
added 14 packages, and audited 15 packages in 4s

nobody/Temp$ npx less tmp.less
ArgumentError: error evaluating function `max`: incompatible types in /Temp/tmp.less on line 4, column 22:
3     @b: 2em;
4     font-size: calc( max( @a, @b ) );
5 }

The above changed in Less.js v4, which is outside this task's scope. A minimal patch to support it as-is is welcome, but otherwise we'l get to that later.

• Don't call escape() on color values. The Node.js implementation of Less allows this (even back in version 2.5.3), but the PHP implementation doesn't, and it's not officially supported according to the Less documentation. Instead, use %A to convert these color values to strings and escape them at the same time.

I've traced this in the upstream codebase to, as it happens, exactly Less.js v2.5.2.

.cdx-fmt {
    @color: #000000;
    @escaped-color: %( '%A', @color );
    background-image: 'data:image/svg+xml;utf8,<svg fill="@{escaped-color}"></svg>';
}

.cdx-esc {
    @color: #000000;
    @escaped-color: escape( @color );
    background-image: 'data:image/svg+xml;utf8,<svg fill="@{escaped-color}"></svg>';
}

http://ecomfe.github.io/est/fiddle/#version=2.5.1&autoprefix=false&est=false&autorun=false

Produces:

.cdx-esc {
  background-image: 'data:image/svg+xml;utf8,<svg fill="undefined"></svg>';
}

Fixing this is in scope of this task, and indeed should have been fixed already.

In T288498#9274763, @Krinkle wrote:

• Don't call escape() on color values. The Node.js implementation of Less allows this (even back in version 2.5.3), but the PHP implementation doesn't, and it's not officially supported according to the Less documentation. Instead, use %A to convert these color values to strings and escape them at the same time.

I've traced this in the upstream codebase to, as it happens, exactly Less.js v2.5.2.

.cdx-fmt {
    @color: #000000;
    @escaped-color: %( '%A', @color );
    background-image: 'data:image/svg+xml;utf8,<svg fill="@{escaped-color}"></svg>';
}

.cdx-esc {
    @color: #000000;
    @escaped-color: escape( @color );
    background-image: 'data:image/svg+xml;utf8,<svg fill="@{escaped-color}"></svg>';
}

http://ecomfe.github.io/est/fiddle/#version=2.5.1&autoprefix=false&est=false&autorun=false

Produces:

.cdx-esc {
  background-image: 'data:image/svg+xml;utf8,<svg fill="undefined"></svg>';
}

Fixing this is in scope of this task, and indeed should have been fixed already.

It doesn't appear to be fixed yet. In current master:

catrope:~/git/less.php (master)$ ./bin/lessc -
.foo {
@color-black: #000000;
color: @color-black;
@foo1: %('%A', @color-black );
color: @foo1;
@foo2: escape( @color-black );
color: @foo2;
}

PHP Warning:  Undefined property: Less_Tree_Color::$value in /home/catrope/git/less.php/lib/Less/Functions.php on line 503
PHP Stack trace:
PHP   1. {main}() /home/catrope/git/less.php/bin/lessc:0
PHP   2. Less_Parser->getCss() /home/catrope/git/less.php/bin/lessc:189
PHP   3. Less_Tree_Ruleset->compile($env = class Less_Environment { public $currentFileInfo = ['entryPath' => '', 'entryUri' => '', 'rootpath' => '', 'currentDirectory' => '', 'currentUri' => 'anonymous-file-0.less', 'filename' => 'anonymous-file-0.less', 'uri_root' => '']; public $importMultiple = FALSE; public $frames = [0 => class Less_Tree_Ruleset { ... }, 1 => class Less_Tree_Ruleset { ... }]; public $mediaBlocks = []; public $mediaPath = []; public $functions = [] }) /home/catrope/git/less.php/lib/Less/Parser.php:182
PHP   4. Less_Tree_Ruleset->compile($env = class Less_Environment { public $currentFileInfo = ['entryPath' => '', 'entryUri' => '', 'rootpath' => '', 'currentDirectory' => '', 'currentUri' => 'anonymous-file-0.less', 'filename' => 'anonymous-file-0.less', 'uri_root' => '']; public $importMultiple = FALSE; public $frames = [0 => class Less_Tree_Ruleset { ... }, 1 => class Less_Tree_Ruleset { ... }]; public $mediaBlocks = []; public $mediaPath = []; public $functions = [] }) /home/catrope/git/less.php/lib/Less/Tree/Ruleset.php:96
PHP   5. Less_Tree_Rule->compile($env = class Less_Environment { public $currentFileInfo = ['entryPath' => '', 'entryUri' => '', 'rootpath' => '', 'currentDirectory' => '', 'currentUri' => 'anonymous-file-0.less', 'filename' => 'anonymous-file-0.less', 'uri_root' => '']; public $importMultiple = FALSE; public $frames = [0 => class Less_Tree_Ruleset { ... }, 1 => class Less_Tree_Ruleset { ... }]; public $mediaBlocks = []; public $mediaPath = []; public $functions = [] }) /home/catrope/git/less.php/lib/Less/Tree/Ruleset.php:96
PHP   6. Less_Tree_Value->compile($env = class Less_Environment { public $currentFileInfo = ['entryPath' => '', 'entryUri' => '', 'rootpath' => '', 'currentDirectory' => '', 'currentUri' => 'anonymous-file-0.less', 'filename' => 'anonymous-file-0.less', 'uri_root' => '']; public $importMultiple = FALSE; public $frames = [0 => class Less_Tree_Ruleset { ... }, 1 => class Less_Tree_Ruleset { ... }]; public $mediaBlocks = []; public $mediaPath = []; public $functions = [] }) /home/catrope/git/less.php/lib/Less/Tree/Rule.php:82
PHP   7. Less_Tree_Expression->compile($env = class Less_Environment { public $currentFileInfo = ['entryPath' => '', 'entryUri' => '', 'rootpath' => '', 'currentDirectory' => '', 'currentUri' => 'anonymous-file-0.less', 'filename' => 'anonymous-file-0.less', 'uri_root' => '']; public $importMultiple = FALSE; public $frames = [0 => class Less_Tree_Ruleset { ... }, 1 => class Less_Tree_Ruleset { ... }]; public $mediaBlocks = []; public $mediaPath = []; public $functions = [] }) /home/catrope/git/less.php/lib/Less/Tree/Value.php:24
PHP   8. Less_Tree_Call->compile($env = class Less_Environment { public $currentFileInfo = ['entryPath' => '', 'entryUri' => '', 'rootpath' => '', 'currentDirectory' => '', 'currentUri' => 'anonymous-file-0.less', 'filename' => 'anonymous-file-0.less', 'uri_root' => '']; public $importMultiple = FALSE; public $frames = [0 => class Less_Tree_Ruleset { ... }, 1 => class Less_Tree_Ruleset { ... }]; public $mediaBlocks = []; public $mediaPath = []; public $functions = [] }) /home/catrope/git/less.php/lib/Less/Tree/Expression.php:45
PHP   9. Less_Functions->escape($str = class Less_Tree_Color { public $parensInOp = FALSE; public $extendOnEveryPath = NULL; public $allExtends = NULL; public $rgb = [0 => 0, 1 => 0, 2 => 0]; public $alpha = 1; public $isTransparentKeyword = NULL }) /home/catrope/git/less.php/lib/Less/Tree/Call.php:84
.foo {
  color: #000000;
  color: '%23000000';
  color: ;
}

Change 1005040 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/vendor@master] Update wikimedia/less.php from v4.1.1 to v4.2.0

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

Change 1006166 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/core@master] composer.json: Use `wikimedia/less.php` version 4.2.1

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

Change 1005040 merged by jenkins-bot:

[mediawiki/vendor@master] Update `wikimedia/less.php` from version 4.1.1 to 4.2.1

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

Change 1006166 merged by jenkins-bot:

[mediawiki/core@master] composer.json: Use `wikimedia/less.php` version 4.2.1

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

I can confirm that the bug I mentioned in T288498#9286018 is fixed in version 4.2.1 of Less.php.

Change #1023945 had a related patch set uploaded (by Krinkle; author: Bartosz Dziewoński):

[mediawiki/libs/less.php@master] Remove backtick evaluation inside quoted strings

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

Change #1026571 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/libs/less.php@master] test: Enable lessjs-3.13.1/compression fixture

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

Change #1026571 merged by jenkins-bot:

[mediawiki/libs/less.php@master] test: Enable lessjs-3.13.1/compression fixture

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

@larissagaulia @Krinkle - I have a suggestion to rename this ticket to "Update less.php port to fully support Less.js 2.5.3 behaviours", and put in the description that the acceptance criteria is that all test fixtures PHP version can support from 2.5.3 have to pass.
Then use this ticket description and create a new ticket to support the less-3.13.

My rationale is that most of the work we did circled 2.5.3 fixtures. Now I think we finally have a 100% parity (with what's possible) with 2.5.3 - it would be good to mark that as a milestone. That would give a good clarity on the work that is done, and what is remaining. IMHO achieving full 2.5.3 parity is a great moment to celebrate.

Plenty of work we did brought us closer to 3.13, but most of the work is based on test/assets/less-2.5.3js, not 3.x version.

Change #1034195 had a related patch set uploaded (by 沈澄心; author: 沈澄心):

[mediawiki/libs/less.php@master] update Less_Parser::parseEntitiesKeyword() to support some Less.js 3.13 behaviours

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

@pmiazga Agreed, with one minor difference. I'd prefer not to change the goal of the current task as this was filed by specific people with a specific rationale in relation to future Codex usage, which I'd rather preserve in-place. I've instead subdivided the work to recognise this milestone as T365481: Update less.php port to complete support Less.js 2.5.3 behaviours, which we can close once T364871: Add support for SVG image size is resolved.

We can then cut the last Less.php 4.x release and focus back on this task for remaining of the quarter. How much we can get done will depend on the unknowns we run into, but I'm hopeful we achieve these two releases:

• Publish Less.php 5.0, to start off the new series. This would essentially be a minimal release only the breaking changes (new maths/strictMaths behaviour to reflect upstream Less.js 3 and Less.js 4), removal of deprecated features, and any changes to default parser options (users can opt-in/out as they see fit).
• Publish one Less.php 5.x with a handful of Less 3.13 features of our choosing.

That works too. Thank you for handling that @Krinkle

Change #1036271 had a related patch set uploaded (by Hokwelum; author: Hokwelum):

[mediawiki/libs/less.php@master] Less.js-3.13.1 Source code

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

Change #1036271 merged by jenkins-bot:

[mediawiki/libs/less.php@master] Add copy of Less.js 3.13.1 source code

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

Change #1023945 merged by jenkins-bot:

[mediawiki/libs/less.php@master] Remove backtick evaluation inside quoted strings

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