[webkit-reviews] review denied: [Bug 133620] Add support for HTMLImageElement's sizes attribute : [Attachment 232729] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 9 22:41:55 PDT 2014


Sam Weinig <sam at webkit.org> has denied Yoav Weiss <yoav at yoav.ws>'s request for
review:
Bug 133620: Add support for HTMLImageElement's sizes attribute
https://bugs.webkit.org/show_bug.cgi?id=133620

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=232729&action=review


> Source/WebCore/css/MediaList.cpp:126
> -    
> +

This change seems extraneous.

> Source/WebCore/css/MediaList.h:55
> -    
> +

The changes to this file seem extraneous.

> Source/WebCore/css/SourceSizeList.h:37
> +    SourceSize(MediaQueryExp* mediaExp, const CSSParserValue& length)

This should take a std::unique_ptr<MediaQueryExp> and move it into m_mediaExp.

> Source/WebCore/css/SourceSizeList.h:53
> +    void append(SourceSize* sourceSize)

This should take a std::unique_ptr<SourceSize> and move it onto the Vector.

> Source/WebCore/css/SourceSizeList.h:62
> +    Vector<std::unique_ptr<SourceSize> > m_list;

No need for the space between > and >

> Source/WebCore/html/HTMLImageElement.cpp:130
> +#if ENABLE_PICTURE_SIZES

We do ENABLE testing using the form #if ENABLE(PICTURE_SIZES). This is repeated
a bunch.

> Source/WebCore/html/parser/HTMLSrcsetParser.cpp:235
> +	       candidate.density = (float)candidate.resourceWidth /
(float)sourceSize;

Please use c++ style casts.


More information about the webkit-reviews mailing list