[webkit-reviews] review granted: [Bug 206909] REGRESSION (r254406): Gmail.com star/favorite icons are not rendering : [Attachment 389111] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Noam Rosenthal
<noam at webkit.org>'s request for review:
Bug 206909: REGRESSION (r254406): Gmail.com star/favorite icons are not
rendering
https://bugs.webkit.org/show_bug.cgi?id=206909

Attachment 389111: Patch

https://bugs.webkit.org/attachment.cgi?id=389111&action=review




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 389111
  --> https://bugs.webkit.org/attachment.cgi?id=389111
Patch

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

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1706
> +    if ((range.peek().type() == StringToken) && (allowedImageTypes &
AllowedImageType::RawStringAsURL)) {

I think we prefer allowedImageTypes.contains(AllowedImageType::RawStringAsURL)

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1714
>	   CSSValueID id = range.peek().functionId();

Not this patch, but please don't use 'id'; it's a reserved word in Objective-C.

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1715
> +	   if ((allowedImageTypes & AllowedImageType::GeneratedImage) &&
isGeneratedImage(id))

.contains()

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1718
> +	   if (allowedImageTypes & AllowedImageType::ImageSet) {

.contains()

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1726
> +    if (allowedImageTypes & AllowedImageType::URLFunction) {

.contains()