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

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({css3})

Trunk
mozilla2.0b7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
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.)
(Assignee)

Updated

7 years ago
Summary: support calc on background-position, background-size, -moz-transform-position, and background-image gradient stop positions → support calc on background-position, background-size, -moz-transform-origin, and background-image gradient stop positions
(Assignee)

Updated

7 years ago
Summary: support calc on background-position, background-size, -moz-transform-origin, and background-image gradient stop positions → support calc() on background-position, background-size, -moz-transform-origin, and background-image gradient stop positions
(Assignee)

Comment 1

7 years ago
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).
blocking2.0: --- → betaN+
(Assignee)

Comment 2

7 years ago
Created attachment 474983 [details] [diff] [review]
patch 1: change background-position and background-size storage a little

I think this makes nsStyleBackground 12 bytes larger.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #474983 - Flags: review?(bzbarsky)
(Assignee)

Comment 3

7 years ago
Created attachment 474984 [details] [diff] [review]
patch 2: implement calc() for these

This has some interdependencies on bug 520234 that aren't worth eliminating.
Attachment #474984 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

7 years ago
Created attachment 474985 [details] [diff] [review]
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.)
Attachment #474984 - Attachment is obsolete: true
Attachment #474985 - Flags: review?(bzbarsky)
Attachment #474984 - Flags: review?(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.
Attachment #474983 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

7 years ago
(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 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
Attachment #474985 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 8

7 years ago
(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.
(Assignee)

Comment 9

7 years ago
http://hg.mozilla.org/mozilla-central/rev/ad02d75f80ab
http://hg.mozilla.org/mozilla-central/rev/983d21f8af4a
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
Depends on: 647885
Depends on: 657905
You need to log in before you can comment on or make changes to this bug.