[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

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

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

Where is the test case that covers “implicit” vs. “explicit”?

> Source/WebCore/css/CSSParser.cpp:3641
> +            return false; // Too many vlaues for this shorthand. Invalid declaration.

Typo "vlaues"

> Source/WebCore/css/CSSParser.cpp:3665
> +        // Time to assign the previously skipped 'auto' values to a property.
> +        // If both properties are unassigned at this point (i.e. 'columns:auto'),
> +        // it doesn't matter that much which one we set.
> +        // The one we don't set here will get an implicit 'auto' value further down.

If the implicit vs. explicit distinction matters, I don’t understand why it doesn’t matter in this case and an implicit auto is sufficient. To put it another way, if the implicit vs. explicit distinction did not matter, then we wouldn’t need hasPendingExplicitAuto at all!

> Source/WebCore/css/CSSParser.cpp:3680
> +        addProperty(CSSPropertyColumnWidth, cssValuePool().createIdentifierValue(CSSValueAuto), important, true /* implicit */);

I think this code should just pass !hasPendingExplicitAuto instead of passing "true" for implicit. Then we could get rid of the entire if statement above.

> Source/WebCore/css/CSSParser.cpp:3685
> +        addProperty(CSSPropertyColumnCount, cssValuePool().createIdentifierValue(CSSValueAuto), important, true /* implicit */);

Ditto.

> Source/WebCore/css/CSSParser.h:196
> +    PassRefPtr<CSSValue> parseColumnWidth();
> +    PassRefPtr<CSSValue> parseColumnCount();

For new functions, return type should just be RefPtr. There is no reason to use PassRefPtr for return values any more.

-- 
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/20150331/fcc56199/attachment-0002.html>