Less.js: Incorrect type-checking for CSS length units

[rjgotten]

For reference, the current contents of the Unit.prototype.isLength method:

Unit.prototype.isLength = function () {
    return Boolean(this.toCSS().match(/px|em|%|in|cm|mm|pc|pt|ex/));
};

If we look at the section of the CSS Values and Units Module Level 3 specification that treats <length> units, we find that the current isLength implementation is incorrect and incomplete.

The function fails to recognize the viewport-relative units (vw, vh, vmin, vmax); the quarter millimeter (Q) absolute unit; and the zero-advance (ch) font-relative units as length units.

Through a fluke, it does recognize the root em (rem) font-relative unit, because the used regex has no start and end anchors in place and it matches "em". However, this works both ways: the function currently also incorrectly recognizes the dots-per-pixel (dppx) resolution unit as a length unit, because it matches "px" in the regex.

The function also should not be recognizing percentages as a length unit. While many operations and properties that accept length units _also_ accept percentages, percentages themselves are not length units according to the specification.

(Also; the implementation should be using the RegExp.prototype.test method which returns an actual boolean, instead of using the String.prototype.match method and casting the result.)

[matthew-dean]

Why is this prototype method useful?

[rjgotten]

Why is this prototype method useful?

In some cases you want to guard mixins to execute only when a length is passed and fail otherwise.

While it's possible to concatenate conditions for all the possible types of length unit together in Less, that tends to lead to a very bloated mixin guard clause. It is far more readable and maintainable to have an islength plugin function, similar to all the other type-checking functions, and have that access the underlying isLength on the Unit prototype.

[jacobwarduk]

I'd like to pick this up if it's still available 🙋

[matthew-dean]

@jacobwarduk Go for it!

[matthew-dean]

@jacobwarduk Unfortunately Github doesn't let me assign to you if you're not on a Github team? Oh well, it's yours.

[jacobwarduk]

Thanks, I'll get to work on it.

[jacobwarduk]

Updated PR #3172 as per comments 🙂

[seven-phases-max]

Closing as fixed in #3172.