[webkit-reviews] review granted: [Bug 66701] Add CSS parsing of -webkit-flex-pack : [Attachment 104715] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 22 14:22:35 PDT 2011


Eric Seidel <eric at webkit.org> has granted Tony Chang <tony at chromium.org>'s
request for review:
Bug 66701: Add CSS parsing of -webkit-flex-pack
https://bugs.webkit.org/show_bug.cgi?id=66701

Attachment 104715: Patch
https://bugs.webkit.org/attachment.cgi?id=104715&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=104715&action=review


> Source/WebCore/css/CSSPrimitiveValueMappings.h:1056
> +template<> inline CSSPrimitiveValue::CSSPrimitiveValue(EFlexPack e)
> +    : m_type(CSS_IDENT)
> +    , m_hasCachedCSSText(false)
> +{
> +    switch (e) {
> +    case PackStart:
> +	   m_value.ident = CSSValueStart;
> +	   break;
> +    case PackEnd:
> +	   m_value.ident = CSSValueEnd;
> +	   break;
> +    case PackCenter:
> +	   m_value.ident = CSSValueCenter;
> +	   break;
> +    case PackJustify:
> +	   m_value.ident = CSSValueJustify;
> +	   break;
> +    }
> +}

I feel a little sad inside every time I read one of these.  More code could be
shared between all of these.  I guess I should pick on Luke.

> Source/WebCore/rendering/style/RenderStyleConstants.h:169
> +// New CSS3 Flexbox Properties

The New isn't needed.  They won't be New forever. :)

> Source/WebCore/rendering/style/StyleFlexibleBoxData.h:54
> +    unsigned m_flexPack : 2; // EFlexPack

Bleh.  public members.	Bleh.


More information about the webkit-reviews mailing list