Bug 144095 – img.currentSrc problem in strict mode with old picturefill

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Simon Pieters (:zcorpan)]

See https://github.com/scottjehl/picturefill/issues/478

Microsoft discovered when implementing srcset in Project Spartan that some sites broke because they were using picturefill < 2.3.1 with "use strict" and picturefill was writing to currentSrc, which throws in strict mode. The issue has been fixed in picturefill but it's not certain all sites will upgrade in time. I think Microsoft decided to remove support for currentSrc for the time being. Alternative approaches would be to mark the property as [Replaceable] or have a no-op setter for now. Or leave it as is and evang any broken sites to upgrade picturefill.

If HTMLPictureElement is also supported, there is no issue with picturefill since it bails out early in that case. Safari 8 does not support currentSrc so this does not show up in current release.

[Liam Quinn]

The EFL WebKit port has been affected by this for awhile now. With the fix for bug #144679, all ports would now be affected.

Most of the images do not load on http://www.espn.com because of this issue.

[Yoav Weiss]

This is first and foremost a bug in the older versions of the polyfill.

With regard to ESPN, we have contacted their front-end engineers, and it seems like they will upgrade to the latest polyfill version shortly: https://twitter.com/steveclancy/status/603280160696225792

We (the RICG) also intend to send out multiple calls to the developer community to upgrade their polyfill versions immediately. I hope that would be enough to make this a non-issue.

Liam - Are there any other sites that you've encountered problems with? We can try to contact them directly as well.

[Liam Quinn]

I have not yet encountered other sites affected by this. Thanks for reaching out to ESPN!

[Dean Jackson]

Created attachment 253820 [details]
Patch

[Dean Jackson]

This patch allows us to remove currentSrc easily. It doesn't actually change the built result by default.

[Dean Jackson]

I landed an ENABLE guard in r184928. This can be removed once the real issue has been fixed.

[Jon Lee]

Is anyone aware how long this has been going on? Is ESPN's use of the polyfill relatively recent? I'm slightly surprised it took this long to notice given that we've had this on for over a year.

[Yoav Weiss]

Comment on attachment 253820 [details]
Patch

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

> Source/WebCore/html/parser/HTMLPreloadScanner.cpp:119
> +#endif

There's no need for these HTMLPreloadScanner changes.

> Source/WebCore/html/parser/HTMLPreloadScanner.cpp:161
> +#endif

Likewise, not needed in order to put currentSrc behind a flag

[Ahmad Saleem]

I think this got landed in following commit:

https://github.com/WebKit/WebKit/commit/9e8b15d2b25350db78f4462b1730a24d67e1dd52

Can this be marked as "RESOLVED FIXED"? Thanks!

[Ryosuke Niwa]

Yup, this patch got landed in https://commits.webkit.org/163437@main

[Radar WebKit Bug Importer]

<rdar://problem/97844424>