-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Image reevaluation not working? #512
Comments
Read: Try this: /**
* FF's first picture implementation is static and does not react to viewport changes, this tiny script fixes this.
*/
(function() {
/*jshint eqnull:true */
var ua = navigator.userAgent;
if ( window.HTMLPictureElement && ((/ecko/).test(ua) && ua.match(/rv\:(\d+)/) && RegExp.$1 < 41) ) {
addEventListener("resize", (function() {
var timer;
var dummySrc = document.createElement("source");
var fixRespimg = function(img) {
var source, sizes;
var picture = img.parentNode;
if(picture.nodeName.toUpperCase() == 'PICTURE'){
source = dummySrc.cloneNode();
picture.insertBefore(source, picture.firstElementChild);
setTimeout(function() {
picture.removeChild(source);
});
} else if(!img._pfLastSize || img.offsetWidth > img._pfLastSize) {
img._pfLastSize = img.offsetWidth;
sizes = img.sizes;
img.sizes += ',100vw';
setTimeout(function(){
img.sizes = sizes;
});
}
};
var findPictureImgs = function() {
var i;
var imgs = document.querySelectorAll("picture > img, img[srcset][sizes]");
for (i = 0; i < imgs.length; i++) {
if (imgs[i].currentSrc && !imgs[i].complete) {
removeEventListener("resize", onResize);
break;
}
fixRespimg(imgs[i]);
}
};
var onResize = function() {
clearTimeout(timer);
timer = setTimeout(findPictureImgs, 99);
};
dummySrc.srcset = "data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==";
return onResize;
})());
}
})(); an updated version of: |
Oh wow! Thanks @aFarkas. That totally worked for me. I'm glad I asked! =) |
@aFarkas @Wilto @mike-engel Should we discuss the possibility of putting this into core? Firefox shipping without handling resize natively is pretty painful. I have a feeling this is going to come up over and over again. I'm unclear from the release notes if they are going to force the patch or if it's going to follow the usual trickledown process. Meanwhile, even if authors don't notice it's broken, we should maybe still fix it. We can could refine the UA test when we know more. Correct resize behavior seems like it's in the wheelhouse for a polyfill. |
It also looks like Mozilla is punting their fix to FF 40 now, which is changing my mind on including this by default: https://bugzilla.mozilla.org/show_bug.cgi?id=1135812#c53 I would still want it as an easily-detached “plugin” kind of thing, just to facilitate removal eventually. |
Agree with putting it in core as a plugin. Is there any way to detect support for this? I'm guessing not. |
That’s tough. I could see cobbling together a test for it, but not without more overhead than it’s probably worth. This feels like it lands squarely in UA-sniff territory, with a detailed comment pointing to the bugzilla thread. |
There is already some kind of a detect in this code: if (imgs[i].currentSrc && !imgs[i].complete) {
removeEventListener("resize", onResize);
break;
} |
So then if we're having the problem of resizing with FF, is that code running prematurely/falsely? |
Alright, from the slack channel, consensus seems to be to include the gecko-picture plugin with pf by default for a while until we're comfortable FF38 usage is low enough. |
@aFarkas one optimization to consider before we ship: consider using timestamp for debouncing instead of setTimeout. The perf is better: http://modernjavascript.blogspot.com/2013/08/building-better-debounce.html |
good call. But this is more important then (because it runs in all browsers): Would be great to do it there as well. Maybe use a simplified smaller variation. |
I'm not so sure wether I still want this in by default. This bug is already happening and we haven't done a quick release for the 2.x branch. We also know, that this bug is already fixed with version 40. So people, who want to fix it, already do this by using the "extension". But if we add it to the core, we might never be able to remove it from the core (at least until next FF ESR update, which is in one year). What do you think? |
I think included by default makes sense—for a while, anyway—but definitely not built into the core itself. |
What is the difference between included and built into the core. My mind changed about this yesterday, because of our webp merge. I'm just a little bit concerned about filesize. We exceeded the 11kb with this. And actually I wanted to stay under 10kb minified. |
Something we build into |
Hi All,
I'm not sure if this is a bug or if my expectations for the behavior are incorrect.
This is in Firefox v38.0.5 for Mac.
1.) Resize a web browser window as small as you can.
2.) Go here: http://scottjehl.github.io/picturefill/examples/demo-01.html
3.) Resize your web browser window larger.
I thought that the image would change and load a larger version if the window is resized. Here's an animated gif showing how upper left always says "medium.jpg".
Many thanks,
-Tim
The text was updated successfully, but these errors were encountered: