Bug 144095

Summary: img.currentSrc problem in strict mode with old picturefill
Product: WebKit Reporter: Simon Pieters (:zcorpan) <zcorpan>
Component: DOMAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: ahmad.saleem792, ap, bfulgham, dino, eoconnor, jonlee, lquinn, mathias, rniwa, webkit-bug-importer, yoav
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=144679
https://bugs.webkit.org/show_bug.cgi?id=150443
Attachments:
Description Flags
Patch simon.fraser: review+

Simon Pieters (:zcorpan)
Reported 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.
Attachments
Patch (55.10 KB, patch)
2015-05-27 17:11 PDT, Dean Jackson
simon.fraser: review+
Liam Quinn
Comment 1 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.
Yoav Weiss
Comment 2 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.
Liam Quinn
Comment 3 2015-05-26 14:41:26 PDT
I have not yet encountered other sites affected by this. Thanks for reaching out to ESPN!
Dean Jackson
Comment 4 2015-05-27 17:11:26 PDT
Dean Jackson
Comment 5 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.
Dean Jackson
Comment 6 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.
Jon Lee
Comment 7 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.
Yoav Weiss
Comment 8 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
Ahmad Saleem
Comment 9 2022-07-30 07:10:55 PDT
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
Comment 10 2022-07-30 13:25:22 PDT
Yup, this patch got landed in https://commits.webkit.org/163437@main
Radar WebKit Bug Importer
Comment 11 2022-07-30 13:26:15 PDT
Note You need to log in before you can comment on or make changes to this bug.