[Webkit-unassigned] [Bug 64464] New: [CSS Exclusions] Fix for comment #23 on wrap-shape parsing bug 61726

[bugzilla-daemon at webkit.org]

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

           Summary: [CSS Exclusions] Fix for comment #23 on wrap-shape
                    parsing bug 61726
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: UNCONFIRMED
          Severity: Normal
          Priority: P2
         Component: CSS
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: achicu at adobe.com


This is a fix for comment #23 from Tony Chang on bug 61726:

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

> Source/WebCore/css/CSSParser.cpp:86
> +#include "CSSWrapShapes.h"

Nit: It's better to put the #if in the .h file and always include the file.  This reduces the number of #ifs and makes it easier for people to move files even if a feature is disabled.

> Source/WebCore/css/CSSParser.cpp:3742
> +        if (!valid)
> +            break;

Nit: Wouldn't it be simpler to just return 0 here?

> Source/WebCore/css/CSSParser.cpp:3765
> +                valid = false;
> +                break;

Nit: ... and here.

> Source/WebCore/css/CSSParser.cpp:3771
> +    if (!valid || argumentNumber < 3)

Nit: Then you wouldn't need the variable valid anymore.

> Source/WebCore/css/CSSWrapShapes.h:152
> +    PassRefPtr<CSSPrimitiveValue> getXAt(unsigned i) { return m_values.at(i << 1); }

Nit: Please use * 2 rather than bit shifting.

> Source/WebCore/css/CSSWrapShapes.h:153
> +    PassRefPtr<CSSPrimitiveValue> getYAt(unsigned i) { return m_values.at((i << 1) & 1); }

This looks wrong, isn't it always 0?  I would just write (i * 2) + 1.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.