Bug 145573 – No need to guard the sizes attribute against PICTURE_SIZES

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dean Jackson]

No need to guard the sizes attribute against PICTURE_SIZES

[Radar WebKit Bug Importer]

<rdar://problem/21210038>

[Dean Jackson]

Created attachment 254111 [details]
Patch

[Dean Jackson]

Committed r185130: <http://trac.webkit.org/changeset/185130>

[Myles C. Maxfield]

Comment on attachment 254111 [details]
Patch

rs=me

[Darin Adler]

Comment on attachment 254111 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        The PICTURE_SIZES feature flag doesn't need to be used to
> +        guard preloading of the sizes attribute.

I guess this should be obvious to me, but why?

[Yoav Weiss]

The PICTURE_SIZES guard was put back in just on the currentSrc attribute implementation, due to https://bugs.webkit.org/show_bug.cgi?id=144095 and the fact that we'd have to remove currentSrc support unless the high-profile sites that use the old polyfill will update it soon.

Since the issue only involves the currentSrc attribute, the flag should only guard its implementation, and not the sizes attribute (despite the flag's name - it retained the name of the old flag, that guarded the entire feature).

[Darin Adler]

OK. We should rename the flag unless it’s going away soon.

[Csaba Osztrogonác]

(In reply to comment #3)
> Committed r185130: <http://trac.webkit.org/changeset/185130>

This change broke the !ENABLE(PICTURE_SIZES) build:

./../Source/WebCore/html/parser/HTMLPreloadScanner.cpp: In member function 'void WebCore::TokenPreloadScanner::StartTagScanner::processAttributes(const AttributeList&, WebCore::Document&)':
../../Source/WebCore/html/parser/HTMLPreloadScanner.cpp:115:46: error: 'm_sizesAttribute' was not declared in this scope
../../Source/WebCore/html/parser/HTMLPreloadScanner.cpp: In member function 'void WebCore::TokenPreloadScanner::StartTagScanner::processAttribute(const NameType&, const WTF::String&)':
../../Source/WebCore/html/parser/HTMLPreloadScanner.cpp:154:57: error: 'm_sizesAttribute' was not declared in this scope

Could you check and fix the build?

[Yoav Weiss]

Took also the m_sizesAttribute definition from behind the guard at https://bugs.webkit.org/show_bug.cgi?id=145926

I believe it should do the trick.

[Csaba Osztrogonác]

(In reply to comment #9)
> Took also the m_sizesAttribute definition from behind the guard at
> https://bugs.webkit.org/show_bug.cgi?id=145926
> 
> I believe it should do the trick.

Thanks, I'll check it soon.

[zalan]

Build fix: http://trac.webkit.org/changeset/185501