calc() CSS function doesn't work with all data types (units) that it should, such as times

NEW
Unassigned

Status

()

Core
CSS Parsing and Computation
4 years ago
2 months ago

People

(Reporter: Guilherme Vieira, Unassigned)

Tracking

(Blocks: 1 bug, {dev-doc-needed, DevAdvocacy})

Trunk
dev-doc-needed, DevAdvocacy
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
Created attachment 8355878 [details]
Working vs. broken HTML files.

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140104030204

Steps to reproduce:

<!doctype html>
<style>
	div
	{
		width: calc(16px + 16px);
		height: 48px;
		background-image: url("http://i.imgur.com/EUdDa8b.png");
		animation-name: spritesheet;
		animation-duration: calc(1s + 1s);
		animation-timing-function: steps(4, end);
		animation-iteration-count: infinite;
	}
	@keyframes spritesheet
	{
		from
		{
			background-position: 0 0;
		}
		to
		{
			background-position: calc(4 * 32px) 0;
		}
	}
</style>
<div></div>


Actual results:

width property computed to 32px successfully, but animation-duration gets removed from the rule.

Adding it using Firebug shows the red box indicating the value entered is invalid, but I see no reason it ought to be.


Expected results:

animation-duration value should be computed to 2s.
(Reporter)

Comment 1

4 years ago
If someone can confirm this is an issue and not my misunderstanding of where calc() is allowed per the spec, I would like to volunteer fixing it.

I've never seen the Gecko code base, so I'd need some help. What's the best place I could get it from? Some mailing list? An IRC channel?
I think your interpretation of the spec is correct, though I also think the spec may have changed since we implemented calc().  (There might not have even been that much of a spec at the time.)

If you're interested in trying to fix it, it's probably best to start with the first few links in https://developer.mozilla.org/en-US/docs/Developer_Guide (mainly the first three, "Getting Started", "Working with Mozilla Source Code", and "Build instructions".)

The code you'd need to change lives entirely within the layout/style/ subdirectory.  You'd need to teach the CSS parser (layout/style/nsCSSParser.cpp) about calc() expressions with time units, for example, by relaxing this bit of ParseVariant:
  if ((aVariantMask & VARIANT_CALC) &&
      (eCSSToken_Function == tk->mType) &&
      (tk->mIdent.LowerCaseEqualsLiteral("calc") ||
       tk->mIdent.LowerCaseEqualsLiteral("-moz-calc"))) {
    // calc() currently allows only lengths and percents inside it.
    return ParseCalc(aValue, aVariantMask & VARIANT_LP);
  }
and then add VARIANT_CALC to the properties with time units in nsCSSPropList.h.  Then you'd need to teach the code in nsRuleNode.cpp about how to compute the calc() expressions for those properties; the code would look more like the existing SETCOORD_CALC_LENGTH_ONLY than the existing SETCOORD_STORE_CALC code, because the calc() expression wouldn't need to remain as part of the computed value of the property.  I think those should be the major pieces, though I might be missing something.

Then you'd need automated tests both to ensure the code you're writing works as you think and to ensure that it keeps working in the future; adding entries to layout/style/test/property_database.js *might* be sufficient since it will cause a bunch of interesting tests to run in the various test files that run tests on all the values in that database.  But it might also be worth adding some additional tests, especially if you hit interesting cases you think are worth testing.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Version: 29 Branch → Trunk
Oh, and to be clear, it'll probably require more changes than just the one to nsCSSParser.cpp, but that chunk I pointed to is a good place to start looking.
(Reporter)

Comment 4

4 years ago
Awesome. I'll see what I can do, thanks a lot. I think I won't immediately assign the issue to myself since I'm not sure I'll manage to resolve it. What do you think?
(Reporter)

Comment 5

4 years ago
I should probably add that this doesn't work for `<integer>` computations either, at least it doesn't in the steps() timing function:

    animation-timing-function: steps(calc(2 + 2), end); /* This blows as well. */

That's why I initially thought it was something specific about animation properties, but maybe it's just not implemented for the rarely-used `<integers>` too, right?
(Reporter)

Updated

4 years ago
Summary: calc() CSS function doesn't work on animation properties → calc() CSS function doesn't work with all data types it should
Integers might be a little harder to get the unit-checking right, since the existing unit-checking code might not work for integer-valued properties or for properties that take mixes of integers and lengths, like 'line-height' (on which I believe there's an existing bug with some discussion).
(Reporter)

Comment 7

4 years ago
In my experience integer operands work well as long as one of the operands is a length. E.g. `calc(4 * 32px)` works just fine.
No, I was talking about getting the unit-checking working correctly for things like
  line-height: calc(0.8 * 1.2 + 0.2 * 16px);

Comment 9

4 years ago
(In reply to David Baron [:dbaron] (needinfo? me) (UTC+8) from comment #6)
> Integers might be a little harder to get the unit-checking right, since the
> existing unit-checking code might not work for integer-valued properties or
> for properties that take mixes of integers and lengths, like 'line-height'
> (on which I believe there's an existing bug with some discussion).

(for background, this is bug 594933 - see also bug 984021)

Comment 10

3 years ago
I just encounter this bug with a simple

line-height: calc(10rem * 4);

See detail trace of my use case here

http://codepen.io/MoOx/pen/sBlhG

Comment 11

3 years ago
Another use case to add that isn't working properly:

http://codepen.io/DavidBradbury/pen/toIKy

-

According to the spec, calc() should be able to be used — “wherever <length>, <frequency>, <angle>, <time>, <number>, or <integer> values are allowed.”

RGB and RGBA both use integer or number values so this should be working (Is there no unit tests?). Chrome seems to get it, but only when using percentages.
There are plenty of tests.  However, because of the way calc() works, it has to be implemented separately (to some extent) for each piece of syntax or property that allows it.  I initially implemented it in the places that seemed most important and then stopped.  If the requirement had been a complete implementation or no implementation, I would have chose no implementation, since it was too much work to be worthwhile.  That said, adding support in additional areas, where they are valuable, is absolutely something we can do.

That said, this bug is about supporting more units, which is separate from comment 11.
Summary: calc() CSS function doesn't work with all data types it should → calc() CSS function doesn't work with all data types (units) that it should, such as times

Comment 13

2 years ago
I just came across this myself while playing around with CSS variables. I can foresee some interesting ways in which variables might be used, but calc not working with all units kind of breaks them. Not sure that I can contribute much here, just wanted to point out a growing need.
Yeah, just got into this issue while using CSS variables.

Comment 15

2 years ago
Another use-case - a time-line for css animations, ie using the animation-delay property in combination with css custom properties in Polymer :

      :host {
...
        --spin1start: 0.5s;
        --spin2start: 2.6s;
      }
...
      .bubble {
...
        animation:
          bubbleboom
            0.75s
            ease-out
            calc( var(--spin2start) + 4.5s )
            forwards,
...
See Also: → bug 1291240
Duplicate of this bug: 1318305

Comment 17

6 months ago
Another usecase not working in Firefox is `transform: rotate(calc(1turn / 6));`

http://codepen.io/anon/pen/RKOjGV?editors=1100
http://codepen.io/dannyfritz/pen/BjdNLy?editors=0100
Keywords: DevAdvocacy
I notice David Khourshid's Reactive Animations presentation from CSSConfEU relies on using calc() for transition-duration quite a bit: http://slides.com/davidkhourshid/getting-reactive-with-css
See Also: → bug 1362409
Keywords: dev-doc-needed
Blocks: 1376206
You need to log in before you can comment on or make changes to this bug.