wikimedia / less.php

Less.js ported to PHP. Mirrored from https://gerrit.wikimedia.org/g/mediawiki/libs/less.php/
Apache License 2.0
111 stars 195 forks source link

Block comments in @font-face are moved to bottom of selector #106

Closed bentideswell closed 9 months ago

bentideswell commented 9 months ago

Less_Tree_Ruleset is moving comments from their true position to the bottom of the selector. Consider the following CSS:

& when (@media-common = true) { @font-face { font-weight: @_weight_normal; /* This is a comment */ font-style: normal; /* Another Comment */ } }

After parsing, this becomes:

& when (@media-common = true) { @font-face { font-weight: @_weight_normal; font-style: normal; /* This is a comment */ /* Another Comment */ } }

This breaks out internal systems which uses block comments to denote critical CSS. Rules are then extracted as part of our post processing routine into critical and non-critical CSS.

The following line causes this:

https://github.com/wikimedia/less.php/blob/6cb92a99774b1228f5f1061d3aba34a7d54a7bab/lib/Less/Tree/Ruleset.php#L383

Commenting out this line fixes the issue so that comments are placed correctly.

Krinkle commented 9 months ago

Thanks for reporting this issue!

It appears this behaviour is consistent with upstream Less.js 2.4.0, but this changed in Less.js 2.5.0, and we have filed to ported over that change.

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

Example input:

.plain {
    font-weight: 100; /* One */
    font-style: normal; /* Two */
}

.fn1() {
    .output {
        font-weight: 200; /* One */
        font-style: normal; /* Two */
    }
}
.fn1();

.fn2() {
    @font-face {
        font-weight: 300; /* One */
        font-style: normal; /* Two */
    }
}
.fn2();

@font-face {
    font-weight: 400; /* One */
    font-style: normal; /* Two */
}

I've imported this issue at https://phabricator.wikimedia.org/T356706.

bentideswell commented 9 months ago

Great, thanks for confirming and importing the issue. I'm working with this package via Magento 2, so it might be a while before I see this fix in my day to day, but good to know it is being resolved.

Krinkle commented 8 months ago

Released in v4.2.0, https://github.com/wikimedia/less.php/blob/v4.2.0/CHANGES.md