Migrate Less compilation to `ipl\Web\Less` by lippserd · Pull Request #5492 · Icinga/icingaweb2

lippserd · 2026-04-09T15:23:13Z

Because the in-tree Less visitor requires wikimedia/less.php, that dependency has to live in icinga-php-thirdparty. icinga-php-library already carries Less-related code with the same dependency, so this PR consolidates everything into icinga-php-library and removes the duplicate in-tree implementation.

Remove the in-tree Icinga\Less\ visitor layer in favor of the shared ipl\Web\Less library, which maintains modernized equivalent CSS var replacement and light-mode functionality.
Strip unused variable-export support from LessCompiler — the feature was never used by any our modules or community modules.
Update LessCompiler to delegate to ipl\Web\Less, assembling sources via @import (less) directives; pass the $minify flag directly to render() instead of via a separate compress() call.
Modernize LessCompiler with PHP type declarations, short array syntax, and tightened PHPDoc.

For testing, use 7da4917 as the baseline, which adapts light mode rule evaluation order to match the new visitor.

Here is my test summary:

The stylesheets are nearly identical. All but one difference are cosmetic normalizations introduced by the new version of the wikimedia Less parser:

Category	Example
Decimal leading zero	`.5em` → `0.5em`
Hex color shortening	`#dddddd` → `#ddd`
Color name normalization	`#ffffff` → `white`, `#ff0000` → `red`
`transparent` → `rgba(0,0,0,0)`	fallback in `var()` expressions
Trailing `.0` stripped	`2.0em` → `2em`, `1.0` → `1`
`calc()` kept as fractions	`calc(14.28571429%)` → `calc(100% / 7)`
Font shorthand explicit line-height	`1em` → `1em/1`

There is one functional difference: the light-mode background color gains an extra level of indirection via a new CSS var call (because it uses @light-body-bg-color):

-  --body-bg-color: #f5f9fa;
+  --body-bg-color: var(--light-body-bg-color, #f5f9fa);

The new CSS var replacement visitor from ipl\Web runs independently from the light mode call visitor, and therefore also adds var overrides to such detached rulesets. Everything else is normalization noise.

closes #5457
requires Icinga/icinga-php-thirdparty#115
requires Icinga/ipl-web#365

This is a preparation for using the new Less utils from `ipl\Web` to simplify the comparison of the old and new stylesheets.

This reverts commit 7c58b3c. Remove variable exports as they were never used in our modules or community modules, and using this feature is discouraged.

Replace the in-tree `Icinga\Less\` visitor layer with its equivalents from `ipl\Web\Less`, where the same color-variable and light-mode functionality is maintained as a shared library. `LessCompiler` is updated to delegate to `WikimediaLessCompiler`, assembling source via `@import (less)` directives and accepting a `$minify` flag directly on `render()` rather than through a separate `compress()` call.

Add missing PHP type declarations to properties and methods, replace long-form `array()` syntax with short `[]` syntax, tighten PHPDoc comments, and correct "LESS" to "Less".

lippserd · 2026-04-10T14:43:28Z

 );

 if (in_array($path, $special)) {
+    putenv('ICINGAWEB_ENVIRONMENT=production');


Please note that this must be removed before merging the PR.

sukhwinder33445 · 2026-04-14T14:00:49Z

I compared the minified CSS output between the main branch and the less branch (both icingaweb2 and ipl-web) and found the following issues:

Decimal leading zero: The change is applied in the wrong direction: 0.5em is being converted to .5em, but it should be the other way around (.5em → 0.5em) as you mentioned above.
Missing semicolon: CSS properties before the closing brace (}) are missing their trailing semicolon (;).
Empty CSS values now throw an error: previously, empty string values were rendered without issue. For example, when trying to configure a new schedule in the Notifications Web module, the following error is thrown:

Here the getColor() returns empty string in this case.

LESS division not evaluated: A LESS expression that previously compiled correctly to max-height: calc(100vh - 4.16666667em); now outputs invalid CSS: max-height: calc(100vh - 50/12em);. According to the LESS docs, to evaluate the division, a LESS variable must be used.

lippserd · 2026-04-14T19:53:12Z

Decimal leading zero: The change is applied in the wrong direction: 0.5em is being converted to .5em, but it should be the other way around (.5em → 0.5em) as you mentioned above.

Are you sure about that?

2. Missing semicolon: CSS properties before the closing brace (}) are missing their trailing semicolon (;).

Why do you think that's an issue?

3. Empty CSS values now throw an error: previously, empty string values were rendered without issue. For example, when trying to configure a new schedule in the Notifications Web module [...]

That's interesting (since this results in property: ; during rendering, which is obviously just invalid), and I wonder what happened before — were the rules simply ignored? Anyway, PHPDoc specifies that a string must be passed, and the notification module violates that. That needs to be fixed there. I'll consider adapting Style /LessRuleset accordingly so that its contract is actually enforced.

4. LESS division not evaluated: A LESS expression that previously compiled correctly to max-height: calc(100vh - 4.16666667em); now outputs invalid CSS: max-height: calc(100vh - 50/12em);. According to the LESS docs, to evaluate the division, a LESS variable must be used.

That's a good find, and it's been invalid CSS/Less since it was introduced. Now that you've brought it up, would you like to add a fix to this PR?

sukhwinder33445 · 2026-04-15T07:38:01Z

Are you sure about that?

Yes, looked again at the before and after CSS minified files and found that in most cases the leading zero has been removed, but in some places the change is applied as you mentioned above, and in others it remains unchanged. So we have three different behaviors, I think there should be consistency here.

Removed leading zero:

/*Before*/
.icinga-module.module-icingadb .item-layout.downtime .visual {
    ...
    padding: 0.5em 0.25em;
    /* The source file (`icingadb/downtime.less`) does not contain the leading zero, so I guess the LESS compiler was adding it before. */
}

/*After*/
.icinga-module.module-icingadb .item-layout.downtime .visual {
    ...
    padding: .5em .25em;
}

A zero is added:

 /*Before*/
.icinga-module.module-icingadb .row-item .performance-data {
    ...
    margin-left: -.25em;
}

 /*After*/
.icinga-module.module-icingadb .row-item .performance-data {
    ...
    margin-left: -0.25em
}

Remains the same in before and after cases:

.icinga-module.module-icingadb .row-item .performance-data .inline-pie {
   ...    
   margin-left: .25em
}

Why do you think that's an issue?

Yes, it is not an issue, but I thought it could be an unwanted change since it was not mentioned in the changes table above. Or did you mention it as normalization noise?

That's a good find, and it's been invalid CSS/Less since it was introduced. Now that you've brought it up, would you like to add a fix to this PR?

Okey, i will push a commit.

lippserd · 2026-04-15T07:51:16Z

Are you sure about that?

Yes, looked again at the before and after CSS minified files [...]

We have no control over how the Wikimedia Less parser minifies the CSS. Therefore, it only makes sense to compare the unminified versions.

Why do you think that's an issue?

Yes, it is not an issue, but I thought it could be an unwanted change since it was not mentioned in the changes table above. Or did you mention it as normalization noise?

Same as above.

sukhwinder33445 · 2026-04-15T09:37:06Z

Yes, you are right. That's the minifier which removes the leading zero and the semicolon. The non-minified file contains only the changes mentioned above.

This is invalid since `wikimedia/less.php:v5.0.0` : https://gerrit.wikimedia.org/r/c/mediawiki/libs/less.php/+/1049149 Less only evaluates its own variables, escaping won't help here. Use a Less variable to evaluate the divison correctly, and remove the unnecessary escape characters.

nilmerg

Category Example

Font shorthand explicit line-height 1em → 1em/1

That's not caused by normalization but by not evaluating it as division. But these variables don't seem to be used anyway, so that's why it wasn't an issue previously and the change won't be noticed now.

nilmerg · 2026-04-17T09:45:35Z

        if ($theme === null || (is_file($theme) && is_readable($theme))) {
            $this->theme = $theme;


I wonder why we didn't use realpath here as we use it for other less files already. Now that you use @import, I think it's safer to use it for theme files as well.

nilmerg · 2026-04-17T09:47:36Z

+    public function setThemeMode(string $themeMode): static
    {
        if (is_file($themeMode) && is_readable($themeMode)) {
            $this->themeMode = $themeMode;


Same here, realpath won't hurt.

nilmerg · 2026-04-17T09:49:19Z

+LESS;
+        $compiler = new WikimediaLessCompiler([
+            // Disable URL adjustment - Less.php tries to make URLs relative to the file path when using @import.
+            'relativeUrls' => false,


What's the exact reason for this? I enabled it again but didn't spot an issue. And looking at wikimedia's implementation, I think it's safer to enable since it escapes certain characters it seems.

You should see issues, e.g., fonts not loading, logo not loading.

Right. Then the comment explaining it is misleading as it doesn't affect @import in any way. Even tried to import a realative path with this enabled and it worked fine.

lippserd · 2026-04-17T10:12:27Z

Font shorthand explicit line-height
1em → 1em/1

That's not caused by normalization but by not evaluating it as division. But these variables don't seem to be used anyway, so that's why it wasn't an issue previously and the change won't be noticed now.

The new version of the Wikimedia Less parser handles mathematical expressions differently, which is why we have to specify 'math' => 'always'. Nevertheless, this particular expression is no longer evaluated.

nilmerg · 2026-04-17T11:51:39Z

The upgrading docs need to mention the changes, especially those to mathematical expressions.

cla-bot bot added the cla/signed label Apr 9, 2026

lippserd force-pushed the less branch from db7009d to 7d41d0f Compare April 9, 2026 15:31

lippserd mentioned this pull request Apr 10, 2026

composer remove wikimedia/less.php Icinga/icinga-php-thirdparty#115

Merged

lippserd added 4 commits April 10, 2026 15:22

Evaluate the light mode rules immediately

7da4917

This is a preparation for using the new Less utils from `ipl\Web` to simplify the comparison of the old and new stylesheets.

Revert "LessCompiler: Add support for variable exports by modules"

6a89757

This reverts commit 7c58b3c. Remove variable exports as they were never used in our modules or community modules, and using this feature is discouraged.

Modernize LessCompiler with typed properties and method signatures

d8b2519

Add missing PHP type declarations to properties and methods, replace long-form `array()` syntax with short `[]` syntax, tighten PHPDoc comments, and correct "LESS" to "Less".

lippserd force-pushed the less branch from 1c6d380 to d8b2519 Compare April 10, 2026 13:28

lippserd mentioned this pull request Apr 10, 2026

Add Less compiler and pre-eval AST visitors for CSS var support Icinga/ipl-web#365

Open

lippserd changed the title ~~Use Less utils from ipl-web~~ Migrate Less compilation to ipl\Web\Less Apr 10, 2026

lippserd added this to the 2.14.0 milestone Apr 10, 2026

lippserd marked this pull request as ready for review April 10, 2026 14:40

lippserd requested a review from nilmerg April 10, 2026 14:40

lippserd commented Apr 10, 2026

View reviewed changes

sukhwinder33445 force-pushed the less branch 2 times, most recently from 7c96160 to 0af9a40 Compare April 15, 2026 09:34

sukhwinder33445 force-pushed the less branch from 0af9a40 to 74b48ec Compare April 15, 2026 10:06

nilmerg reviewed Apr 17, 2026

View reviewed changes

nilmerg added the affects-upgrades The change requires migration or user awareness label Apr 17, 2026

nilmerg assigned lippserd Apr 17, 2026

		if ($theme === null \|\| (is_file($theme) && is_readable($theme))) {
		$this->theme = $theme;

Conversation

lippserd commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lippserd Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

sukhwinder33445 commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lippserd commented Apr 14, 2026

Uh oh!

sukhwinder33445 commented Apr 15, 2026

Removed leading zero:

A zero is added:

Remains the same in before and after cases:

Uh oh!

lippserd commented Apr 15, 2026

Uh oh!

sukhwinder33445 commented Apr 15, 2026

Uh oh!

nilmerg left a comment

Choose a reason for hiding this comment

Uh oh!

nilmerg Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

nilmerg Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

nilmerg Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

lippserd Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nilmerg Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

lippserd commented Apr 17, 2026

Uh oh!

nilmerg commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lippserd commented Apr 9, 2026 •

edited

Loading

sukhwinder33445 commented Apr 14, 2026 •

edited

Loading

lippserd Apr 17, 2026 •

edited

Loading