Closed Bug 956573 Opened 11 years ago Closed 6 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: super.driver.512, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, DevAdvocacy)

Attachments

(1 file)

Working vs. broken HTML files.
495 bytes, application/x-bzip2
Details 
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.
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.
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?
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?
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).
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);
(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)
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
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
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.
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: → 1291240
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
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: → 1362409
Hi,

I got new bug when I set position relative and overflow-x hidde then I've use height with calc value to subtract some value. But height doesn't take from parent element. It shows ZERO height

.threemonk {
	height: calc(100% - 42px);
	overflow-x: hidden;
	position: relative;
}

. threemonk has parent which has position aboslute and it has child with overflow-y: auto. Then inside that child element has this threemonk div

But it works fine in safari and chrome.

Kindly fix it
That's a different bug. Also, I'm pretty sure that with stylo we support calc pretty much everywhere, modulo <resolution>, which is not usable from any CSS property yet at least.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: