Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS3 calc() function doesn't work with LESS #974

Closed
mgol opened this issue Oct 8, 2012 · 51 comments
Closed

CSS3 calc() function doesn't work with LESS #974

mgol opened this issue Oct 8, 2012 · 51 comments
Milestone

Comments

@mgol
Copy link

mgol commented Oct 8, 2012

I know it's been reported already but bugs were closed due to insufficient information.

I have the following CSS code:

#id1 {
    position: fixed;
    left: 0; top: 0;
    width: 130px;
}
#id2 {
    position: fixed;
    right: 0; top: 0;
    width: calc(100% - 130px);
}

I wanted to transform it into LESS using 130px as a parameter but calc gets internpreted by LESS and this:

@id1width: 130px
#id1 {
    position: fixed;
    left: 0; top: 0;
    width: @id1width;
}
#id2 {
    position: fixed;
    right: 0; top: 0;
    width: calc(100% - @id1width);
}

makes the last but one line transformed to width: calc(-30%); which is clearly not what's desired. I tried using width: ~"calc(100% - @id1width)"; but it makes @id1width not interpreted.

I think LESS shouldn't use things reserved by CSS3 for its internal use...

@lukeapage
Copy link
Member

they were probably closed as duplicates.. though I can't find one about calc.. see #146 #122 and #915

workaround: width: ~"calc(100% - @{id1width})"; - note curly brackets around variable.

We are moving to a system where only things inside brackets will be evaluated to fix this problem.

@shankarcabus
Copy link

👍

@jonjohnjohnson
Copy link

@rows: 10;

@row1: 1 / @rows * 100%;

.featured1
{
  top: ~'-webkit-calc(@{row1} + 20px)';
  right: 0;
  bottom: @row5;
  left: @col9;
}

This bit of LESS will process out the value of '@row1' to be 10%, which is great. But when it is inside an escaped seciton of LESS and wrapped in curlys to retain the LESS variable it returns '10' without the '%'.

I've found a workaround that hasn't failed me yet. If you place another '%' after the closing curly of the 'row1' variable inside the escaped code it will work correctly...

~'-webkit-calc(@{row1}% + 20px)';

But that seems like quite a hack to add in another unit type that is suppose to already be in the variable.

@lukeapage
Copy link
Member

@jonjohnjohnson this is fixed in head and will be in 1.3.2

the rest of this bug will be resolved in 1.4.0

@faceless2
Copy link

Not sure if this is the same issue, but I'm having a problem with the following. Note there's no less-specific stuff here, less 1.3.3 is munging otherwise valid CSS.

Here's the CSS

html, body {
    margin:0, padding:0, border:0;
    height: 100%;
}

#content {
    height: -webkit-calc(100% - 40px);
    height: calc(100% - 40px);
    background-color:gray;
}

#footer {
    background-color:black;
}

And here's the HTML to include it

<!doctype html>
<html lang="en">
<head>
<link rel="stylesheet/less" type="text/css" href="test.css" />
<script src="less-1.3.3.min.js"></script>
<body>
<div id="content"></div>
<div id="footer" style="height:40px"></div>
</body>
</html>

"content" is being set to a height of 60%, so less is parsing & incorrectly resolving the calc expression rather than passing it unchanged through to the browser. Tested in Safari 6.0.2 and Firefox.

@lukeapage
Copy link
Member

Fixed on master for 1.4.0

@OliverJAsh
Copy link

height: ~"calc(100% - 50px)";

still produces:

height: calc(50%);

for me. I want:

height: calc(100% - 50px);

@OliverJAsh
Copy link

What’s more, it still produces height: calc(50%); even with strict math set to on.

@seven-phases-max
Copy link
Member

@OliverJAsh I can't reproduce your results with Less 1.7.0 (both escaped value and --strict-math:on work as expected)... Could you please provide more details on the compiler/environment/scripts you use? (Just in case there was Bootstrap build script issue that could lead to such incorrect results: twbs/bootstrap#13604, maybe this is your case too?).

@twiginteractive
Copy link

I was having this issue in 1.6.3 (for some reason, WinLESS is barfing on compile when I upgrade to 1.7.x, so I'm sticking to 1.6.x for now)

My quickfix was just to escape one part of the equation like:

height: calc(~"100%" - unit(@var, px));

This even works for variables, like @var: 50. Or you could escape the whole calculation like calc(~"100% - 50px");

@seven-phases-max
Copy link
Member

@twiginteractive If you have to use escaping with --strict-math option off (default setting) then it's not an issue but expected behaviour. See --strict-math.

@twiginteractive
Copy link

@seven-phases-max Hmmm - according to the docs, with SM off (default) then this should get parsed correctly (i.e. untouched)
height: calc(100% - 10px);

But it doesn't. The output CSS is height: calc(90%);, which isn't the desired result. Maybe this is fixed in 1.7, but as I say I can't use that version right now until I figure out what is breaking the WinLESS compile.

(Unless I am mis-reading the docs when it says "will be processed correctly"... LESS couldn't know the value of 100%, so it shouldn't do a mathematical computation on '100% - 10px')

@seven-phases-max
Copy link
Member

@twiginteractive

according to the docs, with SM off (default) then this should get parsed correctly (i.e. untouched)

No, the word there is currently not correctly (i.e. "will be processed currently").

@seven-phases-max seven-phases-max modified the milestone: 1.4.0 Jul 26, 2014
@twiginteractive
Copy link

@seven-phases-max Ah - my bad, it makes sense now. Thanks for the clarification.

@aamirshahzad
Copy link

height: ~"calc(100% - 50px)"; Worked for me.

@stevenvachon
Copy link

This is where Less.js begins to mess with your code. Supposedly v2.0 will have the option to prefix its functions like so:

height: _calc(50%);
height: calc(100% - 50%); /* browser-native */

@seven-phases-max
Copy link
Member

@stevenvachon

Supposedly v2.0 will have the option to prefix its functions like so:

This won't affect calc in any way. There's no built-in calc function, Less is just evaluating a math expression accordingly to its (Less's) rules. That's it. If you don't want this "mess" use --strict-math.

@lukeapage
Copy link
Member

P.s. the solution to this problem is strict maths. And maybe we should
force strict maths on when in a call to calc?

@stevenvachon
Copy link

The problem extends when using Myth after Less. Even with strictMath, we need to ~"calc(...)" so that Less ignores it.

@lukeapage
Copy link
Member

Why? Does myth remove parentheses? Strict math makes less a proper superset
of css.

@seven-phases-max
Copy link
Member

And maybe we should force strict maths on when in a call to calc?

We've discussed this in #1872 and there're a few arguments against "local strict-math:on" workarounds. The proposed solution (not quite decided yet) is in #1880.

@seven-phases-max
Copy link
Member

@stevenvachon

Even with strictMath, we need to ~"calc(...)" so that Less ignores it.

Example? I.e. a snippet where Less screws an expression inside calc with --strict-math=on.

@lukeapage
Copy link
Member

Since when will v2 prefix functions? Did we decide that and I forgot?

@seven-phases-max
Copy link
Member

Since when will v2 prefix functions? Did we decide that and I forgot?

No. I guess this was just slightly overestimated #2102 (comment).

@stevenvachon
Copy link

Yes, sorry, not "the option to" but "the ability write a plugin providing the option to". I'll have to test some code again with and without ~"" on calc(). I must've been mistaken a while back and just kept moving forward with that misunderstanding. I have some older code that I'll have to modify now.

@oshihirii
Copy link

oshihirii commented Aug 6, 2017

In a UIkit 3 theme I am using this in my_theme.less:

height: ~"calc(100vh - 20px)";

And it works.

@quytm2239
Copy link

It worked for me when writing in *.less file #974 (comment)

Thank @lukeapage ! 🥇

@ciaranlp
Copy link

ciaranlp commented Oct 19, 2017

Hi @mqliutie thanks for adding the mixin suggestion for calc, but, there is an extra ~ in your Mixin that was breaking the compilation:

the mixin should be

.calc(@prop, @val) {
    @{prop}: ~"-webkit-calc('@{val}')";
    @{prop}: ~"-moz-calc('@{val}')";
    @{prop}: ~"calc('@{val}')";
}

Input as follows:
.calc(width, "100% - 60px");

Output as follows:
width: calc(100% - 60px);

@thany
Copy link

thany commented Jan 25, 2018

Why is this issue closed?! It's not fixed.

I'm still getting calc(30%) when I write calc(50% - 20px). This is never what anyone wants. Apart from the fact that it's wrong to blindly calculate with different units, it should leave it as-is in a calc(), unless it can be calculated with 100% reliability (e.g. same units). I'm using LESS 2.7.3.

Please reopen this issue.

@jonjohnjohnson
Copy link

@thany
Refer to these comments, they describe how this isn't exactly an issue, as much as the default setting of processing math is a byproduct of history, and you can always change the default setting.
#974 (comment)
#974 (comment)

@thany
Copy link

thany commented Jan 26, 2018

@jonjohnjohnson That's just stating the problem again. If strict mode fixes it (haven't tried yet, shouldn't be neccesary to enable all kinds of nondefault flags in order to work around bugs) then it should be on by default.

No matter how you put it, this issue is not fixed by any definition.

@jonjohnjohnson
Copy link

@thany I was just trying to help you understand that the developers decided it's not an issue according to them. Those comments don't just restate the problem, they describe the developers stance on the issue. Good luck.

@seven-phases-max
Copy link
Member

@thany

then it should be on by default.

It can't be because this would immediately break zillion projects out there. So if you treat the default behaviour as an issue just set the documented option and be done. By any definition.

Why is this issue closed?!

Because the uptodate disscussion of how to improve the default behaviour is here.

@matthew-dean
Copy link
Member

@thany As @seven-phases-max, if you want to see this solved, contribute to #1880. I think the solution is there, but there hasn't been enough community weigh-in to come to a decision that is ready to implement. It's a long thread, but that's what solution-finding takes: reviewing proposals and weighing in one way or another. And then: someone to take on the work to implement.

matthew-dean added a commit to matthew-dean/less.js that referenced this issue Feb 10, 2018
matthew-dean added a commit that referenced this issue Feb 11, 2018
matthew-dean added a commit that referenced this issue Jun 30, 2018
* calc() fix - fixes #974
* Parses and retrieves a namespaced value
* Adds a bunch of new tests for aliasing and namespacing
* Added more CSS Grid tests
* Added tests for passing mixins into mixins, since it's just another value
* Release v3.5.0-beta.4
@xiaomizhou66
Copy link

 width: 15%;
 height: ~"calc(width)";

how to make height=width

@thany
Copy link

thany commented Nov 26, 2018

@xiaomizhou66
That's got nothing to do with LESS, nor with this issue. It's purely a CSS question. Please search for something like "css keep aspect" and you'll find some answers.

@willatathena
Copy link

So this is still broken. calc(100vh - 138px) comes out to -38px.

@seven-phases-max
Copy link
Member

@willatathena

If you expirience this problem with Less 3.x, please create a separate bug report.
For earlier Less versions this is expected behavior by default (read the posts above to find how to handle it).

wmfgerrit pushed a commit to wikimedia/less.php that referenced this issue Mar 11, 2023
== Less_Tree_Operation ==

In Less_Tree_Operation::operate(), we call Dimension::operate() with
argument $b (named $other there), which Dimension expects to be a
number with a unit, i.e. something we can perform math on.

Change Operation::operate() to not call this if argument $b is
something else, which avoids the reported PHP warning of $other->unit
being undefined. Noting that the only class with a 'unit' property
is Less_Tree_Dimension.

As a side-effect, this immediately fixes another bug, namely that the
"var()" right-hand side of `calc(100% - var(foo))` was completely
ignored previously when it wasn't a number. For example, it can be
a CSS keyword or CSS function like var() that the browser handles
client-side. Noting that CSS also natively supports math. LESS should
only handle math when the input uses PHP variables or literals, and
thus effectively has a constant outcome.

Previously, Dimention::operate() would implicitly read $other->value
as NULL when $other value wasn't a numerical Dimension object, and
thus the math operation silently failed before this commit, e.g.:
> "100% - var(foo)"
> "100 - NULL" (as %)
> "100%"

With the warning avoided/fixed, we now correctly preserve the
equation in its entirely when $b is e.g. a var() call.

== Less_Tree_Call ==

The above change on its own is insufficient, as that elevates the
warning to a fatal error. The added test case exposes this,
where `calc(100% - var(foo) - var(bar))` is parsed as:

  Call (calc)
  \-- Expression
      \-- Operation (-)
          |-- Operation (-_
          |    |- Dimension (100%)
          |    |- Call (var foo)
          |-- Call (var bar)

The above change, makes the first Operation pair no longer compile
Dimension+Call down to a Dimension, instead preserving Operation as
literal CSS output. This means the second Operation will now see a
left-hand value of Operation, which doesn't have an "operate" method,
and thus correctly triggers the `throw` statement that I cleaned up,
because that's not something we can compute server-side.

Fix this by recognising `calc()` higher up as something that
server-side LESS math should not be performed on in the first place.

This effectively backports less/less.js#3162
(less/less.js@a48c24c4dd3c13e00a20ece803237)
for the similarly reported issue upstream at
less/less.js#974.

That bugfix was originally released with less.js 3.0.0, but the
part of that I'm backporting here doesn't change behaviour of any
of our less.js 2.5.3 compliance tests, thus safe to backport.

We already threw a fatal error when one of the arguments wasn't a
Dimension or Color (no other class has the 'operate' method), so this
only adds non-fatal outcomes to previously fatal outcomes.

Bug: T331688
Change-Id: Idd443a76372682de4edddeb64ab5a65b23bbd3ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests