594934 - support calc() on background-position, background-size, -moz-transform-origin, and background-image gradient stop positions

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We probably want to support calc() on support calc on background-position, background-size, -moz-transform-position, and background-image gradient stop positions.  These things all use the same syntax, and don't currently store in in nsStyleCoord form.

This is work that I did not do in bug 363249 / bug 585715, and am not currently planning to do for Mozilla 2 / Firefox 4.

However, it occurs to me that the removal of min() and max() currently pending review in bug 363249 might make this much easier, and thus something I would want to do.  (I think I did get a request for it.)

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since the one piece of concrete feedback I got about http://dbaron.org/log/20100905-calc regarding missing properties was lack of calc() on background-position, I should try get this in; it's pretty easy (I did it this afternoon along with a bunch of other things).

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 1: change background-position and background-size storage a little

I think this makes nsStyleBackground 12 bytes larger.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2: implement calc() for these

This has some interdependencies on bug 520234 that aren't worth eliminating.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2: implement calc() for these

Actually, I had a separate patch right on top of this fixing a bug that it introduced; no point keeping it separate.  (I actually filed bug 596138 before noticing this.)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 474983 [details] [diff] [review]
patch 1: change background-position and background-size storage a little

>+++ b/layout/base/nsCSSRendering.cpp
>+ScaleDimension(const nsStyleBackground::Size::Dimension& aDimension,
>+    case nsStyleBackground::Size::eLengthPercentage:
>+      return (double(aDimension.mPercent) * double(aAvailLength) +

Unit analysis failure here.  This needs a divide by double(aLength), right?

r=me with that fixed.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #5)
> Comment on attachment 474983 [details] [diff] [review]
> patch 1: change background-position and background-size storage a little
> 
> >+++ b/layout/base/nsCSSRendering.cpp
> >+ScaleDimension(const nsStyleBackground::Size::Dimension& aDimension,
> >+    case nsStyleBackground::Size::eLengthPercentage:
> >+      return (double(aDimension.mPercent) * double(aAvailLength) +
> 
> Unit analysis failure here.  This needs a divide by double(aLength), right?
> 
> r=me with that fixed.

It has one.  I guess I should rewrap the lines to match the parentheses better, though.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 474985 [details] [diff] [review]
patch 2: implement calc() for these

> ScaleDimension(const nsStyleBackground::Size::Dimension& aDimension,
>-      return (double(aDimension.mPercent) * double(aAvailLength) +
>-              double(aDimension.mLength)) / double(aLength);
>+      // negative values could result from calc()
>+      return NS_MAX(double(aDimension.mPercent) * double(aAvailLength) +
>+                      double(aDimension.mLength),
>+                    0.0) / double(aLength);

So this actually fixes the bug the previous patch introduced....

Please follow a followup on the FIXMEs in ExtractComputedValue?

r=me

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #7)
> Please follow a followup on the FIXMEs in ExtractComputedValue?

They're already fixed in patch 2 in bug 520234.  I'll add a test, though; I realize that wasn't tested.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/ad02d75f80ab
http://hg.mozilla.org/mozilla-central/rev/983d21f8af4a