Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions packages/less/lib/less/tree/nested-at-rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,40 @@ import Node from './node.js';
* }} NestableAtRuleThis
*/

// A media query may only carry a media type at its front, before any
// conditions (https://drafts.csswg.org/mediaqueries-5/#typedef-media-query-list).
// Operators that join media-query parts, so a fragment leading with one is not
// a media type. Any other bare identifier is treated as a type, since unknown
// media types are valid non-matches in CSS, not syntax errors.
const MEDIA_QUERY_OPERATORS = ['and', 'or'];

/**
* Whether a flattened media-query fragment leads with a media type, e.g.
* `screen`, `only screen`, `print and (color)` or an unknown type like `foo`.
* @param {Node & { value?: * }} fragment
*/
function startsWithMediaType(fragment) {
let head;
if (fragment.type === 'Keyword' || fragment.type === 'Anonymous') {
head = fragment.value;
} else if (fragment.type === 'Expression' && Array.isArray(fragment.value)) {
const parts = fragment.value.filter(p => p && p.value !== undefined);
let idx = 0;
const first = parts[idx] && String(parts[idx].value).toLowerCase();
if (first === 'not' || first === 'only') { idx++; }
head = parts[idx] && parts[idx].value;
}
if (typeof head !== 'string' || head === '') {
return false;
}
// Inspect only the first token. A media type is a bare identifier; a feature
// condition begins with '(' - e.g. an escaped ~"(max-width: 1px)" is an
// Anonymous whose whole value is "(max-width: 1px)", which is not a media type.
const firstToken = head.trim().split(/[\s(]/)[0];
return firstToken !== ''
&& MEDIA_QUERY_OPERATORS.indexOf(firstToken.toLowerCase()) < 0;
Comment on lines +52 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | grep -F 'packages/less/lib/less/tree/nested-at-rule.js' || true

echo "== relevant lines =="
if [ -f packages/less/lib/less/tree/nested-at-rule.js ]; then
  nl -ba packages/less/lib/less/tree/nested-at-rule.js | sed -n '1,240p'
fi

echo "== search startsWithMediaType usages =="
rg -n "startsWithMediaType|MEDIA_QUERY_OPERATORS|NestedAtRule|nested-at-rule" packages/less/lib/less -S || true

Repository: less/less.js

Length of output: 275


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant source =="
awk 'NR>=1 && NR<=260 { printf "%6d\t%s\n", NR, $0 }' packages/less/lib/less/tree/nested-at-rule.js

echo "== searches =="
rg -n "startsWithMediaType|MEDIA_QUERY_OPERATORS|NestedAtRule|nested-at-rule" packages/less/lib/less -S || true

echo "== simple parser probe =="
node - <<'JS'
const MEDIA_QUERY_OPERATORS = ['and', 'or', 'not']; // replace with actual value if needed, but search will show it
function startsWithMediaType(fragment) {
    let head;
    if (fragment.type === 'Keyword' || fragment.type === 'Anonymous') {
        head = fragment.value;
    } else if (fragment.type === 'Expression' && Array.isArray(fragment价值)) {
      // actual impl not needed for this probe
    }
    if (typeof head !== 'string' || head === '') return false;
    const firstToken = head.trim().split(/[\s(]/)[0];
    return firstToken !== '' && ['and','or','not'].indexOf(firstToken.toLowerCase()) < 0;
}
for (const input of ['screen', 'not (color)', 'only screen']) {
  console.log(input, '->', startsWithMediaType({type:'Anonymous', value: input}));
}
JS

Repository: less/less.js

Length of output: 12974


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== find parser source for `@media` and Anonymous/Expression/Keyword values =="
rg -n "Anonymous|Expression|Keyword|MEDIA|media|not\\(|only " packages/less/testless packages/less/lib packages/less/src -S --glob '!**/dist/**' --glob '!**/build/**' || true

echo "== AST/source candidate files =="
git ls-files | rg '(^|/)(anonymous|expression|keyword|media|nested-at-rule)\.(js|ts)$|\.less$' | head -200

Repository: less/less.js

Length of output: 49897


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== `@media` parsing section =="
awk 'NR>=1880 && NR<=2130 { printf "%6d\t%s\n", NR, $0 }' packages/less/lib/less/parser/parser.js

echo "== tests for escaped media not/only =="
rg -n "~\"not|~'not|not \\(|only screen|only (screen|print)|nested at-rule|issue `#3694`|issue `#3764`" packages/less/test.less packages/test-data -S -g '*.less' --glob '!**/node_modules/**' || true

echo "== deterministic startsWithMediaType behavior with current impl =="
node - <<'JS'
const MEDIA_QUERY_OPERATORS = ['and', 'or'];
function startsWithMediaType(fragment) {
  let head;
  if (fragment.type === 'Keyword' || fragment.type === 'Anonymous') {
    head = fragment.value;
  } else if (fragment.type === 'Expression' && Array.isArray(fragment.value)) {
    const parts = fragment.value.filter(p => p && p.value !== undefined);
    let idx = 0;
    const first = parts[idx] && String(parts[idx].value).toLowerCase();
    if (first === 'not' || first === 'only') { idx++; }
    head = parts[idx] && parts[idx].value;
  }
  if (typeof head !== 'string' || head === '') return false;
  const firstToken = head.trim().split(/[\s(]/)[0];
  return firstToken !== '' && MEDIA_QUERY_OPERATORS.indexOf(firstToken.toLowerCase()) < 0;
}
for (const value of ['screen', 'not (color)', 'only screen', '~"not (color)"', 'only screen and (color)', 'screen and not (color)']) {
  console.log(value, '->', startsWithMediaType({ type: 'Anonymous', value }));
}
JS

Repository: less/less.js

Length of output: 17993


Skip media-query modifiers in string-valued fragments too.

Keyword/Anonymous fragments do not skip not/only, so parsed inputs like @media not (color) are reclassified as Anonymous('not (color)') and sorted ahead of later types like screen. Inspect the non-modifier token for these fragments before deciding whether a following (...) must remain a condition.

Proposed fix
     if (fragment.type === 'Keyword' || fragment.type === 'Anonymous') {
-        head = fragment.value;
+        const raw = fragment.value;
+        if (typeof raw === 'string') {
+            const tokens = raw.trim().split(/\s+/);
+            let idx = 0;
+            const first = tokens[idx] && tokens[idx].toLowerCase();
+            if (first === 'not' || first === 'only') { idx++; }
+            head = tokens[idx];
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function startsWithMediaType(fragment) {
let head;
if (fragment.type === 'Keyword' || fragment.type === 'Anonymous') {
head = fragment.value;
} else if (fragment.type === 'Expression' && Array.isArray(fragment.value)) {
const parts = fragment.value.filter(p => p && p.value !== undefined);
let idx = 0;
const first = parts[idx] && String(parts[idx].value).toLowerCase();
if (first === 'not' || first === 'only') { idx++; }
head = parts[idx] && parts[idx].value;
}
if (typeof head !== 'string' || head === '') {
return false;
}
// Inspect only the first token. A media type is a bare identifier; a feature
// condition begins with '(' - e.g. an escaped ~"(max-width: 1px)" is an
// Anonymous whose whole value is "(max-width: 1px)", which is not a media type.
const firstToken = head.trim().split(/[\s(]/)[0];
return firstToken !== ''
&& MEDIA_QUERY_OPERATORS.indexOf(firstToken.toLowerCase()) < 0;
function startsWithMediaType(fragment) {
let head;
if (fragment.type === 'Keyword' || fragment.type === 'Anonymous') {
const raw = fragment.value;
if (typeof raw === 'string') {
const tokens = raw.trim().split(/\s+/);
let idx = 0;
const first = tokens[idx] && tokens[idx].toLowerCase();
if (first === 'not' || first === 'only') { idx++; }
head = tokens[idx];
}
} else if (fragment.type === 'Expression' && Array.isArray(fragment.value)) {
const parts = fragment.value.filter(p => p && p.value !== undefined);
let idx = 0;
const first = parts[idx] && String(parts[idx].value).toLowerCase();
if (first === 'not' || first === 'only') { idx++; }
head = parts[idx] && parts[idx].value;
}
if (typeof head !== 'string' || head === '') {
return false;
}
// Inspect only the first token. A media type is a bare identifier; a feature
// condition begins with '(' - e.g. an escaped ~"(max-width: 1px)" is an
// Anonymous whose whole value is "(max-width: 1px)", which is not a media type.
const firstToken = head.trim().split(/[\s(]/)[0];
return firstToken !== ''
&& MEDIA_QUERY_OPERATORS.indexOf(firstToken.toLowerCase()) < 0;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/less/lib/less/tree/nested-at-rule.js` around lines 52 - 71, Update
startsWithMediaType so Keyword and Anonymous fragments also skip leading
not/only modifiers before evaluating the media type. Inspect the first
non-modifier token and preserve the existing condition handling for following
parenthesized expressions, so inputs like “not (color)” are not classified as
media types.

}

const NestableAtRulePrototype = {

isRulesetLike() {
Expand Down Expand Up @@ -149,6 +183,13 @@ const NestableAtRulePrototype = {
/** @param {Node & { toCSS?: Function }} fragment */
fragment => fragment.toCSS ? fragment : new Anonymous(/** @type {string} */ (/** @type {unknown} */ (fragment))));

// A media type nested inside conditions must move ahead of them
// so the flattened query stays valid (issue #3694, #3764).
const types = /** @type {Node[]} */ (path).filter(startsWithMediaType);
if (types.length && types.length < /** @type {Node[]} */ (path).length) {
path = types.concat(/** @type {Node[]} */ (path).filter(f => !startsWithMediaType(f)));
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}

for (i = /** @type {Node[]} */ (path).length - 1; i > 0; i--) {
/** @type {Node[]} */ (path).splice(i, 0, new Anonymous('and'));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
@media screen and (max-width: 500px) {
.a {
color: red;
}
}
@media only screen and (min-width: 100px) {
.b {
color: blue;
}
}
@media all and (max-width: 9px) {
.c {
color: green;
}
}
@media print and (color) and (min-width: 1px) {
.d {
color: black;
}
}
@media screen and (max-width: 500px) {
.e {
color: red;
}
}
@media (max-width: 500px) and (min-width: 100px) {
.f {
color: red;
}
}
@media tester and (max-width: 500px) {
.g {
color: red;
}
}
@media screen and (max-width: 500px) {
.h {
color: red;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// A media type nested inside conditions must lead the flattened query,
// otherwise the result is invalid CSS (issue #3694, #3764).
@media (max-width: 500px) {
@media screen {
.a { color: red; }
}
}
@media (min-width: 100px) {
@media only screen {
.b { color: blue; }
}
}
@media (max-width: 9px) {
@media all {
.c { color: green; }
}
}
@media (min-width: 1px) {
@media print and (color) {
.d { color: black; }
}
}

// Already-correct ordering is preserved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
@media screen {
@media (max-width: 500px) {
.e { color: red; }
}
}

// Queries without a media type are left untouched.
@media (max-width: 500px) {
@media (min-width: 100px) {
.f { color: red; }
}
}

// Unknown media types are valid CSS non-matches, so they reorder too.
@media (max-width: 500px) {
@media tester {
.g { color: red; }
}
}

// An escaped-string feature condition (Anonymous) must not be mistaken for a media type.
@fc: ~"(max-width: 500px)";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Prevent Stylelint from rejecting the valid LESS variable declaration.

@fc: ... is valid LESS, but the active scss/at-rule-no-unknown rule flags it and can fail lint. Add a LESS-specific override or suppress this fixture line.

Proposed local suppression
+// stylelint-disable-next-line scss/at-rule-no-unknown
 `@fc`: ~"(max-width: 500px)";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@fc: ~"(max-width: 500px)";
// stylelint-disable-next-line scss/at-rule-no-unknown
`@fc`: ~"(max-width: 500px)";
🧰 Tools
🪛 Stylelint (17.14.0)

[error] 46-46: Unexpected unknown at-rule "@fc:" (scss/at-rule-no-unknown)

(scss/at-rule-no-unknown)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.less` at
line 46, Update the LESS variable declaration `@fc` in the media-nested-type
fixture so the active scss/at-rule-no-unknown lint rule no longer rejects it,
using a narrowly scoped LESS-specific override or suppression for this line
only.

Source: Linters/SAST tools

@media @fc {
@media screen {
.h { color: red; }
}
}
Loading