[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 #5 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/ChangeLog:11
> +        * css3/calc/calc-errors-expected.txt:

Most of these tests are passing without your change. This only one that doesn't is:
FAIL Tests calc() with time units. assert_equals: calc(4s + 1s) should equal to 5s expected "5s" but got "0.005s"

This does look like a bug. But otherwise, converting time, angle and frequency units in CSS calc() function is already working. Right now, it is not really clear what this patch is doing.
I would focus on fixing what actually doesn't work (e.g. the case above) instead of refactoring.

> LayoutTests/css3/calc/calc-errors-expected.txt:1
> +unclosed calc

Could you explain what changes you did to this test and why? It is not in the Changelog and not obvious to me at least.

> 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.

> Source/WebCore/css/CSSPrimitiveValue.cpp:540
> +    switch (isCalculated() ? cssCalcValue()->primitiveType() : m_primitiveUnitType) {

Can't you call CSSPrimitiveValue::primitiveType() here?

-- 
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/383305d1/attachment-0002.html>