[webkit-reviews] review denied: [Bug 86160] CSS3 calc: blending involving expressions : [Attachment 149255] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Mike Lawther
<mikelawther at chromium.org>'s request for review:
Bug 86160: CSS3 calc: blending involving expressions
https://bugs.webkit.org/show_bug.cgi?id=86160

Attachment 149255: Patch
https://bugs.webkit.org/attachment.cgi?id=149255&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=149255&action=review


Code looks OK, but I'd like to see some more tests.

> Source/WebCore/ChangeLog:8
> +	   If either endpoint of a blend involves a colc expression, we create
a new

Nit: colc -> calc

> Source/WebCore/platform/Length.cpp:223
> +    return Length(CalculationValue::create(blend.release(),
CalculationRangeAll));

Is it necessary to return a calculation Length here?  E.g., if you evaluated
'from' and 'to' to create Fixed/Percent Lengths, could you call blend() from
the results?  This might avoid having to allocate 7 objects on the heap for
each frame of the transition.

> LayoutTests/css3/calc/transitions.html:13
> +#startCalcEndCalc, #startCalcEndPx, #startCalcEndPercent {
> +    width: -webkit-calc(10% + 50px);
> +    width: -moz-calc(10% + 50px);
> +}

Can we add a second test file that tests a calculation that is based on a
transitioning element?	E.g.,
#outer's width is transitioning and #inner's width is a calculation with a
percentage size (#inner could either be a fixed calculation or a calculation
that is transitioning).