|Summary:||img.currentSrc problem in strict mode with old picturefill|
|Product:||WebKit||Reporter:||Simon Pieters <zcorpan>|
|Component:||DOM||Assignee:||Dean Jackson <dino>|
|Severity:||Normal||CC:||dino, eoconnor, jonlee, lquinn, mathias, yoav|
|Version:||528+ (Nightly build)|
Description Simon Pieters 2015-04-23 05:10:50 PDT
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.
Comment 1 Liam Quinn 2015-05-26 06:45:17 PDT
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.
Comment 2 Yoav Weiss 2015-05-26 14:33:31 PDT
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.
Comment 3 Liam Quinn 2015-05-26 14:41:26 PDT
I have not yet encountered other sites affected by this. Thanks for reaching out to ESPN!
Comment 5 Dean Jackson 2015-05-27 17:12:52 PDT
This patch allows us to remove currentSrc easily. It doesn't actually change the built result by default.
Comment 6 Dean Jackson 2015-05-27 17:29:39 PDT
I landed an ENABLE guard in r184928. This can be removed once the real issue has been fixed.
Comment 7 Jon Lee 2015-05-29 00:50:31 PDT
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.
Comment 8 Yoav Weiss 2015-06-01 01:06:11 PDT
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