Allow number-percentage calc() in border-image-slice

Allow number-percentage calc() in border-image-slice

According to [1] border-image-slice should support calc() of the
number-percentage type, since it supports both <number> and <percentage>.

To support this add consumeNumberOrPercentage to enable CSSPropertyParser
to parse number-percentage calc(). When resolving border-image-slice use
convertToLength to correctly evaluate the calculation expression.

BUG=446860, 397061
[1] https://drafts.csswg.org/css-values-3/#percentages

[rwlbuis, 2016-03-29 14:56:34]

Description was changed from

==========
446860 WIP

BUG=
==========

to

==========
446860 WIP

BUG=446860, 397061
==========

[rwlbuis, 2016-03-29 21:04:03]

Description was changed from

==========
446860 WIP

BUG=446860, 397061
==========

to

==========
Allow number-percentage calc() in border-image-slice

According to [1] border-image-slice should support calc() of the
number-percentage type, since it supports both <number> and <percentage>.
To support this add consumeNumberOrPercentage to enable CSSPropertyParser
to parse number-percentage calc(). When resolving border-image-slice use
convertToLength to correctly evaluate the calculation expression.

BUG=446860, 397061

[1] https://drafts.csswg.org/css-values-3/#percentages
==========

[rwlbuis, 2016-03-29 21:04:41]

Description was changed from

==========
Allow number-percentage calc() in border-image-slice

According to [1] border-image-slice should support calc() of the
number-percentage type, since it supports both <number> and <percentage>.
To support this add consumeNumberOrPercentage to enable CSSPropertyParser
to parse number-percentage calc(). When resolving border-image-slice use
convertToLength to correctly evaluate the calculation expression.

BUG=446860, 397061

[1] https://drafts.csswg.org/css-values-3/#percentages
==========

to

==========
Allow number-percentage calc() in border-image-slice

According to [1] border-image-slice should support calc() of the
number-percentage type, since it supports both <number> and <percentage>.

To support this add consumeNumberOrPercentage to enable CSSPropertyParser
to parse number-percentage calc(). When resolving border-image-slice use
convertToLength to correctly evaluate the calculation expression.

BUG=446860, 397061

[1] https://drafts.csswg.org/css-values-3/#percentages
==========

[rwlbuis, 2016-03-29 21:05:29]

rob.buis@samsung.com changed reviewers:
+ alancutter@chromium.org

[rwlbuis, 2016-03-29 21:05:30]

PTAL.

[alancutter (OOO until 2018), 2016-03-30 00:45:47]

I suspect the presence of numbers in calc will hit ASSERT(category() !=
CalcNumber) during CSSBorderImageLengthBoxInterpolationType::maybeConvertValue()
when animating calcs.
Please add a test case to
animations/interpolation/border-image-width-interpolation.html that uses calc.

You'll probably have to extend accumulateLengthArray() to accept numbers as well
to make this work correctly. I don't like the idea of units unaccepted in
regular lengths appearing in there  but that's sort of already the case with
percentage. That function might be better named as accumulateCalcArray().

https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css3/calc/border-image-slice-with-calc.html
(right):

https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css3/calc/border-image-slice-with-calc.html:17:
border-image-slice: calc(50% - 6);
We should also test serialising these values from getComputedStyle().

[alancutter (OOO until 2018), 2016-03-30 01:14:01]

alancutter@chromium.org changed reviewers:
+ timloh@chromium.org

[alancutter (OOO until 2018), 2016-03-30 01:14:02]

My mistake, I got mixed up with border-image-width (which is another can of
worms).

This will probably hit ASSERT_UNUSED(side, side.isNumber() ||
side.isPercentage()) in CSSImageSliceInterpolationType::maybeConvertValue() when
animating with calcs.

https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSCalculationValue.cpp (right):

https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSCalculationValue.cpp:206: break;
Numbers aren't always converted straight to pixels in CSS, it would be better to
avoid adding this.

In terms of unit type sanity it's probably better to stop using Lengths on
ComputedStyle for border slices and instead make a PercentNumber data type.
+timloh WDYT?

[rwlbuis, 2016-03-31 20:59:04]

On 2016/03/30 00:45:47, alancutter wrote:
>
https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/css3/calc/border-image-slice-with-calc.html:17:
> border-image-slice: calc(50% - 6);
> We should also test serialising these values from getComputedStyle().

I changed border-image-slice-with-calc.html to use getComputedStyle instead.
Could you have a look to see if the expectation is correct? Or would we expect a
calc() expression here?

[alancutter (OOO until 2018), 2016-04-05 06:24:49]

On 2016/03/31 at 20:59:04, rob.buis wrote:
> On 2016/03/30 00:45:47, alancutter wrote:
> >
https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Layo...
> >
third_party/WebKit/LayoutTests/css3/calc/border-image-slice-with-calc.html:17:
> > border-image-slice: calc(50% - 6);
> > We should also test serialising these values from getComputedStyle().
> 
> I changed border-image-slice-with-calc.html to use getComputedStyle instead.
Could you have a look to see if the expectation is correct? Or would we expect a
calc() expression here?

I think we should be testing serialisation in addition to the ref test you had
before. The latest change drops the ref test.

I don't think calc(50% - 6) should compute to 15. According to spec "Percentages
are relative to the size of the image: the width of the image for the horizontal
offsets, the height for vertical offsets."
(https://drafts.csswg.org/css-backgrounds-3/#the-border-image-slice). Without
knowing the image data we shouldn't be able to resolve this to a number at
computed value time and so it should continue to resolve to calc(50% - 6).

[rwlbuis, 2016-04-19 20:42:36]

Patchset #5 (id:80001) has been deleted

[rwlbuis, 2016-04-28 03:05:41]

On 2016/04/05 06:24:49, alancutter wrote:
> On 2016/03/31 at 20:59:04, rob.buis wrote:
> > On 2016/03/30 00:45:47, alancutter wrote:
> > >
>
https://codereview.chromium.org/1835403003/diff/20001/third_party/WebKit/Layo...
> > >
> third_party/WebKit/LayoutTests/css3/calc/border-image-slice-with-calc.html:17:
> > > border-image-slice: calc(50% - 6);
> > > We should also test serialising these values from getComputedStyle().
> > 
> > I changed border-image-slice-with-calc.html to use getComputedStyle instead.
> Could you have a look to see if the expectation is correct? Or would we expect
a
> calc() expression here?
> 
> I think we should be testing serialisation in addition to the ref test you had
> before. The latest change drops the ref test.

I brought it back in the latest CL.

> I don't think calc(50% - 6) should compute to 15. According to spec
"Percentages
> are relative to the size of the image: the width of the image for the
horizontal
> offsets, the height for vertical offsets."
> (https://drafts.csswg.org/css-backgrounds-3/#the-border-image-slice). Without
> knowing the image data we shouldn't be able to resolve this to a number at
> computed value time and so it should continue to resolve to calc(50% - 6).

Sounds good, the latest CL now matches that (but with different order of number
and percentage). Still it is a bit messy, maybe we should try changing
PixelsAndPercent to know about numbers?

[dstockwell, 2016-05-20 04:36:20]

ping? @alancutter