[Webkit-unassigned] [Bug 143248] [CSS MultiColumn] Parse "columns: auto <length>" shorthand property value properly

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #249890|review?                     |review+, commit-queue-
              Flags|                            |

--- Comment #17 from Darin Adler <darin at apple.com> ---
Comment on attachment 249890
  --> https://bugs.webkit.org/attachment.cgi?id=249890
Patch

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

> Source/WebCore/css/CSSParser.cpp:3618
> +RefPtr<CSSValue> CSSParser::parseColumnWidth()
> +{
> +    ValueWithCalculation valueWithCalculation(*m_valueList->current());
> +    CSSValueID id = valueWithCalculation.value().id;
> +    // Always parse this property in strict mode, since it would be ambiguous otherwise when used in the 'columns' shorthand property.
> +    if (id == CSSValueAuto || (validateUnit(valueWithCalculation, FLength | FNonNeg, CSSStrictMode) && parsedDouble(valueWithCalculation))) {
> +        RefPtr<CSSValue> parsedValue = parseValidPrimitive(id, valueWithCalculation);
> +        m_valueList->next();
> +        return parsedValue;
> +    }
> +
> +    return nullptr;
> +}

I think these functions would read better with early return (check for error and return nullptr) rather than nesting the normal code path inside an if.

> Source/WebCore/css/CSSParser.cpp:3661
> +    ASSERT(columnCount || coulmnWidth);

This will fail to compile because of the typo: coulmnWidth.

> LayoutTests/fast/css/getPropertyValue-columns.html:38
> +      shouldBeEqualToString('columnsValue("columns4")', 'auto auto');

Is this serialization of "auto" as "auto auto" correct behavior, or a bug?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150401/c5c391ea/attachment.html>