[Webkit-unassigned] [Bug 138356] Converting time, angle and frequency units in CSS calc() function

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138356

--- Comment #6 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 240930
  --> https://bugs.webkit.org/attachment.cgi?id=240930
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240930&action=review

> LayoutTests/css3/calc/calc-with-time-angle-and-frequency-values-expected.txt:2
> +PASS Tests calc() with time units. 

We need more example that are currently failing. As I said before, most of these seem to be passing without fix.

>> Source/WebCore/css/CSSCalculationValue.h:97
>> +    unsigned short primitiveType() const { return m_expression->primitiveType(); }
> 
> Do we really need this? CSSPrimitiveValue::primitiveType() already handles calculated values.

It seems this is needed after call, but it should probably be called from CSSPrimitiveValue::primitiveType().

>> Source/WebCore/css/CSSPrimitiveValue.cpp:540
>> +    switch (isCalculated() ? cssCalcValue()->primitiveType() : m_primitiveUnitType) {
> 
> Can't you call CSSPrimitiveValue::primitiveType() here?

Hmm, ok, so CSSPrimitiveValue::primitiveType()'s handling of calculated values is actually not great. It does not really check the unit inside the calculated expression. Instead, it relies on the category and returns CSS_MS if category is CalcTime. We should fix CSSPrimitiveValue::primitiveType().

> Source/WebCore/css/CSSPrimitiveValue.h:-262
> -        if (timeUnit == Seconds && m_primitiveUnitType == CSS_S)

We could potentially keep the same implementation if we call primitiveType() instead of accessing m_primitiveUnitType directly.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150106/8b8d438d/attachment-0002.html>