[webkit-reviews] review granted: [Bug 134057] [CSS Grid Layout] Update grid-auto-flow to the new syntax : [Attachment 234706] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jul 11 01:39:30 PDT 2014
Sergio Villar Senin <svillar at igalia.com> has granted Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 134057: [CSS Grid Layout] Update grid-auto-flow to the new syntax
https://bugs.webkit.org/show_bug.cgi?id=134057
Attachment 234706: Patch
https://bugs.webkit.org/attachment.cgi?id=234706&action=review
------- Additional Comments from Sergio Villar Senin <svillar at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=234706&action=review
Awesome! Thanks for the patch.
> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2057
> + else if (style->isGridAutoFlowDirectionColumn())
Is there any other possibility? I mean the direction is either row or column so
we can use a simple "else". If we want to be ultra-sure we could always add an
ASSERT(isColumn() || isRow()).
> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2063
> +
list->append(cssValuePool().createIdentifierValue(CSSValueStack));
Same comment here.
> Source/WebCore/css/CSSParser.cpp:5191
> + // Second parameter, if any.
We must improve this comment mentioning that this can be used by the grid
shorthand. It isn't obvious from the code why we don't bail out in some,
apparently erroneous cases.
More information about the webkit-reviews
mailing list